RFR: 8252935: Add treeShowing listener only when needed
jhendrikx at openjdk.java.net
Wed Sep 9 21:35:57 UTC 2020
On Fri, 17 Apr 2020 16:59:29 GMT, Kevin Rushforth <kcr at 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.
More information about the openjfx-dev