Setting onRestore always calls removeEventListener() which allocates a Vector.<Function>

[joshtynjala]

Setting the onRestore property of ConcreteTexture always calls removeEventListener():

public function set onRestore(value:Function):void
{
    Starling.current.removeEventListener(Event.CONTEXT3D_CREATE, onContextCreated);

    if (Starling.handleLostContext && value != null)
    {
        mOnRestore = value;
        Starling.current.addEventListener(Event.CONTEXT3D_CREATE, onContextCreated);
    }
    else mOnRestore = null;
}

This might seem inexpensive to call removeEventListener() when a listener hasn't been added. However, I discovered that removeEventListener() can allocate a new Vector even when the listener hasn't been added. It checks if numListeners is > 0, but not that the listener is currently stored in the Vector.

In a Feathers List, new textures may be created frequently as the List scrolls. All of these allocations will cause garbage collection, which will affect performance negatively.

A quick fix for ConcreteTexture would be to remove the event listener only when required:

public function set onRestore(value:Function):void
{
    if (Starling.handleLostContext && value != null)
    {
        mOnRestore = value;
        Starling.current.addEventListener(Event.CONTEXT3D_CREATE, onContextCreated);
    }
    else if (mOnRestore != null)
    {
        Starling.current.removeEventListener(Event.CONTEXT3D_CREATE, onContextCreated);
        mOnRestore = null;
    }
}

However, I also think that this Vector. allocation in removeEventListener() should be addressed. It's likely to affect performance in other places.

Additionally, using indexOf() may allow you to skip some of the iterations in the for loop. Instead of creating a new empty Vector, you could use slice() to include all of the items before the listener in a copy. I didn't test this code, but it should give you an idea of what I have in mind:

if (numListeners > 0)
{
    // we must not modify the original vector, but work on a copy.
    // (see comment in 'invokeEvent')

    var index:int = listeners.indexOf(listener);
    if (index != -1)
    {
        var restListeners:Vector.<Function> = listeners.slice(0, numListeners);

        for (var i:int=index+1; i<numListeners; ++i)
        {
            restListeners[i-1] = listeners[i];
        }

        mEventListeners[type] = restListeners;
    }
}

[PrimaryFeather]

A great observation, Josh, thanks! I could add your "removeEventListener" optimization just like that, it works flawlessly. Since this now works fast and without any allocations for the given use-case, I think we can leave the "onRestore" method unchanged.

[joshtynjala]

Thank you, Daniel!

[PrimaryFeather]

You're welcome, Josh, as always! :smile: