On Wed, 26 Feb 2020 07:37:06 GMT, yosbits <github.com+7517141+yos...@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); > } > } I've run SelectListView and SelectTableView tests and both run fine. As for the SelectTableView test, with 700_000 rows, when there is no selection, pressing SelectToEnd takes around 3.1 seconds, as expected. However, if there is one single row selected, after pressing SelectToEnd, the selection doesn't complete, and I have to abort and quit the app. Instead of: tableView.getSelectionModel().selectRange(tableView.getSelectionModel().getFocusedIndex(), tableView.getItems().size()); this: int focusedIndex = tableView.getSelectionModel().getFocusedIndex(); tableView.getSelectionModel().clearSelection(); tableView.getSelectionModel().selectRange(focusedIndex, tableView.getItems().size()); seems to work and times are again around 3 seconds or less. Maybe you can check why that is happening? And about the test discussion above, I understand that running the included tests as part of the automated test wouldn't make much sense (as these can take too long). However, maybe these could be added as unit tests with a reduced number of item/rows. Instead of measuring performance (selection times would depend on each machine), I'd say you could simply verify that selection works as expected. While most of the changes are just about caching `size()`, the new implementation of `MultipleSelectionModelBase::indexOf` adds some new code that should be tested, as part of the unit tests, again not for performance, but to assert it returns the expected values. ------------- PR: https://git.openjdk.java.net/jfx/pull/127
