RFR: 8197991: Selecting many items in a TableView is very slow [v5]

[Kevin Rushforth]

On Thu, 6 Jan 2022 08:17:53 GMT, Abhinay Agarwal <duke at openjdk.java.net> wrote:

>> This work improves the performance of `MultipleSelectionModel`  over large data sets by caching some values and avoiding unnecessary calls to `SelectedIndicesList#size`. It further improves the performance by reducing the number of iterations required to find the index of an element in the BitSet.
>> 
>> The work is based on [an abandoned patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs
>> 
>> There are currently 2 manual tests for this fix.
>
> Abhinay Agarwal has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remove commented out method. Document constructors.
>  - Add tests

The fix looks good. I left a few comments on the tests.

modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java line 1427:

> 1425:         assertTrue(model.isSelected(2));
> 1426:         assertTrue(model.isSelected(5));
> 1427:         assertFalse(model.isSelected(0));

I recommend to also check the size.

modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java line 1436:

> 1434:         model.clearSelection();
> 1435: 
> 1436:         assertTrue(model.getSelectedIndices().isEmpty());

I recommend to also check the size.

tests/manual/controls/SelectTableViewTest.java line 40:

> 38: public class SelectTableViewTest extends Application {
> 39: 
> 40:     final int ROW_COUNT = 700_000;

This count is too large, even with the fix. I recommend changing it to something smaller (no more than `70_000`, which matches what you did for `SelectListViewTest`).

tests/manual/controls/SelectTableViewTest.java line 101:

> 99:         long t = System.currentTimeMillis();
> 100:         tableView.getSelectionModel().selectAll();
> 101:         System.out.println("time:"+ (System.currentTimeMillis() - t));

Minor: space before the `+` here and a few places below (also in the other test as noted).

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

PR: https://git.openjdk.java.net/jfx/pull/673