Memory Leak or Intended Recycling

[johnjohn]

I've done some profiling of my game and noticed that many nodes, components and entities linger about.  Specifically any system (i ususally use the ListIteratingSystem) seems to cause associated Entities to stick around even after they're passed to the engine for removal.  As best I can tell, it seems the NodeLists are not popping the associated node out of the list.  I created a test project with which others are welcome to test.  It would be nice to gain insight on this as porting to a mobile application it raises concerns regarding mem-leaks and overall performance.

Am I wrong in the assumption that once an entity is passed to the engine for removal that its components get culled from the associated nodes?  I see that when I remove the entity, the system no longer processes those nodes on the update call but I just can't seem to see that it removes and GCs the objects.

[johnjohn]

Also I am not sure if the sanitation of the entity's components is necessary.  Regardless, it didn't seem to work either way.

[johnjohn]

Sorry for spamming with posts but figured I would jot my thoughts down.  So the issue is partly because the associated Node from the node list is not being purged of the entity.  I changed the following function in the ComponentMatchingFamily around line 144 to this:

private function removeIfMatch( entity:Entity ):void

{

if ( entities[ entity ])

{

var node:Node = entities[ entity ];

delete entities[ entity ];

nodes.remove( node );

if ( node.entity == entity )

node.entity = null;

if ( engine.updating )

{

nodePool.cache( node );

engine.updateComplete.add( releaseNodePoolCache );

}

else

{

nodePool.dispose( node );

}

}

}

It should be noted that also a node does not get purged of it's associated component values either.  I'm not sure if this is by design or not.  I know that a ListIteratingSystem allows us to tap into a node when it's added and removed.  Would it be advised to clean out the node's component props there or should the ComponentMatchingFamily class or another Ash core class be handling this?

[James Wrightson]

I think you are right, using Adobes profiler I think I see it as well :)

[johnjohn]

James thanks for confirming my findings.  I was able to produce a solution for this issue by extending the ComponentMatchingFamily class.  It DID however require that I change a private var to a protected var in ComponentMatchingFamily:

protected var components:Dictionary; //was private

It looks like when the nodes are recycled, their component props are not "sanitized".  This means if you have a component that targets another entity, that entity won't be release for GC if it was removed from the engine.  So I made a sanitize method that does this if need be after the Engine's update cycle is complete.  If you sanitize before, then you get null-pointer errors (NPEs) in your various systems that use those nodes.  Here is my custom ComponentMatchinFamily in case you guys want to use it:

package ds2d.core

{

import ash.core.ComponentMatchingFamily;

import ash.core.Engine;

import ash.core.Node;

import de.polygonal.ds.Itr;

import de.polygonal.ds.LinkedQueue;

public class DS2DComponentMatchingFamily extends ComponentMatchingFamily

{

private var engine:Engine;

public function DS2DComponentMatchingFamily( nodeClass:Class, engine:Engine )

{

super( nodeClass, engine );

this.engine = engine;

nodeList.nodeRemoved.add( delNode );

}

private var q:LinkedQueue = new LinkedQueue();

private function delNode( node:Node ):void

{

q.enqueue( node );

if ( !bListenerAdded )

{

bListenerAdded = true;

engine.updateComplete.add( sanitize );

}

}

private var bListenerAdded:Boolean;

private function sanitize():void

{

var itr:Itr = q.iterator();

while ( itr.hasNext())

sanitizeNode( itr.next() as Node );

q.clear( true );

engine.updateComplete.remove( sanitize );

bListenerAdded = false;

}

private function sanitizeNode( node:Node ):void

{

var componentClass:*;

for ( componentClass in components )

{

// ComponentPool.dispose( node[ components[ componentClass ]]);

node[ components[ componentClass ]] = null;

}

node.entity = null;

}

}

}

[James Wrightson]

This should be added the ash...

[johnjohn]

What are the protocols for making a change to the library?  Because if I were to add this to the core, I would change how the MatchingComponentFamily is instantiated so that it is more flexible for advanced developers.  I'd be happy to make the change or submit it for review if someone kinda gave me guidance on how to do so.

On Friday, May 24, 2013 2:56:33 PM UTC-5, johnjohn wrote:

[Neil Manuell]

Fork from github
create new branch
make changes
commit
send pull request
wait :)

[Neil Manuell]

[johnjohn]

k, will give that a go.  I'm not a GIT person so I will probably screw this up.

[Richard]

Just took a quick look at this. Yes, there's clearly a leak. I think there's a cleaner way to fix it - https://github.com/richardlord/Ash/commit/dfa978fd6059135b301017fa5d1e8f1c5de4673f. I haven't tested it but I think this will fix it. Does anyone have time to test it? Or better still write a unit test for it?

Thanks

Richard

[johnjohn]

Yes much cleaner than my hack.  And yes it appears to work.  Attached is my test app if anyone else wants to give it a go.  Thanks for looking into this Richard.