Game screen memory leak

[phet-steele]

http://www.colorado.edu/physics/phet/dev/html/graphing-lines/1.2.0-dev.1/graphing-lines_en.html?screens=4&fuzzMouse to isolate just the game screen. Heap snapshot recorded every 1 minute.

Minutes	MB
0	27.7
1	325
2	681
3	1020

(No, I'm not missing decimals. It reached ~1 GB in 3 minutes)

Compared to http://www.colorado.edu/physics/phet/dev/html/graphing-lines/1.2.0-dev.1/graphing-lines_en.html?screens=1,2,3&fuzzMouse to show memory without the game screen. The sim performs fine with the game screen excluded.

Minutes	MB
0	41.0
1	45.1
2	45.1
3	45.1

To bring merit to this memory leak, I've tested the Game screen on an iPad 2 iOS 9.3.5. In this version of graphing-lines, mobile Safari will crash after ~2-3 complete runs of a level (the level in question does not seem to matter much). Throughout the level, there are slight hangs as each next challenge loads, and one large hang before each reward screen.

For the record, here is graphing-slope-intercept. Basically the same results: http://www.colorado.edu/physics/phet/dev/html/graphing-slope-intercept/1.0.0-rc.1/graphing-slope-intercept_en.html?screens=2&fuzzMouse

Minutes	MB
0	34.4
1	377
2	732
3	1081

Seen on macOS 10.12.4 Chrome and iPad 2 iOS 9.3.5. For phetsims/tasks#815 and phetsims/tasks#816.
URL: http://www.colorado.edu/physics/phet/dev/html/graphing-lines/1.2.0-dev.1/graphing-lines_en.html?screens=4&fuzzMouse
Version: 1.2.0-dev.1 2017-04-12 20:14:47 UTC
Features missing: touch
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
Language: en-US
Window: 870x650
Pixel Ratio: 1/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 32 uniform: 1024
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 80)
Max viewport: 16384x16384
OES_texture_float: true
Dependencies JSON: {"assert":{"sha":"a707328c","branch":"HEAD"},"axon":{"sha":"81919d26","branch":"HEAD"},"babel":{"sha":"fdde4356","branch":"master"},"brand":{"sha":"054ca5c3","branch":"HEAD"},"chipper":{"sha":"6218b3c7","branch":"master"},"dot":{"sha":"a0c5770e","branch":"HEAD"},"graphing-lines":{"sha":"032bc04e","branch":"master"},"joist":{"sha":"91516011","branch":"HEAD"},"kite":{"sha":"81166ce9","branch":"HEAD"},"phet-core":{"sha":"c5c6c2a8","branch":"HEAD"},"phetcommon":{"sha":"68053a7d","branch":"HEAD"},"query-string-machine":{"sha":"d8a4ff18","branch":"HEAD"},"scenery":{"sha":"17b5a7e4","branch":"HEAD"},"scenery-phet":{"sha":"8ea5852c","branch":"HEAD"},"sherpa":{"sha":"0e326d54","branch":"HEAD"},"sun":{"sha":"50c6a1d0","branch":"HEAD"},"tandem":{"sha":"18703f2f","branch":"HEAD"},"vegas":{"sha":"3c3561d2","branch":"HEAD"},"vibe":{"sha":"49fdbe7d","branch":"HEAD"}}

[pixelzoom]

@phet-steele Would you mind running the same test with the published version of Graphing Lines, to verify that this is a new problem? (... which I suspect it is, but want to make sure I'm looking in the right place.) https://phet.colorado.edu/en/simulation/graphing-lines

[phet-steele]

@pixelzoom it looks like the current published version (1.1.10) has a memory leak, but not nearly as dramatic:

https://phet.colorado.edu/sims/html/graphing-lines/latest/graphing-lines_en.html?screens=4&fuzzMouse

Minutes	MB
0	26.4
1	71.2
2	115
3	171

[pixelzoom]

Thanks @phet-steele.

[pixelzoom]

I see zero occurrences on dispose in graphing-lines source code. That's fine for the first 3 screens, since things are not dynamically created. But in the Line Game screen, we're dynamically creating the view and wiring it to the model (challenges), so there needs to be some cleanup after we're done with a challenge.

I don't see any cleanup that needs to be handled in graphing-slope-intercept source code, it should all be handled in the code that is reused from graphing-lines.

[pixelzoom]

In the above commit, I added //TODO #78 in the code for 8 cases where we definitely need to unlink from the model when a challenge is no longer needed.

[pixelzoom]

I addressed the obvious leaks in the above commits.

Heap snapshots with ?ea&screens=4&fuzzMouse (Line Game screen, Chrome 58, Mac OS 10.11.6):

Minutes	MB
0	32
1	41
2	62
3	75
4	68
5	85
10	82
15	141
20	183
60	569

Forcing a garbage collection at 60 minutes, then taking an immediate heap snapshot shows no significant drop in memory.

So... Much better, and perhaps acceptable for typical use. But still leaking, and I'm not satisfied yet.

[pixelzoom]

This sim pre-dates PhET standards for implementation and documentation of dispose. So while I'm at it, I'm going to document registration of all observers (link, multilink, addListener, etc) to indicate whether or not de-registration is necessary. This should help the next developer (likely me).

[pixelzoom]

Heap snapshots with ?ea&screens=4&fuzzMouse (Line Game screen, Chrome 58, Mac OS 10.11.6):

Minutes	MB
0	33
1	35
2	36
3	49
4	45
5	46
10	64
15	83
20	99
30	124
45	169
60	181

Forcing a garbage collection at 60 minutes, then taking an immediate heap snapshot shows no significant drop in memory.

So... still leaking.

[pixelzoom]

OK, I think I have the leaks plugged in graphing-lines.

Heap snapshots with ?ea&screens=4&fuzzMouse (Line Game screen, Chrome 58, Mac OS 10.11.6):

Minutes	MB
0	30.4
1	33.2
2	34.2
3	34.3
4	34.5
5	34.6
10	34.8
15	35.0
20	35.5
30	35.2

[pixelzoom]

graphing-slope-intercept also looks good.

Heap snapshots with ?ea&screens=2&fuzzMouse (Line Game screen, Chrome 58, Mac OS 10.11.6):

Minutes	MB
0	28.9
1	32.9
2	33.2
3	33.3
4	33.4
5	33.8
10	33.9

... I didn't feel the need to test at 60 minutes, because graphing-slope-intercept does no memory allocation of it's own, relies totally on graphing-lines.

[pixelzoom]

Given the number of files that I had to touch, I felt obliged to test all features of the sim. I didn't notice anything broken, but I've given how much I've been staring at this sim, it's probable that I missed something.

@phet-steele feel free to verify my memory tests. And please give take a quick look at the entire sim to see if I missed anything.

[phet-steele]

@pixelzoom, a quick look didn't reveal anything obvious, we will of course be more thorough during the next dev test. The memory leak certainly looks plugged. I let the game screen run for a couple hours and it stayed around ~33 MB; the full sim stayed ~64 MB. Great work fixing this one, seemed like a good amount of work 😄! Back to you to close or mark for verification in the next dev version.

[pixelzoom]

Our work here is done. Any collateral damage will be ferreted out by the next dev test.