implement specialized listener map for EventManager

[ieatglu3]

This changes the listenerMap field inside EventManager

packetevents/api/src/main/java/com/github/retrooper/packetevents/event/EventManager.java

Line 51 in 95afd3f

private final Map<PacketListenerPriority, Set<PacketListenerCommon>> listenersMap = new ConcurrentHashMap<>();

to a specialized implementation (ConcurrentListenerMap).

This implementation is a drop in replacement for the old ConcurrentHashMap that was being used with an enum key type (PacketListenerPriority).

The PacketListenerPriority enum is a very limited enum, with only 6 usable entries, and will most likely never amount to anything considered "large". PacketEvents already makes use of its embedded ordinal and values field, so It was used for this implementation aswell.

The specialized implementation uses a single AtomicReferenceArray allocated with a size of PacketListenerPriority#values().length, using PacketListenerPriority#ordinal as the "hash" function of the map. Then CAS operations are used on the array to achieve concurrency.

This implementation does not implement Map or ConcurrentMap, because it is simply a specialized drop-in replacement for what was being used previously.

This implementation provides a few benefits; it is a lock-free architecture, is thread-safe, and is more efficient than any JDK concurrent map implementation for its (!!!) use case (!!!) in the EventManager, as there is minimal indirection with only a single backing array.

[booky10]

Hmm, I'm not too sure whether we want to use this as there were some discussions about just using integer priorities because an enum is pretty limiting

[ieatglu3]

okay cool

[AbhigyaKrishna]

Isn't this just a micro optimization? Effectively we are only doing crud over the map in case of listener registration/un-registration. On an average, a plugin registers like ~10 listeners that too on startup and unregisters them on shutdown, so the overall impact in this optimization is negligible. The EventManager class being a singleton also doesn't support the need of micro memory efficiency.

This change introduces unnecessary complexity imo.

Have you benchmarked this, if its a significant optimization?

[ieatglu3]

Complexity? It's 28 lines of code. I also disregarded the micro-optimization concern due to the fact the map type was already micro-optimized initially; going from a synchronized EnumMap to a ConcurrentHashMap in this case made little sense to begin with. (Also considering the fact that it was wrapper synchronized, and not done manually).

Although, if the general consensus is to move towards integer priorities, then this PR is insignificant.

[retrooper]

If you wish to contribute to PacketEvents, feel free to make a new PR that implements the event manager with integer priorities.

[ieatglu3]

If you wish to contribute to PacketEvents, feel free to make a new PR that implements the event manager with integer priorities.

#1504