On Thu, 16 Apr 2020 07:11:58 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> The patch proposed here does not share the case where the listener deletion >> performance becomes a bottleneck. >> >> I think that it is necessary to reproduce it by testing first, but >> >> If you just focus on improving listener removal performance, >> >> If the lifespan of a large number of registered listeners is biased, >> It seems like the next simple change can improve delete performance without >> changing the data structure. >> >> * Change the search from the front of the list to the search from the back. >> >> This will reduce the number of long-life listeners matching. > > Looking at the commit > https://github.com/openjdk/jfx/commit/e21606d3a1b73cd4b44383babc750a4b4721edfd > > it seems that the long listener lists are actually part of the `Scene`'s > `Window` property and the `Window`'s `Showing` property. Each `Node` > registers itself on those and so the listener lists for those properties > would scale with the number of nodes. > > A test case showing this problem would really be great as then the patch can > also be verified to solve the problem, but I suppose it could be reproduced > simply by having a large number of Nodes in a scene. @dannygonzalez could > you give us an idea how many Nodes we're talking about? 1000? 10.000? > > It also means there might be other options, do Nodes really need to add these > listeners and for which functionality and are there alternatives? It would > also be possible to target only these specific properties with an optimized > listener list to reduce the impact of this change. The listeners added by `Node` are apparently internally required for internal properties `TreeShowing` and `TreeVisible`, and are used to take various decisions like whether to play/pause animations. There is also a couple of listeners registering on these properties in turn (in `PopupWindow`, `SwingNode`, `WebView` and `MediaView`). A lot of the checks for visibility / showing could easily be done by using the `Scene` property and checking visibility / showing status from there. No need for so many listeners. The other classes mentioned might register their own listener, instead of having `Node` do it for them (and thus impacting *every* node). Alternatively, `Node` may lazily register the listeners for Scene.Window and Window.Showing only when needed (which from what I can see is for pretty specific classes only, not classes that you'd see a lot in a TableView...) ------------- PR: https://git.openjdk.java.net/jfx/pull/108
