On Mon, 30 Mar 2026 15:48:49 GMT, Andy Goryachev <[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 Reviewers: @andy-goryachev-oracle @Maran23 ------------- PR Comment: https://git.openjdk.org/jfx/pull/2131#issuecomment-4185534030
