RFR: 8197991: Selecting many items in a TableView is very slow
Ajit Ghaisas
aghaisas at openjdk.java.net
Mon Apr 12 08:34:46 UTC 2021
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits <github.com+7517141+yososs at openjdk.org> wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly due to the design problem of the cache of TreeUtil.getTreeItem ().
>
> Reference value of the number of data that can be handled within 3 seconds of processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> ``` SelectListViewTest.java
> 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);
> }
> }
>
>
> ``` SelectTableViewTest.java
> import javafx.application.Application;
> import javafx.beans.property.SimpleStringProperty;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.control.TableColumn;
> import javafx.scene.control.TableView;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectTableViewTest extends Application {
>
> final int ROW_COUNT = 700_000;
> // final int ROW_COUNT = 80_000;
> // final int ROW_COUNT = 50_000;
> // final int ROW_COUNT = 8_000;
> final int COL_COUNT = 3;
>
> @Override
> public void start(Stage stage) {
> TableView<String[]> tableView = new TableView<>();
> tableView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
> // tableView.getSelectionModel().setSelectionMode(SelectionMode.SINGLE);
>
> final ObservableList<TableColumn<String[], ?>> columns = tableView.getColumns();
> for(int i=0; i<COL_COUNT; i++) {
> TableColumn<String[], String> column = new TableColumn<>("Col"+i);
> final int colIndex=i;
> column.setCellValueFactory((cell)->new SimpleStringProperty(cell.getValue()[colIndex]));
> column.setPrefWidth(150);
> columns.add(column);
> }
>
> ObservableList<String[]> items = tableView.getItems();
> for(int i=0; i<ROW_COUNT; i++) {
> String[] rec = new String[COL_COUNT];
> for(int j=0; j<rec.length; j++) {
> rec[j] = i+":"+j;
> }
> items.add(rec);
> }
> BorderPane root = new BorderPane(tableView);
> 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(tableView));
> clearSelection.setOnAction((e)->clearSelection(tableView));
> selectToStart.setOnAction((e)->selectToStart(tableView));
> selectToEnd.setOnAction((e)->selectToLast(tableView));
> selectPrevious.setOnAction((e)->selectPrevious(tableView));
> selectNext.setOnAction((e)->selectNext(tableView));
>
> stage.show();
> }
>
> private void selectAll(TableView tableView) {
> long t = System.currentTimeMillis();
> tableView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(TableView tableView) {
> long t = System.currentTimeMillis();
> tableView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(TableView tableView) {
> long t = System.currentTimeMillis();
> tableView.getSelectionModel().selectRange(0, tableView.getSelectionModel().getFocusedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(TableView tableView) {
> long t = System.currentTimeMillis();
> tableView.getSelectionModel().selectRange(tableView.getSelectionModel().getFocusedIndex(), tableView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(TableView tableView) {
> long t = System.currentTimeMillis();
> tableView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(TableView tableView) {
> long t = System.currentTimeMillis();
> tableView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
This is a very good attempt to improve selection performance. I have few review comments.
I ran the manual test that you have provided. It does show the improvement.
Can you please provide an automated test along with your fix?
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 861:
> 859: }
> 860:
> 861: /** Returns number of true bits in BitSet */
Method description and work done by it is no more matching. Can you please update the comment?
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 881:
> 879: Number n = (Number) obj;
> 880: int index = n.intValue();
> 881: if(!bitset.get(index)) {
Spacing correction needed.
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 885:
> 883: }
> 884: // is left most bit
> 885: if(index==0) {
Spacing correction needed.
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 890:
> 888:
> 889: // is right most bit
> 890: if( index == bitset.length()-1 ){
Spacing correction needed.
Correct spacing is : "if (index == bitset.length()-1) {"
Please correct at other places in this PR.
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 895:
> 893:
> 894: // count right bit
> 895: if( index > bitset.length()/2 ) {
Spacing correction needed.
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 899:
> 897: for (;;) {
> 898: index = bitset.nextSetBit(index + 1);
> 899: if (index < 0) {
As we are checking for nextSetBit, shouldn't we be checking for overflow rather than underflow?
Refer - [javadoc](https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/util/BitSet.html#nextSetBit(int))
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java line 912:
> 910: for(;;){
> 911: index = bitset.previousSetBit(index-1);
> 912: if(index<0){
Spacing correction needed.
-------------
PR: https://git.openjdk.java.net/jfx/pull/127
More information about the openjfx-dev
mailing list