RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

yosbits 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.
>
> @hjohn
> 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.

-------------

PR: https://git.openjdk.java.net/jfx/pull/108


More information about the openjfx-dev mailing list