RFR: 8252936: Optimize removal of listeners from ExpressionHelper.Generic [v3]

[John Nader]

On Wed, 14 Apr 2021 12:33:23 GMT, dannygonzalez <github.com+6702882+dannygonzalez at openjdk.org> wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8185886
>> 
>> Optimisation to ExpressionHelper.Generic class to use Sets rather than Arrays to improve listener removal speed.
>> 
>> This was due to the removal of listeners in TableView taking up to 50% of CPU time on the JavaFX Application thread when scrolling/adding rows to TableViews.
>> 
>> This may alleviate some of the issues seen here:
>> 
>> TableView has a horrific performance with many columns #409
>> https://github.com/javafxports/openjdk-jfx/issues/409#event-2206515033
>> 
>> JDK-8088394 : Huge memory consumption in TableView with too many columns
>> JDK-8166956: JavaFX TreeTableView slow scroll performance
>> JDK-8185887: TableRowSkinBase fails to correctly virtualise cells in horizontal direction
>> 
>> OpenJFX mailing list thread: TableView slow vertical scrolling with 300+ columns
>> https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-January/024780.html
>
> dannygonzalez has updated the pull request incrementally.

The behavior is the same between openjdk 16.0.2 and 17.0.0.1. The test to recreate the problem adds many nodes (Rectangle) to a parent Group. The process causes each node to add two invalidation listeners (disabledProperty and treeVisibleProperty) to the parent node [here](https://github.com/openjdk/jfx/blob/64aa92631fa5aad9293553e8dd174eab647de2f3/modules/javafx.graphics/src/main/java/javafx/scene/Node.java#L965) using ExpressionHelper.Generic.addListener(). When the children are cleared from the group the inefficient linear search through the listener array and remove occurs, two linear searches per child.

Here is the minimal application I am using to recreate the issue.


public class ClearGroupPerformance extends Application {

    private static final int GAP = 10;
    private static final int SIZE = 40;
    private Group groupBase;

    public static void main(String[] args) {

        launch(args);
    }

    @Override
    public void start(Stage stage) {

        int width = 10000;
        int height = 10000;

        ScrollPane scrollPane = new ScrollPane();
        StackPane root = createGroupRoot(width, height);
        scrollPane.setContent(root);

        Button clearGroupButton = new Button("Clear Group");
        clearGroupButton.onActionProperty().set(e -> {

            long startTime = System.nanoTime();
            ObservableList<Node> children = groupBase.getChildren();
            int totalChildren = children.size();
            children.clear();
            System.out.printf("Clearing %d nodes took %.2fms%n", totalChildren, (System.nanoTime()-startTime)/1_000_000.0);

        });

        BorderPane borderPane = new BorderPane();
        borderPane.setTop(clearGroupButton);
        borderPane.setCenter(scrollPane);

        Scene scene = new Scene(borderPane, 100, 100);
        stage.setScene(scene);
        stage.show();

    }

    private StackPane createGroupRoot(int width, int height) {

        groupBase = new Group();
        groupBase.getChildren().add(new Rectangle(width, height, new Color(0.5, 0.5, 0.5, 1.0)));

        for (int posX = GAP; posX + SIZE + GAP < width; posX += SIZE + GAP) {
            for (int posY = GAP; posY + SIZE + GAP < height; posY += SIZE + GAP) {
                Rectangle rectangle = new Rectangle(posX, posY, SIZE, SIZE);
                rectangle.setFill(Color.GREEN);

                groupBase.getChildren().add(rectangle);
            }
        }

        return new StackPane(groupBase);
    }

}

-------------

PR: https://git.openjdk.java.net/jfx/pull/108