On Mon, 24 Feb 2020 09:14:51 GMT, dannygonzalez <github.com+6702882+dannygonza...@openjdk.org> wrote:
>> 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? > 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. The GC'd weak listeners do need to be purged, but why is the original behavior of the counters still applicable? ------------- PR: https://git.openjdk.java.net/jfx/pull/108
