Why hasn't EventListenerList been replaced? (Or: what are the pitfalls in replacing it?)

Question

Our legacy app is stuck with a terrible framework (okay, I'll name names, it's Tapestry 4) that involves a ridiculous number of EventListeners (~100,000) for the simplest operations. I'm guessing this is beyond what javax.swing.event.EventListenerList was ever meant to handle, and in this unfortunate use case it's causing us some nasty performance headaches.

I spent a couple of hours whipping up the fairly naive HashMap/ArrayList-based replacement below, and it's massively faster in almost every way:

Add 50,000 listeners:

• EventListenerList > 2 seconds
• EventListenerMap ~ 3.5 milliseconds

Fire event to 50,000 listeners:

• EventListenerList 0.3-0.5 milliseconds
• EventListenerMap 0.4-0.5 milliseconds

Remove 50,000 listeners (one at a time):

• EventListenerList > 2 seconds
• EventListenerMap ~280 milliseconds

Firing might be just a hair slower, but modification is enormously faster. Admittedly, the situation this framework has put us in is pathological, but it still seems like EventListenerList could have been replaced a long time ago. Obviously there are issues with the public API (e.g., it exposes its raw internal state array), but there must be more to it than that. Maybe there's multithreaded cases where EventListenerList is much safer or more performant?

public class EventListenerMap
{

    private final ReadWriteLock lock = new ReentrantReadWriteLock();
    private final Lock readLock = lock.readLock();
    private final Lock writeLock = lock.writeLock();

    private Map<Class, List> llMap = new HashMap<Class, List>();

    public <L extends EventListener> void add ( Class<L> listenerClass, L listener )
    {
        try
        {
            writeLock.lock();
            List<L> list = getListenerList( listenerClass );
            if ( list == null )
            {
                list = new ArrayList<L>();
                llMap.put( listenerClass, list );
            }
            list.add( listener );
        }
        finally
        {
            writeLock.unlock();
        }
    }

    public <L extends EventListener> void remove ( Class<L> listenerClass, L listener )
    {
        try
        {
            writeLock.lock();
            List<L> list = getListenerList( listenerClass );
            if ( list != null )
            {
                list.remove( listener );
            }
        }
        finally
        {
            writeLock.unlock();
        }
    }

    @SuppressWarnings("unchecked")
    public <L extends EventListener> L[] getListeners ( Class<L> listenerClass )
    {
        L[] copy = (L[]) Array.newInstance( listenerClass, 0 );
        try
        {
            readLock.lock();
            List<L> list = getListenerList( listenerClass );
            if ( list != null )
            {
                copy = (L[]) list.toArray( copy );
            }
        }
        finally
        {
            readLock.unlock();
        }
        return copy;
    }

    @SuppressWarnings("unchecked")
    private <L extends EventListener> List<L> getListenerList ( Class<L> listenerClass )
    {
        return (List<L>) llMap.get( listenerClass );
    }
}

Answer 1

It's a question of optimization. Swing's EventListenerList assumes:

• The number of Listeners on a list is very small
• The number of ListenerLists could be very large
• Add/remove events are very infrequent

Given those assumptions, the computational cost of adding and removing items is negligible, but the memory cost of having those lists sitting around could be significant. That's why EventListenerList works by allocating an array that is exactly large enough to hold the Listeners, thus having the smallest possible memory footprint. (As the docs note, it doesn't even allocate anything until the first Listener is added, to make sure you don't waste space if there are no Listeners.) The downside of this is that every time a new element is added, it re-allocates the array and copies all the old elements, giving you astronomical costs when you have too many listeners.

(Actually, it's not quite as memory-efficient as possible; the list is {Type,Listener} pairs, so if it was an array of arrays it would be slightly smaller in some cases.)

As for your solution: HashMaps over-allocate memory to ensure efficient hashing. Likewise, the default ArrayList constructor allocates space for 10 elements and grows in chunks. In your bizarre code-base where there are 100k listeners on each list, this extra memory is a tiny addition to the memory you're using to hold all the listeners.

Under a lighter Listener load, though, your implementation costs 16 pointers for every empty list (the default allocation of HashMap), 26 pointers for an EventListenerMap with one element, and 36 pointers for a map with two elements of different classes. (This is not counting the rest of the HashMap and ArrayList structure size.) For those same cases, EventListenerList costs 0, 2, and 4 pointers respectively.

It seems like a huge improvement for the code you've got.