On Sat, 7 Mar 2026 05:45:24 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!

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.

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.

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

Changes requested by kcr (Lead).

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

Reply via email to