8197991: Selecting many items in a TableView is very slow

Backport-of: 8c4f966b163f68c517574e38c394b9349551ad08

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java

@@ -113,7 +113,7 @@ public void callObservers(Change<E> c) {
     @Override public int indexOf(Object o) {
         if (o == null) return -1;
 
-        for (int i = 0; i < size(); i++) {
+        for (int i = 0, max = size(); i < max; i++) {
             Object obj = get(i);
             if (o.equals(obj)) return i;
         }
@@ -185,8 +185,9 @@ public Iterator<E> iterator() {
 
     @Override
     public Object[] toArray() {
-        Object[] arr = new Object[size()];
-        for (int i = 0; i < size(); i++) {
+        int max = size();
+        Object[] arr = new Object[max];
+        for (int i = 0; i < max; i++) {
             arr[i] = get(i);
         }
         return arr;

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

@@ -345,6 +345,7 @@ boolean isAtomic() {
         BitSet selectedIndicesCopy = new BitSet();
         selectedIndicesCopy.or(selectedIndices.bitset);
         selectedIndicesCopy.clear(row);
+        // No modifications should be made to 'selectedIndicesCopy' to honour the constructor.
         List<Integer> previousSelectedIndices = new SelectedIndicesList(selectedIndicesCopy);
 
         // RT-32411 We used to call quietClearSelection() here, but this
@@ -626,6 +627,7 @@ private void quietClearSelection() {
     class SelectedIndicesList extends ReadOnlyUnbackedObservableList<Integer> {
         private final BitSet bitset;
 
+        private int size = -1;
         private int lastGetIndex = -1;
         private int lastGetValue = -1;
 
@@ -637,10 +639,18 @@ class SelectedIndicesList extends ReadOnlyUnbackedObservableList<Integer> {
 //            throw new RuntimeException("callObservers unavailable");
 //        }
 
+        /**
+         * Constructs a new instance of SelectedIndicesList
+         */
         public SelectedIndicesList() {
             this(new BitSet());
         }
 
+        /**
+         * Constructs a new instance of SelectedIndicesList from the provided BitSet.
+         * The underlying source BitSet shouldn't be modified once it has been passed to the constructor.
+         * @param bitset Bitset to be used.
+         */
         public SelectedIndicesList(BitSet bitset) {
             this.bitset = bitset;
         }
@@ -694,6 +704,7 @@ public void set(int index) {
             }
 
             _beginChange();
+            size = -1;
             bitset.set(index);
             int indicesIndex = indexOf(index);
             _nextAdd(indicesIndex, indicesIndex + 1);
@@ -714,6 +725,7 @@ public void set(int index, boolean isSet) {
 
         public void set(int index, int end, boolean isSet) {
             _beginChange();
+            size = -1;
             if (isSet) {
                 bitset.set(index, end, isSet);
                 int indicesIndex = indexOf(index);
@@ -791,6 +803,7 @@ public void set(int index, int... indices) {
         public void clear() {
             _beginChange();
             List<Integer> removed = bitset.stream().boxed().collect(Collectors.toList());
+            size = 0;
             bitset.clear();
             _nextRemove(0, removed);
             _endChange();
@@ -800,51 +813,12 @@ public void clear(int index) {
             if (!bitset.get(index)) return;
 
             _beginChange();
+            size = -1;
             bitset.clear(index);
             _nextRemove(index, index);
             _endChange();
         }
 
-//        public void clearAndSelect(int index) {
-//            if (index < 0 || index >= getItemCount()) {
-//                clearSelection();
-//                return;
-//            }
-//
-//            final boolean wasSelected = isSelected(index);
-//
-//            // RT-33558 if this method has been called with a given row, and that
-//            // row is the only selected row currently, then this method becomes a no-op.
-//            if (wasSelected && getSelectedIndices().size() == 1) {
-//                // before we return, we double-check that the selected item
-//                // is equal to the item in the given index
-//                if (getSelectedItem() == getModelItem(index)) {
-//                    return;
-//                }
-//            }
-//
-//            List<Integer> removed = bitset.stream().boxed().collect(Collectors.toList());
-//            boolean isSelected = removed.contains(index);
-//            if (isSelected) {
-//                removed.remove((Object)index);
-//            }
-//
-//            if (removed.isEmpty()) {
-//                set(index);
-//            }
-//
-//            bitset.clear();
-//            bitset.set(index);
-//            _beginChange();
-//            if (isSelected) {
-//                _nextRemove(0, removed);
-//            } else {
-//                _nextAdd(0, 1);
-//                _nextRemove(0, removed);
-//            }
-//            _endChange();
-//        }
-
         public boolean isSelected(int index) {
             return bitset.get(index);
         }
@@ -855,7 +829,11 @@ public boolean isNotSelected(int index) {
 
         /** Returns number of true bits in BitSet */
         @Override public int size() {
-            return bitset.cardinality();
+            if (size >= 0) {
+                return size;
+            }
+            size = bitset.cardinality();
+            return size;
         }
 
         /** Returns the number of bits reserved in the BitSet */
@@ -864,8 +842,40 @@ public int bitsetSize() {
         }
 
         @Override public int indexOf(Object obj) {
-            reset();
-            return super.indexOf(obj);
+            if (!(obj instanceof Number)) {
+                return -1;
+            }
+            Number n = (Number) obj;
+            int index = n.intValue();
+            if (!bitset.get(index)) {
+                return -1;
+            }
+
+            // is left most bit
+            if (index == 0) {
+                return 0;
+            }
+
+            // is right most bit
+            if (index == bitset.length() - 1) {
+                return size() - 1;
+            }
+
+            // count right bit
+            if (index > bitset.length() / 2) {
+                int count = 1;
+                for (int i = bitset.nextSetBit(index+1); i >= 0; i = bitset.nextSetBit(i+1)) {
+                    count++;
+                }
+                return size() - count;
+            }
+
+            // count left bit
+            int count = 0;
+            for (int i = bitset.previousSetBit(index-1);  i >= 0; i = bitset.previousSetBit(i-1)) {
+                count++;
+            }
+            return count;
         }
 
         @Override public boolean contains(Object o) {

modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java

@@ -1305,4 +1305,53 @@ public void test_jdk_8088896() {
         assertEquals(2, model.getSelectedItems().size());
         assertEquals(1, counter.get());
     }
+
+    // Test for MultipleSelectionModelBase.SelectedIndicesList#set(int index)
+    @Test public void testSelectedIndicesList_SetMethod() {
+        model.clearSelection();
+        model.setSelectionMode(SelectionMode.MULTIPLE);
+        model.select(1);
+
+        assertTrue(model.isSelected(1));
+    }
+
+    // Test for MultipleSelectionModelBase.SelectedIndicesList#set(int index, int end, boolean isSet)
+    @Test public void testSelectedIndicesList_SetRangeMethod() {
+        model.clearSelection();
+        model.setSelectionMode(SelectionMode.MULTIPLE);
+        model.selectAll();
+
+        assertEquals(data.size(), model.getSelectedItems().size());
+    }
+
+    // Test for MultipleSelectionModelBase.SelectedIndicesList#set(int index, int... indices)
+    @Test public void testSelectedIndicesList_SetIndicesMethod() {
+        model.clearSelection();
+        model.setSelectionMode(SelectionMode.MULTIPLE);
+        model.selectIndices(1, 2, 5);
+
+        assertTrue(model.isSelected(1));
+        assertTrue(model.isSelected(2));
+        assertTrue(model.isSelected(5));
+        assertEquals(3, model.getSelectedIndices().size());
+    }
+
+    // Test for MultipleSelectionModelBase.SelectedIndicesList#clear()
+    @Test public void testSelectedIndicesList_ClearMethod() {
+        model.setSelectionMode(SelectionMode.MULTIPLE);
+        model.selectIndices(1, 2, 5);
+        model.clearSelection();
+
+        assertTrue(model.getSelectedIndices().isEmpty());
+    }
+
+    // Test for MultipleSelectionModelBase.SelectedIndicesList#clear()
+    @Test public void testSelectedIndicesList_ClearIndexMethod() {
+        model.clearSelection();
+        model.setSelectionMode(SelectionMode.MULTIPLE);
+        model.selectIndices(1, 2, 5);
+        model.clearSelection(2);
+
+        assertEquals(2, model.getSelectedIndices().size());
+    }
 }

tests/manual/controls/SelectListViewTest.java

@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import javafx.application.Application;
+import javafx.collections.ObservableList;
+import javafx.scene.Scene;
+import javafx.scene.control.Button;
+import javafx.scene.control.ListView;
+import javafx.scene.control.SelectionMode;
+import javafx.scene.layout.BorderPane;
+import javafx.scene.layout.VBox;
+import javafx.stage.Stage;
+
+public class SelectListViewTest extends Application {
+
+    final int ROW_COUNT = 70_000;
+    //  final int ROW_COUNT = 400_000;
+    //  final int ROW_COUNT = 10_000_000;
+    //  final int ROW_COUNT = 7_000;
+
+    @Override
+    public void start(Stage stage) {
+        ListView<String> listView = new ListView<>();
+        listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
+        ObservableList<String> items = listView.getItems();
+        for(int i = 0; i < ROW_COUNT; i++) {
+            String rec = String.valueOf(i);
+            items.add(rec);
+        }
+
+        BorderPane root = new BorderPane(listView);
+        Button selectAll = new Button("selectAll");
+        Button clearSelection = new Button("clearSelection");
+        Button selectToStart = new Button("selectToStart");
+        Button selectToEnd = new Button("selectToEnd");
+        Button selectPrevious = new Button("selectPrevious");
+        Button selectNext= new Button("selectNext");
+
+        selectAll.setFocusTraversable(true);
+        clearSelection.setFocusTraversable(true);
+        selectToStart.setFocusTraversable(true);
+        selectToEnd.setFocusTraversable(true);
+        selectPrevious.setFocusTraversable(true);
+        selectNext.setFocusTraversable(true);
+
+        root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd, selectPrevious, selectNext, clearSelection));
+        stage.setScene(new Scene(root, 600, 600));
+
+        selectAll.setOnAction(e -> selectAll(listView));
+        clearSelection.setOnAction(e -> clearSelection(listView));
+        selectToStart.setOnAction(e -> selectToStart(listView));
+        selectToEnd.setOnAction(e -> selectToLast(listView));
+        selectPrevious.setOnAction(e -> selectPrevious(listView));
+        selectNext.setOnAction(e -> selectNext(listView));
+
+        stage.show();
+    }
+
+    private void selectAll(ListView listView) {
+        long t = System.currentTimeMillis();
+        listView.getSelectionModel().selectAll();
+        System.out.println("time:" + (System.currentTimeMillis() - t));
+    }
+
+    private void clearSelection(ListView listView) {
+        long t = System.currentTimeMillis();
+        listView.getSelectionModel().clearSelection();
+        System.out.println("time:" + (System.currentTimeMillis() - t));
+    }
+
+    private void selectToStart(ListView listView) {
+        long t = System.currentTimeMillis();
+        listView.getSelectionModel().selectRange(0, listView.getSelectionModel().getSelectedIndex());
+        System.out.println("time:" + (System.currentTimeMillis() - t));
+    }
+
+    private void selectToLast(ListView listView) {
+        long t = System.currentTimeMillis();
+        listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(), listView.getItems().size());
+        System.out.println("time:" + (System.currentTimeMillis() - t));
+    }
+
+    private void selectPrevious(ListView listView) {
+        long t = System.currentTimeMillis();
+        listView.getSelectionModel().selectPrevious();
+        System.out.println("time:" + (System.currentTimeMillis() - t));
+    }
+
+    private void selectNext(ListView listView) {
+        long t = System.currentTimeMillis();
+        listView.getSelectionModel().selectNext();
+        System.out.println("time:" + (System.currentTimeMillis() - t));
+    }
+    public static void main(String[] args) {
+        Application.launch(args);
+    }
+}