On Fri, 17 Apr 2020 08:48:35 GMT, dannygonzalez <[email protected]> wrote:
>> @dannygonzalez I added a proof of concept here if you want to play with it: >> https://github.com/openjdk/jfx/pull/185 > > @hjohn Thanks for looking into this. It looks like your changes do improve > the issue with the JavaFX thread being swamped with listener > de-registrations. Looking at the JavaFX thread in VisualVM, the > removeListener call has dropped off the hotspots in the same way it did with > my pull request. > > I wasn't fully confident of making changes to the Node hierarchy to remove > listeners hence why I approached the issue from the other direction i.e. the > obvious bottleneck which was the listener de-registration slowness. > > I do worry however that any changes down the road which add listeners to the > Node hierarchy again without fully understanding the implications would lead > us to the same point we are now where the slowness of listener > de-registrations becomes an issue again. There are no tests that catch this > scenario. > I feel that ideally both solutions are needed but am happy to bow to the more > experienced OpenJFX devs opinions here as I know my changes may be more > fundamental and hence risky. The problem is that there are usually many nodes, but only very few scenes and windows, so registering a listener for each node on a scene or window is pretty bad design (also consider the amount of notifications that a scene change would trigger in such scenarios). As far as I can see, this is the only such listener and only needed for two very limited cases, and its addition in the past may have slipped through the cracks. Adding a performance unit test that specifically checks add/remove performance of nodes may prevent such future regressions. ------------- PR: https://git.openjdk.java.net/jfx/pull/108
