Skip to content
This repository has been archived by the owner on Jun 24, 2021. It is now read-only.

TableView has a horrific performance with many columns #409

Open
Folling opened this issue Mar 15, 2019 · 15 comments
Open

TableView has a horrific performance with many columns #409

Folling opened this issue Mar 15, 2019 · 15 comments

Comments

@Folling
Copy link

Folling commented Mar 15, 2019

Having a TableView with > 60 TableColumns will start being unreasonably slow.
I'm at a point where creating a single item with 50 Columns can take upwards of 0.5s.
You can easily test this, using just a normal TableView with 100 columns doing nothing but having the cellFactory:

tableView.setCellValueFactory((cellData) -> {
    return new SimpleStringProperty("Hey!");
})

Now of course in real production code this will be significantly more complex so it already starts lagging for me at ~50 columns.
Using a cellFactory instead does improve the performance - using a cache to cache the values even further improves the performance, however this isn't acceptable.
I can have 10 columns and add 1000 items without any lag, but try having 1000 columns and adding 10 items - you'll be able to make half a coffee in that time.

For anyone willing to test, here's a MVCE, written in groovy - it shouldn't be hard to translate:

Controller.groovy

package sample

import javafx.beans.property.SimpleStringProperty;
import javafx.collections.FXCollections
import javafx.fxml.FXML;
import javafx.fxml.Initializable;
import javafx.scene.control.ListView
import javafx.scene.control.TableCell
import javafx.scene.control.TableColumn
import javafx.scene.control.TableView
import java.util.concurrent.atomic.AtomicInteger;


public class Controller implements Initializable {

    @FXML
    TableView<String> tableView

    @Override
    public void initialize(URL url, ResourceBundle resourceBundle) {
        1000.times {
            def column = new TableColumn<>(it.toString())
            column.setCellFactory({
                return new TableCell<String, String>(){
                    @Override
                    protected void updateItem(String value, boolean empty) {
                        super.updateItem(value, empty)

                        if(empty) {
                            setText("")
                            return
                        }
                        setText("azt")
                    }
                }
            })
            tableView.columns.add(column)
        }
    }

    @FXML
    public void addItem() {
        1000.times {
            tableView.items.add("Hah!")
        }
    }
}

sample.fxml

<?import javafx.scene.control.Button?>
<?import javafx.scene.control.TableView?>
<?import javafx.scene.layout.VBox?>
<VBox fx:controller="sample.Controller"
      xmlns:fx="http://javafx.com/fxml" alignment="center">
    <Button onAction="#addItem"/>
    <TableView fx:id="tableView" VBox.vgrow="ALWAYS">
    </TableView>
</VBox>

Main.groovy

package sample;

import javafx.application.Application;
import javafx.fxml.FXMLLoader;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.stage.Stage;

public class Main extends Application {

    @Override
    public void start(Stage primaryStage) throws Exception{
        Parent root = FXMLLoader.load(getClass().getResource("sample.fxml"));
        primaryStage.setTitle("Hello World");
        Scene scene = new Scene(root, 1280, 720)
        primaryStage.setScene(scene);
        primaryStage.maximized = true
        primaryStage.show();
    }


    public static void main(String[] args) {
        launch(Main,args);
    }
}

Simply switch the .times closures in Controller.groovy, i.e. add 10 items but 1000 columns, and you'll see the result.

A few hints perhaps:
The cellValueFactory is called multiple times for each cell, the same goes for updateItem when using custom Cells.
With 50 columns, having just the plain Text rendering, javaFX allocates upwards of 30 MB of ram for each item created, now this memory is quickly freed, however that's still between 0.5 and 1.5 MB per TableCell, which shouldn't even be possible.

@Folling Folling changed the title TableView has a horrific performance with multiple columns TableView has a horrific performance with many columns Mar 15, 2019
@sghpjuikit
Copy link

