On Sat, 21 Mar 2026 04:18:41 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. 
>> 
>> `TableView`:
>> 
>> Item Count | Master Select | Fixed Select | Select Improv. | Master Deselect 
>> | Fixed Deselect | Deselect Improv.
>> -- | -- | -- | -- | -- | -- | --
>> 16,384 | 7ms | 7ms | — | 83ms | 4ms | ~20.8x
>> 32,768 | 13ms | 12ms | ~1.1x | 331ms | 5ms | ~66.2x
>> 65,536 | 25ms | 17ms | ~1.5x | 1,283ms | 8ms | ~160.4x
>> 131,072 | 63ms | 28ms | ~2.3x | 5,142ms | 13ms | ~395.5x
>> 262,144 | 195ms | 58ms | ~3.4x | 20,526ms | 23ms | ~892.4x
>> 
>> `TreeTableView`:
>> 
>> Item Count | Master Select | Fixed Select | Select Improv. | Master Deselect 
>> | Fixed Deselect | Deselect Improv.
>> -- | -- | -- | -- | -- | -- | --
>> 16,384 | 310ms | 313ms | — | 81ms | 1ms | ~81.0x
>> 32,768 | 1,451ms | 1,448ms | — | 315ms | 3ms | ~105.0x
>> 65,536 | 6,505ms | 6,629ms | — | 1,247ms | 6ms | ~207.8x
>> 131,072 | 37,034ms | 39,452ms | — | 5,045ms | 11ms | ~458.6x
>> 262,144 | 260,019ms | 256,854ms | — | 20,025ms | 22ms | ~910.2x
>> 
>> 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:
> 
>   add unit test

modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java
 line 6520:

> 6518:         sm1.selectRange(3, 7);
> 6519:         sm2.selectIndices(3, 4, 5, 6);
> 6520:         assertEquals(sm1.getSelectedIndices(), 
> sm2.getSelectedIndices());

I guess we should also `assert` the selected indices here and below. 
`assertEquals(List.of(3, 4, 5, 6), sm1.getSelectedIndices());`

So we are 100% ccertain that the content is also what we expected (Right now we 
only assert that `selectRange` and `selectIndices` works the same. But if both 
would suffer from the very same bug, we would not catch that regression here).

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

PR Review Comment: https://git.openjdk.org/jfx/pull/2100#discussion_r2976859731

Reply via email to