RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

[Nir Lisker]

On Thu, 6 Feb 2020 16:13:33 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

modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelper.java line 283:

> 282:             final Map<InvalidationListener, Integer> curInvalidationList = new LinkedHashMap<>(invalidationListeners);
> 283:             final Map<ChangeListener<? super T>, Integer> curChangeList = new LinkedHashMap<>(changeListeners);
> 284: 

You only need the entry set, so you don't need to copy the map, just the set.

modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelper.java line 285:

> 284: 
> 285:             curInvalidationList.entrySet().forEach(entry -> fireInvalidationListeners(entry));
> 286: 

The lambda can be converted to a method reference.

modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelperBase.java line 64:

> 63: 
> 64:         listeners.entrySet().removeIf(e -> p.test(e.getKey()));
> 65:     }

This can be `listeners.keySet().removeIf(p::test);`.

modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelper.java line 197:

> 196:         private int weakChangeListenerGcCount = 2;
> 197:         private int weakInvalidationListenerGcCount = 2;
> 198: 

Why are these set to 2 and why do you need them at all? The previous implementation needed to grow and shrink the array so it had to keep these, but `Map` takes care of this for you.

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

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