On Wed, 20 Jan 2021 08:46:13 GMT, John Hendrikx <jhendr...@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.

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

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

Reply via email to