On Sun, 29 Mar 2026 04:10:35 GMT, Charlie Schlosser <[email protected]> wrote:

> Fixes [8380933](https://bugs.openjdk.org/browse/JDK-8380933)
> 
> `TableView` and `TreeTableView` notify listeners when the selected cells 
> change after a sort with a very expensive check:
> 
> 
> if (!newState.contains(prevItem)) {
>     removed.add(prevItem);
> }
> 
> 
> If `removed` is not empty, the method conservatively notifies the listeners 
> that the entire table was replaced:
> 
> 
> ListChangeListener.Change<TreeTablePosition<S, ?>> c = new 
> NonIterableChange.GenericAddRemoveChange<>(0, itemCount, removed, newState);
> 
> 
> The slowness is not attributed to the sort at all, but the logic in handling 
> change notifications. We can preserve this behavior and address the 
> performance issues with minimal changes by creating a temporary `HashSet` for 
> the `contains` checks. 
> 
> However, this change notification is conservative and includes cells that 
> weren't affected by the sort. For example:
> 
> before sort: `{"a", "c", "b"}` with selected cells `{"0", "1"}`
> 
> after sort: `{"a", "b", "c"}` with selected cells `{"0", "2"}`
> 
> Although the first item is not affected by the sort, the current code will 
> notify listeners that cells `{"0", "2"}` were added and `{"1"}` was removed. 
> That is, the entire selection set was replaced. 
> 
> This PR scans `prevState` and `newState` and fires change events for the 
> affected ranges only, which addresses the performance issues, and minimizes 
> notifications. This PR also handles the scenario where the size of the 
> selection set is changed after the sort. This seemed prudent as a custom sort 
> policy could do anything to the selection set.
> 
> Some benchmarks with a `TableView` of integers with `selectAll`, illustrating 
> that the performance issue had nothing to do with the sort, and was entirely 
> attributed to the `contains` loop. 
> 
> Input Size | Old Time (ms) | New Time (ms)
> -- | -- | --
> 64 | 1 | 1
> 128 | 0 | 0
> 256 | 0 | 0
> 512 | 1 | 0
> 1024 | 3 | 0
> 2048 | 4 | 1
> 4096 | 14 | 0
> 8192 | 47 | 1
> 16384 | 186 | 1
> 32768 | 844 | 1
> 65536 | 3603 | 2
> 131,072 | 16,096 | 2

Reviewers: @andy-goryachev-oracle

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

PR Comment: https://git.openjdk.org/jfx/pull/2131#issuecomment-4156090764

Reply via email to