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 194:

> 193:         private Map<InvalidationListener, Integer> invalidationListeners = new LinkedHashMap<>();
> 194:         private Map<ChangeListener<? super T>, Integer> changeListeners = new LinkedHashMap<>();
> 195:         private T currentValue;

Two comments on this:

1. The old implementation initialized these lazily, so we might want to preserve that behavior. I think it is reasonable since in most cases an observable won't have both types of listeners attached to it.
2. Since we're dealing wither performance optimizations here, we should think about what the `initialCapcity` and `loadFactor` of these maps should be, as it can greatly increase performance when dealing with a large amount of entries.

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

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