RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

[John Hendrikx]

On Fri, 17 Apr 2020 07:22:20 GMT, dannygonzalez <github.com+6702882+dannygonzalez at openjdk.org> wrote:

>> @hjon
>> 
>>> 3. Even though the current version of this pull request takes care to notify duplicate listeners the correct amount of
>>> times, it does not notify them in the correct order with respect to other listeners. If one registers listeners (a, b,
>>> a) the notification order will be (a, a, b).
>> 
>> Unless I'm missing something I don't think this is the case. I used a LinkedHashMap which preserved the order of
>> notifications. Actually some unit tests failed if the notifications weren't carried out in the same order as
>> registration which was the case when I used a HashMap. See here:
>> https://github.com/openjdk/jfx/pull/108#issuecomment-590883183
>
> @hjohn
> 
>> 2. There is a significant increase in memory use. Where before each listener occupied an entry in an array, now each
>> listener is wrapped by Map.Entry (the Integer instance used per entry can be disregarded). I estimate around 4-8x more
>> heap will be consumed (the numbers are small however, still well below 1 MB for 20.000 entries). If this is an issue, a
>> further level could be introduced in the listener implementation hierarchy (singleton -> array -> map).
> 
> There was discussion about lazy initialisation of the LinkedHashMap when needed and/or have some sort of strategy where
> we could use arrays or lists if the number of listeners are less than some threshold (i.e. introducing another level to
> the hierarchy as you mentioned). This was mentioned here also:
> https://github.com/openjdk/jfx/pull/108#issuecomment-590838942

I've implemented an alternative solution: Removing the listeners on `Window.showingProperty` and `Scene.windowProperty`
completely.  They are in fact only used in two places: `PopupWindow` in order to remove itself if the `Window` is no
longer showing, and `ProgressIndicatorSkin`.  These two can be easily replaced with their own listeners for these
properties instead of burdening **all** nodes with these listeners only to support these two classes.

I left the `isTreeShowing` method in, and implemented it simply as `isTreeVisible() && isWindowShowing()` as that's
really the only difference between "visible" and "showing" apparently.

Here is the test result with 20.000 nested StackPanes with only this change in:

- Add all 45 ms
- Remove all 25 ms

I think this might be a good solution as it completely avoids these listeners.

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

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