On Fri, 17 Apr 2020 08:07:08 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> @hjohn >> >>> 2. There is a significant increase in memory use. Where before each >>> listener occupied an entry in an array, now each >>> listener is wrapped by Map.Entry (the Integer instance used per entry can >>> be disregarded). I estimate around 4-8x more >>> heap will be consumed (the numbers are small however, still well below 1 MB >>> for 20.000 entries). If this is an issue, a >>> further level could be introduced in the listener implementation hierarchy >>> (singleton -> array -> map). >> >> There was discussion about lazy initialisation of the LinkedHashMap when >> needed and/or have some sort of strategy where >> we could use arrays or lists if the number of listeners are less than some >> threshold (i.e. introducing another level to >> the hierarchy as you mentioned). This was mentioned here also: >> https://github.com/openjdk/jfx/pull/108#issuecomment-590838942 > > @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. ------------- PR: https://git.openjdk.java.net/jfx/pull/108
