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

[John Hendrikx]

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.

Thank you for the test case.  I see that this is indeed another issue, although again one that seems to be triggered by an excessive amount of listeners being added by `Node` to something higher up in the hierarchy.  Although I'm sure this is just a test to show the problem, work arounds would include organizing the rectangles in groups to avoid too many children under a single parent, or using Canvas.

Since the listeners added seem to be internal to `Node` to recursively keep track of disabled/visible state (and are not "optional", in other words, every child always registers them) it might be prudent to avoid this overhead and listener management completely.  This could be achieved by having the parent `Node` pro-actively inform its children about changes without needing listeners, for example by just calling a private method.  This should result in similar performance and would reduce memory overhead per node as there is no need for listener instances.

There is a 3rd property as well involved in the case there are transformations applied which could be given the same treatment (localToSceneTransformProperty).

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

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