RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

dannygonzalez github.com+6702882+dannygonzalez at openjdk.java.net
Fri Apr 17 08:50:52 UTC 2020


On Fri, 17 Apr 2020 08:07:08 GMT, John Hendrikx <jhendrikx at 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


More information about the openjfx-dev mailing list