JDK-8197991: Selecting many items in a TableView is very slow

[koeberlue]

I created a TableView, set the selection model to SelectionMode.MULTIPLE and added lots of items to it (~8000). When I hit ctrl + a to select all elements in the table, the application freezes for approximately 6 seconds. This gets significantly worse when I add more items. With 20000 items I have to wait for more than a minute.

Here is a small sample application to reproduce the behavior:

public class SlowTableApplication extends Application {

    @Override
    public void start(Stage stage) {
        var table = new TableView<>();
        table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
        table.getColumns().add(new TableColumn<>());
        IntStream.range(0, 20_000).forEach(i -> table.getItems().add(new Object()));
        stage.setScene(new Scene(table));
        stage.show();
    }
}

Do I need to implement my own SelectionModel to make this work? Since TableView uses VirtualFlow, I assumed that there would not be a great performance hit when selecting a large amount of items.

---

I am using Version 12.0.1 of javafx-controls and the javafx-maven-plugin to run the application.

[koeberlue]

This seems to be a known issue: https://bugs.openjdk.java.net/browse/JDK-8197991

Does anyone have an idea for a workaround until the issue is resolved?

[Folling]

This issue does not seem to exist on arch-linux with javafx 8 and jdk 8.
However I can confirm it persists on javafx 11+
Custom TableCell does not seem to help, neither does a custom TableRow.
However a TableViewSelectionModel which does literally nothing fixes the issue, so I presume there might be a workaround in that direction.

[yososs]

When I checked the hotspot for the selectAll () operation in the profile, I noticed that there was a problem with the next call flow.

• ReadOnlyUnbackedObservableList.indexOf ()
• MultipleSelectionModelBase$SelectedIndicesList.size ()
• java.util.BitSet.cardinality ()

In the indexOf method of ReadOnlyUnbackedObservableList.java,
Size () is called in the loop.

    @Override public int indexOf (Object o) {
        if (o == null) return -1;

        for (int i = 0; i <size (); i ++) {
            Object obj = get (i);
            if (o.equals (obj)) return i;
        }
        return -1;
    }

It is thought that this performance problem will be improved by rewriting as follows.

    @Override public int indexOf (Object o) {
        if (o == null) return -1;
        final int n = size ();
        for (int i = 0; i <n; i ++) {
            Object obj = get (i);
            if (o.equals (obj)) return i;
        }

        return -1;
    }

It seems that the toArray () method has the same problem, so it needs to be fixed.

   @Override
     public Object [] toArray () {
         Object [] arr = new Object [size ()];
         for (int i = 0; i <size (); i ++) {
             arr [i] = get (i);
         }
         return arr;
     }

I think that I can fix this problem easily, so please fix it.

[kevinrushforth]

@yososs If you would like to contribute a fix, please see CONTRIBUTING.md. As noted there, we will need an Oracle Contributor Agreement (OCA) from you.

[yososs]

all right. I will proceed, so please wait.

[yososs]

I signed the OCA and sent an email.

How to fix this problem is described above, but do you think that you need PR for contribution?

[kevinrushforth]

I signed the OCA and sent an email.

Your OCA is now recorded, so we can accept contributions from you. Welcome!

but do you think that you need PR for contribution?

Yes, that is the best way to contribute this fix. Please also provide a test program that can be used to measure the performance before and after your proposed fix. A manual test in tests/manual is fine, since we don't have a automated performance testing framework. Finally, please run all of the existing tests, including the automated system tests + robot, and the Ensemble app, locally to help ensure no regressions.

Thanks.

[KniKnaKnorke]

Any news here? It is a real blocker for me.

[cstack89]

I went ahead and implemented the various classes needed to be able to override those functions. Unfortunately the fix provided does not seem to improve performance enough to make much difference for me.

Both the indexOf call in ReadOnlyUnbackedObservableList and the get call in SelectedIndicesList scale with the number of selected rows.

I am currently looking at ControlUtils to see if changing updateSelectedIndices to use parallelStream instead of stream would make any difference.

[yososs]

By adding the following patch, the waiting time is about 2 seconds in the case of 20000 records.

./modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java

    class SelectedIndicesList extends ReadOnlyUnbackedObservableList<Integer> {
        private final BitSet bitset;
        private int size = -1;

...

        @Override public int size() {
              if(size>=0){
                  return size;
              }
              size = bitset.cardinality();
              return size;
        }

....

        @Override public void _endChange() {
            if (!isAtomic()) {
                super._endChange();
            }
            size = -1;
        }

Before thinking about parallel execution,
I think that it is the stage where the duplication process inside the loop is taken out of the loop first.

[yososs]

With the next patch, it will be a practical processing time up to about 500,000 records. This should be investigated for side effects.

   class SelectedIndicesList extends ReadOnlyUnbackedObservableList<Integer> {

...

        public void set(int index) {
            if (!isValidIndex(index) || isSelected(index)) {
                return;
            }
            _beginChange();
            bitset.set(index);
//            int indicesIndex = indexOf(index);
              int indicesIndex = lastIndexOf(index);

            _nextAdd(indicesIndex, indicesIndex + 1);
            _endChange();
        }

If OpenJFX restricts this list to the premise of using a unique list, you can avoid linear search by using lastIndexOf instead of indexOf.

As you can see from the SelectedIndicesList name, this assumption is probably true.

[Antezedens]

@yososs, did you create the PR yet? We would also greatly benefit from the fix, so I hope that there is some progress here.

[yososs]

I've actually seen performance improvements with the code suggested in my comments. The rest of the work is just writing a PR, but I need to end the high priority work that I am currently working on.

[yososs]

Pull Request(openjdk/jfx#127) was made.