On Sat, 7 Mar 2026 15:05:27 GMT, Kevin Rushforth <[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!
>
> We won't formally review this until your OCA is recorded, but there is one 
> thing I'll comment on now. Adding new public API to a publicly exported class 
> (by virtue of inheritance in this case) to fix a bug is not appropriate 
> unless that public API stands on its own merit. I doubt that is the case 
> here, but before we would even consider it, it needs discussion on the 
> mailing list. See [this section in the CONTRIBUTING 
> guidelines](https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md#new-features--api-additions).
> 
> I recommend looking for a fix that doesn't require new public API.

@kevinrushforth Happy Friday Kevin. My OCA is approved according to my account 
in https://oca.opensource.oracle.com/. Is there anything I can do on my end to 
assist with the OCA verification? Thank you.

> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
>  line 836:
> 
>> 834:         }
>> 835: 
>> 836:         public void clear(List<Integer> indices) {
> 
> This is new public API and will need further discussion before we can 
> consider it.

Thank you for the feedback. I did not consider the impact of introducing a new 
public API. 

I have pushed some proposed changes that use the pre-existing public API and 
achieve the same (better even) speed up. 


Select item count 500000 took 266
Deselect item count 500000 took 97


Thank you!

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

PR Comment: https://git.openjdk.org/jfx/pull/2100#issuecomment-4057153631
PR Review Comment: https://git.openjdk.org/jfx/pull/2100#discussion_r2901376086

Reply via email to