Improve EventListenerList Performance Bugzilla #255534

[azoitl]

By separating the event listener types into individual lists and using a map the startup performance could be greatly reduced. It is for small number of EditParts (i.e., 2000) already twice as fast as the previous implementation.
Furthermore iterating through the event listeners is simpler and most probably also faster.

The work is based on the patch submitted by Iwan Birrer to the following Bugzilla entry:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=255534

[ptziegler]

Do you also want to add the snippet that was attached to the Bugzilla item? I think it would be nice to have an easy way to compare the performance of the different implementations.

[azoitl]

Do you also want to add the snippet that was attached to the Bugzilla item? I think it would be nice to have an easy way to compare the performance of the different implementations.

I was not sure about the legal implications of adding this one.

[ptziegler]

Do you also want to add the snippet that was attached to the Bugzilla item? I think it would be nice to have an easy way to compare the performance of the different implementations.

I was not sure about the legal implications of adding this one.

Fair point. Though I still believe that a controlled test to measure the performance would help out a lot.

[ptziegler]

Maybe to just summarize the open topics:

• I'm not a big fan of creating a copy whenever the getters are called, as it feels like it just shifts the startup delay. I'm fine with either an ArrayList or a LinkedList.

• If a concurrent list is used, we also need to decide how to handle "null" listeners, which are currently supported, even though I don't see a real use case for them...

[azoitl]

Maybe to just summarize the open topics:

* I'm not a big fan of creating a copy whenever the getters are called, as it feels like it just shifts the startup delay. 

me neither.

I'm fine with either an ArrayList or a LinkedList.

I only had concerns that a LinkedList normally uses at least one additional pointer (next) mostly two (next and prev) so we would have a 200% overhead compared to an ArrayList. But as your measurement proved differently I think we could go with the ConcurrentLinkedDeque or did I miss anything?

* If a concurrent list is used, we also need to decide how to handle "null" listeners, which are currently supported, even though I don't see a real use case for them...

I'm not sure if I get you here. You mean removing the map entry if the list is empty? If yes I would do so.

[ptziegler]

I only had concerns that a LinkedList normally uses at least one additional pointer (next) mostly two (next and prev) so we would have a 200% overhead compared to an ArrayList. But as your measurement proved differently I think we could go with the ConcurrentLinkedDeque or did I miss anything?

The linked list is still larger, I only wondered whether that's by a significant margin. At the end of the day, it's your PR so you're free to choose the approach you prefer. ;D

I'm not sure if I get you here. You mean removing the map entry if the list is empty? If yes I would do so.

That's an oversight on my part. I though that you could call addListener(..., null) without error, but I missed the check at the start of the method.

Tough I still believe that removing the map entry when the list of listeners is empty to be a good idea. Even if it just prevents any hard reference to the classes and thus potentially blocking class loaders from being garbage collected. I experienced a similar issues a few months ago with the platform, which made me cautions whenever I see something related to classes.

[azoitl]

I did some further reading and and some more experiments. The Deque/ConcurrentLinkedDeque is faster then the CopyOnWriteArrayList. Also when I used this approach in 4diac IDE with larger diagrams and many editors (app. 100 editors with large diagrams) i didn't see a major increase in memory usage. So I would go with the Deque/ConcurrentLinkedDeque approach for now.

[ptziegler]

Also looking good, outside of a very small optimization.

[ptziegler]

I did some further reading and and some more experiments. The Deque/ConcurrentLinkedDeque is faster then the CopyOnWriteArrayList. Also when I used this approach in 4diac IDE with larger diagrams and many editors (app. 100 editors with large diagrams) i didn't see a major increase in memory usage. So I would go with the Deque/ConcurrentLinkedDeque approach for now.

That's good to hear! I also apologize for being really picky about this class. 😅 I just always fear that a lot of stuff can easily go wrong with this kind of optimizations.

[azoitl]

I did some further reading and and some more experiments. The Deque/ConcurrentLinkedDeque is faster then the CopyOnWriteArrayList. Also when I used this approach in 4diac IDE with larger diagrams and many editors (app. 100 editors with large diagrams) i didn't see a major increase in memory usage. So I would go with the Deque/ConcurrentLinkedDeque approach for now.

That's good to hear! I also apologize for being really picky about this class. 😅 I just always fear that a lot of stuff can easily go wrong with this kind of optimizations.

I'm very happy that you are here so strict on me. This is a very central class breaking it may have rather large consequences.