On Sun, 15 Mar 2026 11:38:42 GMT, chuckyschluz <[email protected]> wrote:

>> Fixes [JDK-8311505](https://bugs.openjdk.org/browse/JDK-8311505)
>> 
>> This PR uses `set(int index, int end, boolean isSet)` in 
>> `javafx.scene.control.MultipleSelectionModelBase` to avoid excessive calls 
>> to `indexOf` when deselecting continuous ranges of rows. 
>> 
>> Benchmarks produced with the test application provided here: 
>> https://bugs.openjdk.org/browse/JDK-8311505
>> 
>> Before (mainline):
>> 
>> 
>> Select item count 500000 took 724
>> Deselect item count 500000 took 73103
>> 
>> 
>> After (this PR):
>> 
>> 
>> Select item count 500000 took 249
>> Deselect item count 500000 took 88
>> 
>> 
>> Passes all unit tests:
>> 
>> `.\gradlew.bat :controls:test`
>> `.\gradlew.bat :systemTests:test`
>> 
>> Thank you!
>
> chuckyschluz has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   minor cleanup

`MultipleSelectionModelBase` is also used by `TreeView` and `ListView`, so I'd 
expect the same issue there as well.  However, that's not exactly what I see 
(I've updated the standalone monkey tester to include a 500,000 item option - 
https://github.com/andy-goryachev-oracle/MonkeyTest ).

In the `ListView`, there seems to be no problem deselecting after selection all 
500,000 items (there is a related PR #2109 , with a reproducer that does not 
seem to select anything, I am not sure about the exact use case there).

In the `TreeView`, on the other hand, selecting all locks up the UI (I pressed 
cmd-A in the tree view page a few minutes ago, the beach ball is still 
spinning).  This occurs with this PR fix as well - why?

Lastly, would it be possible to come up with a (headless) test?  It might be 
hard to write a test for timing, but the ticket says the deselection step takes 
something like 100x the time it takes to select - maybe we can have two 
measurements taken - one to select all 500k rows, another to deselect, and 
check if delesection take less than 2x ?  What do you think?

modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
 line 741:

> 739:         public void set(int index, int end, boolean isSet) {
> 740:             _beginChange();
> 741:             size = -1;

was this change necessary?

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

PR Review: https://git.openjdk.org/jfx/pull/2100#pullrequestreview-3955915523
PR Review Comment: https://git.openjdk.org/jfx/pull/2100#discussion_r2942251183

Reply via email to