RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

dannygonzalez github.com+6702882+dannygonzalez at openjdk.java.net
Mon Feb 24 09:17:04 UTC 2020

On Mon, 24 Feb 2020 08:14:02 GMT, dannygonzalez <github.com+6702882+dannygonzalez at openjdk.org> wrote:

>> modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelper.java line 197:
>>> 196:         private int weakChangeListenerGcCount = 2;
>>> 197:         private int weakInvalidationListenerGcCount = 2;
>>> 198: 
>> Why are these set to 2 and why do you need them at all? The previous implementation needed to grow and shrink the array so it had to keep these, but `Map` takes care of this for you.
> I agree, I kept these in as I wasn't sure if there was a need to manually force the garbage collection of weak listeners at the same rate as the original implementation.
> Removing this would make sense to me also.
> Updated my thoughts on this, see my comments below.

As far as I know a LinkedHashMap doesn't automatically remove weak listener entries. The listener maps can be holding weak listeners as well as normal listeners.
So we need to keep the original behaviour from before where a count was checked on every addListener call and the weak references were purged from the array using the original calculation for this count.
Otherwise the map would never garbage collect these weak references.

The initial value of 2 for these counts was just the starting count although this is not necessarily strictly accurate. To be completely accurate then we would have to set the appropriate count in each constructor as follows:

i.e. in the Constructor with 2 InvalidationListeners:
weakChangeListenerGcCount = 0
weakInvalidationListenerGcCount = 2

in the Constructor with 2 ChangeListeners:
weakChangeListenerGcCount = 2
weakInvalidationListenerGcCount = 0

in the Constructor with 1 InvalidationListener and 1 ChangeListener:
weakChangeListenerGcCount = 1
weakInvalidationListenerGcCount = 1

Now, I could have used a WeakHashMap to store the listeners where it would automatically purge weak listeners but this doesn't maintain insertion order. Even though the specification doesn't mandate that listeners should be called back in the order they are registered, the unit tests failed when I didn't maintain order.

I am happy to remove this weak listener purge code (as it would make the code much simpler) but then we wouldn't automatically remove the weak listeners, but this may not be deemed a problem anyway?


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

More information about the openjfx-dev mailing list