On Mon, 24 Feb 2020 07:39:43 GMT, yosbits
<github.com+7517141+yos...@openjdk.org> wrote:
> If there are many columns, the current TableView will stall scrolling.
> Resolving this performance issue requires column virtualization.
> Virtualization mode is enabled when the row height is fixed by the following
> method.
>
> `tableView.setFixedCellSize(height)`
>
> This proposal includes a fix because the current code does not correctly
> implement column virtualization.
>
> The improvement of this proposal can be seen in the following test program.
>
> ``` Java
> import java.util.Arrays;
> import java.util.Collections;
>
> import javafx.animation.AnimationTimer;
> 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.TableColumn;
> import javafx.scene.control.TableView;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.HBox;
> import javafx.stage.Stage;
>
> public class BigTableViewTest2 extends Application {
> private static final boolean USE_WIDTH_FIXED_SIZE = false;
> private static final boolean USE_HEIGHT_FIXED_SIZE = true;
> // private static final int COL_COUNT=30;
> // private static final int COL_COUNT=300;
> // private static final int COL_COUNT=600;
> private static final int COL_COUNT = 1000;
> private static final int ROW_COUNT = 1000;
>
> @Override
> public void start(final Stage primaryStage) throws Exception {
> final TableView<String[]> tableView = new TableView<>();
>
> // tableView.setTableMenuButtonVisible(true); //too heavy
> if (USE_HEIGHT_FIXED_SIZE) {
> tableView.setFixedCellSize(24);
> }
>
> final ObservableList<TableColumn<String[], ?>> columns =
> tableView.getColumns();
> for (int i = 0; i < COL_COUNT; i++) {
> final TableColumn<String[], String> column = new
> TableColumn<>("Col" + i);
> final int colIndex = i;
> column.setCellValueFactory((cell) -> new
> SimpleStringProperty(cell.getValue()[colIndex]));
> columns.add(column);
> if (USE_WIDTH_FIXED_SIZE) {
> column.setPrefWidth(60);
> column.setMaxWidth(60);
> column.setMinWidth(60);
> }
> }
>
> final Button load = new Button("load");
> load.setOnAction(e -> {
> final ObservableList<String[]> items =
> tableView.getItems();
> items.clear();
> for (int i = 0; i < ROW_COUNT; i++) {
> final String[] rec = new String[COL_COUNT];
> for (int j = 0; j < rec.length; j++) {
> rec[j] = i + ":" + j;
> }
> items.add(rec);
> }
> });
>
> final Button reverse = new Button("reverse columns");
> reverse.setOnAction(e -> {
> final TableColumn<String[], ?>[] itemsArray =
> columns.toArray(new TableColumn[0]);
> Collections.reverse(Arrays.asList(itemsArray));
> tableView.getColumns().clear();
>
> tableView.getColumns().addAll(Arrays.asList(itemsArray));
> });
>
> final Button hide = new Button("hide % 10");
> hide.setOnAction(e -> {
> for (int i = 0, n = columns.size(); i < n; i++) {
> if (i % 10 == 0) {
> columns.get(i).setVisible(false);
> }
> }
> });
>
> final BorderPane root = new BorderPane(tableView);
> root.setTop(new HBox(8, load, reverse, hide));
>
> final Scene scene = new Scene(root, 800, 800);
> primaryStage.setScene(scene);
> primaryStage.show();
> this.prepareTimeline(scene);
> }
>
> public static void main(final String[] args) {
> Application.launch(args);
> }
>
> private void prepareTimeline(final Scene scene) {
> new AnimationTimer() {
> @Override
> public void handle(final long now) {
> final double fps =
> com.sun.javafx.perf.PerformanceTracker.getSceneTracker(scene).getInstantFPS();
> ((Stage) scene.getWindow()).setTitle("FPS:" +
> (int) fps);
> }
> }.start();
> }
> }
>
>
> ``` Java
> import javafx.animation.AnimationTimer;
> import javafx.application.Application;
> import javafx.application.Platform;
> import javafx.beans.property.ReadOnlyStringWrapper;
> import javafx.scene.Scene;
> import javafx.scene.control.TreeItem;
> import javafx.scene.control.TreeTableColumn;
> import javafx.scene.control.TreeTableColumn.CellDataFeatures;
> import javafx.scene.control.TreeTableView;
> import javafx.stage.Stage;
>
> public class BigTreeTableViewTest extends Application {
>
> private static final boolean USE_HEIGHT_FIXED_SIZE = true;
> // private static final int COL_COUNT = 900;
> private static final int COL_COUNT = 800;
> // private static final int COL_COUNT = 600;
> // private static final int COL_COUNT = 500;
> // private static final int COL_COUNT = 400;
> // private static final int COL_COUNT = 300;
> // private static final int COL_COUNT = 200;
> // private static final int COL_COUNT = 100;
>
> public static void main(final String[] args) {
> Application.launch(args);
> }
>
> @Override
> public void start(final Stage stage) {
> final TreeItem<String> root = new TreeItem<>("Root");
> final TreeTableView<String> treeTableView = new
> TreeTableView<>(root);
> if (USE_HEIGHT_FIXED_SIZE) {
> treeTableView.setFixedCellSize(24);
> }
> treeTableView.setPrefWidth(800);
> treeTableView.setPrefHeight(500);
> stage.setWidth(800);
> stage.setHeight(500);
>
> Platform.runLater(() -> {
> for (int i = 0; i < 100; i++) {
> TreeItem<String> child = this.addNodes(root);
> child = this.addNodes(child);
> child = this.addNodes(child);
> child = this.addNodes(child);
> }
> });
>
> final TreeTableColumn<String, String>[] cols = new
> TreeTableColumn[COL_COUNT + 1];
> final TreeTableColumn<String, String> column = new
> TreeTableColumn<>("Column");
> column.setPrefWidth(150);
> column.setCellValueFactory(
> (final CellDataFeatures<String, String> p) ->
> new ReadOnlyStringWrapper(p.getValue().getValue()));
> cols[0] = column;
>
> for (int i = 0; i < COL_COUNT; i++) {
> final TreeTableColumn<String, String> col = new
> TreeTableColumn<>(Integer.toString(i));
> col.setPrefWidth(60);
> col.setCellValueFactory(val -> new
> ReadOnlyStringWrapper(val.getValue().getValue()+":"+val.getTreeTableColumn().getText()));
> cols[i + 1] = col;
> }
> treeTableView.getColumns().addAll(cols);
>
> final Scene scene = new Scene(treeTableView, 800, 500);
> stage.setScene(scene);
> stage.show();
> this.prepareTimeline(scene);
> }
>
> private TreeItem<String> addNodes(final TreeItem<String> parent) {
>
> final TreeItem<String>[] childNodes = new TreeItem[20];
> for (int i = 0; i < childNodes.length; i++) {
> childNodes[i] = new TreeItem<>("N" + i);
> }
> final TreeItem<String> root = new TreeItem<>("dir");
> root.setExpanded(true);
> root.getChildren().addAll(childNodes);
> parent.setExpanded(true);
> parent.getChildren().add(root);
> return root;
> }
>
> private void prepareTimeline(final Scene scene) {
> new AnimationTimer() {
> @Override
> public void handle(final long now) {
> final double fps =
> com.sun.javafx.perf.PerformanceTracker.getSceneTracker(scene).getInstantFPS();
> ((Stage) scene.getWindow()).setTitle("FPS:" +
> (int) fps);
> }
> }.start();
> }
>
> }
I'm signed to OCA and already a contributor.
My name is "Naohiro Yoshimoto"
I have received an approved email on 2019-8-3.
19fabf2eafcb02b519d39a1b0a9dad5b9209db64
* Constructor creates multiple cell nodes, but the
Fixed a wasteful problem of creating all cells and deleting out-of-display
cells because the fixedCellSize attribute was not initialized in the
constructor..
* Reduce the load on ExpressionHelper.removeListener by removing cells outside
of the display range from the back of the scrolling operation.
When setting the cell height to a fixed value with setFixedCellSize(), column
virtualization can improve performance.
As the next step, I think it is possible to try to enable column virtualization
regardless of whether setFixedCellSize() is set or not.
Before doing this step, the direct access validation of the internal structure
of the test code needs to be changed via the public API.
Since there was a problem that the cell was not updated when the window was
maximized, I added a fix to TreeTableViewSkin.java.
Added test code (BigTreeTableViewTest) for TreeTableView to the first post.
@kleopatra
Does the test code I added (BigTreeTableViewTest.java) also have side effects?
When the startup time is measured by eye, the impression changes depending on
the individual difference.
The effect of runLater affects your experience.
However, I succeeded in further improving performance by eliminating another
bottleneck in TreeTableView. Of course, it also includes improvements in
startup time.
I plan to commit at a later date.
Column virtualization causes shortness of breath when scrolling a large stroke
horizontally.
This does not happen when stroking on the scrollbar. Looks like a potential
problem with VirtualFlow.
If you are worried about shortness of breath, the following code will help
reduce the problem.
``` Java
　private static final int OVERLAP_MARGIN = 500;
private static boolean isOverlap(double start, double end, double start2,
double end2){
start = Math.max(start-OVERLAP_MARGIN, start2);
end = Math.min(end+OVERLAP_MARGIN, end2);
return (start<=end2 && end >= start2);
}
Below are the answers to JBS's suggestions.
> The patch below resolves the issue, but it is not likely the correct solution
> (we are trying to determine the table width, but we are getting the width and
> padding on the underlying virtualflow (the width is fine, the padding should
> really come from tableview directly):
You probably mention this part,
At least padding refers to Skinnable(TableRow or TreeTableRow). This is the
same as before (L683). The width of VirtualFlow is determined by the header of
Table.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java#L360-L365
``` Java
final VirtualFlow<?> virtualFlow = getVirtualFlow();
final double scrollX = virtualFlow == null ? 0.0 :
virtualFlow.getHbar().getValue();
final Insets padding = getSkinnable().getPadding();
final double vfWidth = virtualFlow == null ? 0.0:virtualFlow.getWidth();
final double headerWidth = vfWidth - (padding.getLeft() + padding.getRight());
For example, can you assume a case that does not work properly?
This is the answer to JBS's comment.
The BigTreeTableViewTest.java attached to this PR commentary is a modified
version of the JDK-8166956 test code. The original test code is good for
reproducing the problem, but I have decided that it cannot be used as a test
code because the cell values are random and the cell values change randomly
with the close / expand operation.
-------------
PR: https://git.openjdk.java.net/jfx/pull/125
