On Fri, 17 Apr 2020 07:22:20 GMT, dannygonzalez 
<github.com+6702882+dannygonza...@openjdk.org> wrote:

>> @hjon
>> 
>>> 3. Even though the current version of this pull request takes care to 
>>> notify duplicate listeners the correct amount of
>>> times, it does not notify them in the correct order with respect to other 
>>> listeners. If one registers listeners (a, b,
>>> a) the notification order will be (a, a, b).
>> 
>> Unless I'm missing something I don't think this is the case. I used a 
>> LinkedHashMap which preserved the order of
>> notifications. Actually some unit tests failed if the notifications weren't 
>> carried out in the same order as
>> registration which was the case when I used a HashMap. See here:
>> https://github.com/openjdk/jfx/pull/108#issuecomment-590883183
>
> @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

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

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

Reply via email to