RFR: 8252935: Add treeShowing listener only when needed [v3]
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.
More information about the openjfx-dev