RFR: 8185886: Improve scrolling performance of TableView and TreeTableView
github.com+7517141+yososs at openjdk.java.net
Thu Apr 16 06:36:54 UTC 2020
On Wed, 15 Apr 2020 07:11:27 GMT, dannygonzalez <github.com+6702882+dannygonzalez at openjdk.org> wrote:
>> @dannygonzalez You mentioned "There are multiple change listeners on a Node for example, so you can imagine with the
>> number of nodes in a TableView this is going to be a problem if the unregistering is slow.".
>> Your changes in this PR seem to be focused on improving performance of unregistering listeners when there are thousands
>> of listeners listening on changes or invalidations of the **same** property. Is this actually the case? Is there a
>> single property in TableView or TreeTableView where such a high amount of listeners are present? If so, I think the
>> problem should be solved there. As it is, I donot think this PR will help unregistration performance of listeners when
>> the listeners are spread out over dozens of properties, and although high in total number, the total listeners per
>> property would be low and their listener lists would be short. Performance of short lists (<50 entries) is almost
>> unbeatable, so it is relevant for this PR to know if there are any properties with long listener lists.
> I haven't quantified exactly where all the listeners are coming from but the Node class for example has various
> listeners on it.
> The following changeset (which added an extra listener on the Node class) impacted TableView performance significantly
> (although it was still bad before this change in my previous tests):
>> commit e21606d3a1b73cd4b44383babc750a4b4721edfd
>> Author: Florian Kirmaier <fk at sandec.de<mailto:fk at sandec.de>>
>> Date: Tue Jan 22 09:08:36 2019 -0800
>> 8216377: Fix memoryleak for initial nodes of Window
>> 8207837: Indeterminate ProgressBar does not animate if content is added after scene is set on window
>> Add or remove the windowShowingChangedListener when the scene changes
> As you can imagine, a TableView with many columns and rows can be made up of many Node classes. The impact of having
> multiple listeners deregistering on the Node when new rows are being added to a TableView can be a significant
> performance hit on the JavaFX thread. I don't have the time or knowledge to investigate why these listeners are all
> needed especially around the Node class. I went directly to the source of the problem which was the linear search to
> deregister each listener. I have been running my proposed fix in our JavaFX application for the past few months and it
> has saved it from being unusable due to the JavaFx thread being swamped.
If the lifespan of a large number of registered listeners is biased,
It seems like the next simple change can improve delete performance without changing the data structure.
* Change the search from the front of the list to the search from the back.
This will reduce the number of long-life listeners matching.
More information about the openjfx-dev