RFR: 8197991: Selecting many items in a TableView is very slow [v3]

Abhinay Agarwal duke at openjdk.java.net
Thu Jan 6 08:32:16 UTC 2022


On Wed, 22 Dec 2021 13:32:06 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> A cache that's manually invalidated and then validated when needed is a form of lazy evaluation. The main point, regardless of how you name it, is to ensure that every call that modifies the underlying BitSet invalidates the size. If it does, and it can be proven to do so, then that's sufficient.
>
> I checked, and I think that `set(int index, int... indices)` is fine, since it doesn't modify `bitset` directly, and all of the methods it calls that do modify it, correctly invalidate `size`.
> 
> One more thing I spotted that needs to be checked, is the `SelectedIndicesList(BitSet)` constructor. It saves a reference to the source `BitSet` without making a copy. If a caller ever modified the source `BitSet`, it would change the underlying `BitSet` seen by the `SelectedIndicesList` class, and the size would therefore be wrong. Even if the current code doesn't do this, it's somewhat fragile. I recommend adding a comment to the constructor, and to the one place it's called, noting that the source `BitSet` must not be modified after passing it to the constructor.
> 
> Finally, there is a commented out `clearAndSelect` method that would break the new contract of invalidating size whenever the bitmap is modified, if that method were ever uncommented. Either it should be deleted as part of this PR or it should be modified with a (commented out, of course) `size = -1` in the right place(s).

I have added some tests which calls public methods in `MultipleSelectionModelBase` and in turn calls methods from `SelectedIndicesList`.

-------------

PR: https://git.openjdk.java.net/jfx/pull/673


More information about the openjfx-dev mailing list