On Fri, 17 Apr 2020 16:59:29 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> This is a PoC for performance testing. >> >> It contains commented out code in PopupWindow and ProgressIndicatorSkin and >> two tests are failing because of that. >> >> This code avoids registering two listeners (on Scene and on Window) for each >> and every Node to support the >> aforementioned classes. The complete change should update these two classes >> to add their own listeners instead. > > These listeners exist for a good reason. Removing them will almost certainly > cause regressions in behavior. See > [JDK-8090322](https://bugs.openjdk.java.net/browse/JDK-8090322) as well as > the follow-on fixes (since that was a rather > involved fix in the first place). @kevinrushforth I don't propose to remove them, only to move them to where they are needed. Currently they are part of Node, which means all nodes, whether they need to know the state of the Window or not are registering a listener. However, only PopupWindow and the ProgressIndicatorSkin are registering a listener on this property (other uses are limited to a simple "isTreeShowing" checks which can be determined directly). It is very wasteful to have all nodes register this listener, as there are potentially tens of thousands of nodes. All of these nodes are listening on the exact same two properties (when there is only one Scene and Window), causing huge listener lists there. The listener is not registered in a lazy fashion (ie, only registered if something else is listening to the property downstream, like reactfx would do). This also means that when a Scene change or Window showing change occurs, thousands of nodes will receive a change event, but only 2 types of nodes are actually interested. The other nodes are just doing a lot of house keeping to keep watching the correct property (as there is an indirection from Scene to Window). Therefore my proposal is to have the two cases involved register their own listener on Scene and Window. There will not be any regressions. The two tests that currently fail in this PR are expected to fail as I commented out the listener code in the classes involved, but that can easily be fixed by adding the correct listeners there. I'll update it so you can see all tests will pass. ------------- PR: https://git.openjdk.java.net/jfx/pull/185
