On Sun, 24 Jan 2021 09:02:17 GMT, John Hendrikx <jhendr...@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