From my experience JavaFX tables are quite memory hungry. I have yet to notice any performance problems though. Biggest table I'm usually using has about 10 columns and up to 50k items and 50 visible rows (thus 500 cells), with plentiful custom styling, as well as sorting/filtering support using SortedList(FilteredListbackingList)).

I just tried to set visible about 40-50 columns and changed all items on the table and I am not sure I observed severe slowdown like you. But my testing was not rigorous enough to rule it out.

I would try to profile the app to see where most of the time is spent.

@dlemmermann
Copy link

dlemmermann commented Mar 28, 2019 via email

@Folling
Copy link
Author

Folling commented Mar 28, 2019

@sghpjuikit Well the slowdown can be seen in the little example I provided, which shouldn't cause a slowdown. It might have to do with KDE? I'm using Archlinux and I've already noticed three different bugs (some of which are reported here already) which are only present on KDE on Archlinux - usually to do with a poor GTK3 integration.

@dlemmermann I'm well aware of this fact, however that is imo both a fairly senseless decision (why not lazily load table columns too?) and inexcusable.
There should not be a significant slowdown with 1000 cells, lazily loaded or not. Though 100 columns and 10 cells do cause such a slowdown.

@dlemmermann
Copy link

dlemmermann commented Mar 28, 2019 via email

@Folling
Copy link
Author

Folling commented Mar 29, 2019

I have to say that whilst I would love to accept such a challenge I'm currently far too busy to dive into such low-level characteristics.
Perhaps if I find time after this crunch phase.
For anyone curious: The current way to circumnavigate the issue is to have a "paginated" tableview, where you at most display N columns and the user can switch between them in chunks. Which I actually feel is better UX wise than having a couple dozen columns either way.

@sghpjuikit
Copy link

@dlemmermann There already is GridView in ControlsFX and I have made my custom 2D virtualized control (which I dont think is worth contributing due to messy and questionable implementation). 2D virtualized table could perhaps be built on top of the GridView.

@Folling I do not think it should be expected for table columns to be lazily loaded because of the complexity of the implementation.

@Folling
Copy link
Author

Folling commented Mar 31, 2019

@sghpjuikit Hiding behind "it's too complex to implement" is a terrible excuse imo. It shouldn't be that hard tbh, though perhaps it is difficult to switch the current implementation to contain lazily loaded columns

@sghpjuikit
Copy link

It is not an excuse. Table and spreadsheet have different use cases and there is an expectation that table will not have so many columns as to need virtualization. I.e. it is too complex for the benefit. Table with 50 columns is unwieldy and may suggests UX problems, best battled with a customized control.

@Maxoudela
Copy link
Contributor

I do agree with @sghpjuikit (what a username you have there!). TableView was initially made for displaying SQL tables. Usually, you do not have many columns but you may have plenty of rows.

If you are working with a Spreadsheet, you may indeed have lots of columns. You can take a look at the SpreadsheetView code of ControlsFX here : https://github.com/controlsfx/controlsfx/blob/master/controlsfx/src/main/java/impl/org/controlsfx/spreadsheet/GridRowSkin.java#L111
I have ran into this issue and "virtualized" (although not really) the columns. The virtualization could be improved of course.

@baumeister
Copy link

baumeister commented May 19, 2019

There are very good use cases for having many columns and it's not a UX problem.

TableView has significant performance issues which really should be addressed in the core business UI control. SpreadsheetView from controlsfx has even greater performance/memory issues. The only way out right now is to use JTable which works well regardless of column/row count.

@sghpjuikit
Copy link

Well, it is not like I like the current situation either (in my application, tables are way too heavy for my liking), but even worse, I actually do not understand the exact cause/problem.

So what is the issue with TableView? Lack of horizontal virtualization? Memory consumption? Slowness? Has anyone identified what exactly is causing any of these problems?

For instance horizontal virtualization can only help you so far - fullscreen application in 4k window with 100% dpi scaling, displaying one table with lots of narrow columns will not take advantage of that at all. I.e., when user wants to see lots of data, the table will need to display lot of data. Id like to know @baumeister how could TableView be made better here?

