RFR: 8252935: Add treeShowing listener only when needed [v3]

John Hendrikx jhendrikx at openjdk.java.net
Sun Jan 24 16:25:45 UTC 2021

On Sun, 24 Jan 2021 09:02:17 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> I left a few inline comments below.
> Sorry about the force push, merging over a year of changes from master did not seem right to me.  It was only for getting up to date, I will do the other commits normally.

> 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.

I hadn't seen that in practice yet, it looks really nice.

> 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 PR #34 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.

I put a test under `tests/system` using the example you gave.  It creates a Scene with about 16k nodes, and then does 10.000 operations on them.  This runs in around 2-2.5 seconds before the fix and in about 100-200 ms after.  I put the cut-off point somewhere in the middle (800 ms).

I am planning to address the rest of the comments somewhere in the coming week.


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

More information about the openjfx-dev mailing list