phetsims / scenery

Scenery is an HTML5 scene graph.
MIT License
53 stars 12 forks source link

Noisy Heap Comparisons #542

Open samreid opened 8 years ago

samreid commented 8 years ago

When looking for memory leaks by using Chrome heap snapshot comparison, scenery shows a pretty clean delta when nothing is happening in the simulation. However, in CCK Basics when I create a wire node and then put it back in the toolbox immediately (with a heap snapshot right before and after), the heap comparison tool shows hundreds of new objects including new Emitters, SVGGelements, Instance, Fittability, etc:

image

Many of the retained objects show a retaining tree through the through previousFirstDrawable and previousLastDrawable.

image

Is this reference in scenery to improve performance? Can it be disabled temporarily for memory testing? What about the larger issue of reducing scenery noise so that leaks in simulation code can be easily identified?

samreid commented 8 years ago

Marking for developer meeting to make everyone aware of this issue, see if there are quick answers, prioritizing next steps, etc.

samreid commented 8 years ago

@jonathanolson and @samreid will look into this.

samreid commented 8 years ago

If I create multiple new battery nodes then do the same before/after test, then there are not too many new allocations:

image

Perhaps the original issue description was due to code paths being run for the first time. I'll reduce the priority of this issue and continue in https://github.com/phetsims/circuit-construction-kit-basics/issues/71

@jonathanolson this may no longer be an issue, I'll leave open for now and report back if anything else appears to be trouble.

samreid commented 8 years ago

I am still seeing large amounts of spurious scenery internals in the heap views, while I have been working on CCK: Black box study, even if the simulation has been running long enough to have explored all code paths. We should investigate this.

samreid commented 8 years ago

Blast is coming out very clean after running for a few minutes:

image

samreid commented 8 years ago

Blast with fuzzMouse=100 and screens=2 is a bit messier:

image

samreid commented 8 years ago

Almost all of the noise is from NavigationBar.js, when I tried making it unpickable, most of the noise disappeared. Here's what remained:

image

samreid commented 8 years ago

I may work on this more next time I am memory profiling. Before I go, I wanted to comment something about a misunderstanding I had with the heap comparison tool (seems obvious in retrospect). If MyClass has a Vector2 instance which is replaced 1000 times between snapshots, the deltas will show up as +1, -1. That is to say, one Vector2 deleted and one Vector2 allocated. When looking for memory leaks, the net delta (0) is the only relevant number.

samreid commented 8 years ago

@jonathanolson and I discussed this a bit today and agreed it would be nice to collaborate on this when the time is right, with the possibilities of (a) coming up with rules about how to understand and look past the scenery noise or (b) finding ways to get rid of the scenery noise in the first place (if possible).