@yososs
Copy link

yososs commented May 20, 2019

The problem is that the UI design task is limited by the number of columns from 100 to 300 or less.
Although this is whether the designer of the actual application which is not a benchmark feels inconvenient, it is thought that there are many who feel that it is not inconvenient.
However, I agree that solving this problem is necessary to promote JavaFX technology as the next generation technology.

The following is a minimal test to profile.

import javafx.application.Application;
import javafx.beans.property.SimpleStringProperty;
import javafx.collections.ObservableList;
import javafx.scene.Scene;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;
import javafx.scene.layout.BorderPane;
import javafx.stage.Stage;

public class BigTableViewTest extends Application {

	private static final boolean USE_FIXED_SIZE = true;
	private static final int COL_COUNT=500;
	private static final int ROW_COUNT=1000;
	
	@Override
	public void start(Stage primaryStage) throws Exception {
	    final TableView<String[]> tableView = new TableView<>();
        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]));
			columns.add(column);
			if(USE_FIXED_SIZE) {
				column.setMaxWidth(55);
				column.setPrefWidth(55);
				column.setMinWidth(55);
			}
    	}
    	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);
	if(USE_FIXED_SIZE) {
		tableView.setFixedCellSize(24);
	}

    	
    	Scene scene = new Scene(root, 800, 800);
    	primaryStage.setScene(scene);
    	primaryStage.show();
	}
	
	public static void main(String[] args) {
		Application.launch(args);
	}
}

As of JDK 8, there is a workaround to reduce the performance impact of this issue. (Not usable with JDK 9 or later)It looks like the effect of simplifying the implementation with JDK 9 to fix the problem of fixing the cell size with JDK 8.

By starting with the next option,
It is possible to confirm that the increase in rendering nodes can be suppressed if the cell size is specified by setFixedCellSize().

  -Djavafx.pulseLogger = true

However, changing the window size causes another problem in which some cells are not rendered.

@yososs
Copy link

yososs commented Sep 12, 2019

This problem was found to be caused by incorrect determination of isColumnPartiallyOrFullyVisible. This is the root cause of this problem.

It can be considered as a method that determines the overlap between the display range of the cell and scroll. headerWidth should be the scrolling display range, but is the maximum scrolling width. As a result, at the horizontal minimum scroll position, it is determined that all the cells in all the display rows are displayed, and the vehicle stalls. The judgment is appropriate at the maximum scroll position, and the influence of the problem is reduced.

private boolean isColumnPartiallyOrFullyVisible(TableColumnBase col) {
if (col == null || !col.isVisible()) return false;
final VirtualFlow<?> virtualFlow = getVirtualFlow();
double scrollX = virtualFlow == null ? 0.0 : virtualFlow.getHbar().getValue();
// work out where this column header is, and it's width (start -> end)
double start = 0;
final ObservableList<? extends TableColumnBase> visibleLeafColumns = getVisibleLeafColumns();
for (int i = 0, max = visibleLeafColumns.size(); i < max; i++) {
TableColumnBase<?,?> c = visibleLeafColumns.get(i);
if (c.equals(col)) break;
start += c.getWidth();
}
double end = start + col.getWidth();
// determine the width of the table
final Insets padding = getSkinnable().getPadding();
double headerWidth = getSkinnable().getWidth() - padding.getLeft() + padding.getRight();
return (start >= scrollX || end > scrollX) && (start < (headerWidth + scrollX) || end <= (headerWidth + scrollX));
}

@kevinrushforth
Copy link
Collaborator

This GitHub issue tracker is not actively tracked or managed and will stop being looked at entirely once we transition the official openjfx repo to https://github.com/openjdk/jfx. Please file a bug with a complete standalone testcase at bugreport.java.com.

@yososs
Copy link

yososs commented Feb 26, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants