RFR: 8252935: Add treeShowing listener only when needed
kcr at openjdk.java.net
Fri Jan 22 21:28:42 UTC 2021
On Wed, 20 Jan 2021 08:46:13 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> @hjohn Per [this message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-September/027534.html) on the openjfx-dev mailing list, I have filed a new JBS issue for this PR to use. Please change the title to:
>> 8252935: Add treeShowing listener only when needed
> So, will this actually get reviewed?
Now that we are past RDP1 for JavaFX 16, this seems a good time to look at some of these performance issue. One of the challenges will be ensuring no regressions.
I did a preliminary review of this, along with some testing, and it looks less scary than I was afraid it would be. The overall approach seems sound: preserve the `NodeHelper::isTreeVisible` method, but don't provide it as an observable property, instead adding the listeners only to those that need them.
Can you do the following?
1. Merge in the latest changes from the upstream master branch into your local feature branch? Among other things, this will enable running tests via GitHub Actions.
2. Add a test program under `tests/performance` that can be used to verify the performance gain. Even better would be if you can create an automated test under `tests/system`. I was thinking something like what we did for [JDK](URL) where there was a pathological performance bug fixed and the test checked that the operation in question didn't take more than some small number of seconds. That might be overkill for this fix, though.
More information about the openjfx-dev