phetsims / plinko-probability

"Plinko Probability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
5 stars 7 forks source link

[Safari 10] Large heap memory #96

Closed phet-steele closed 7 years ago

phet-steele commented 7 years ago

Taking heap snapshots on Safari 10 reveal this sim is pretty large. This does not seem to affect performance, but it does kick on my fan:

Home screen after load After visiting both screens (no interaction)
Chrome 24.9 MB 24.4 MB
Safari 10 55.35 MB 85.18 MB

Inspecting the Safari snapshot (the one that is 55.35 MB), @pixelzoom hypothesized that it looks like there are two canvases that should be different sizes, but Safari is ignoring that. If so, this could be a scenery issue:

screen shot 2016-09-22 at 12 18 21 pm

For phetsims/tasks/issues/702. URL: http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-rc.1/plinko-probability_en.html Version: 1.0.0-rc.1 2016-09-16 19:30:26 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12) AppleWebKit/602.1.50 (KHTML, like Gecko) Version/10.0 Safari/602.1.50 Language: en-us Window: 1920x1036 Pixel Ratio: 2/1 WebGL: WebGL 1.0 (2.1 ATI-1.44.68) GLSL: WebGL GLSL ES 1.0 (1.20) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 32 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 16) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"7d27130a","branch":"HEAD"},"axon":{"sha":"e0192608","branch":"HEAD"},"babel":{"sha":"c041de05","branch":"master"},"brand":{"sha":"f0b1f7da","branch":"HEAD"},"chipper":{"sha":"07058555","branch":"HEAD"},"dot":{"sha":"39436598","branch":"HEAD"},"joist":{"sha":"b513b5ee","branch":"HEAD"},"kite":{"sha":"73302899","branch":"HEAD"},"phet-core":{"sha":"c48bf320","branch":"HEAD"},"phetcommon":{"sha":"83ea84c8","branch":"HEAD"},"plinko-probability":{"sha":"3a105d26","branch":"HEAD"},"query-string-machine":{"sha":"05231e54","branch":"HEAD"},"scenery":{"sha":"e78ee413","branch":"HEAD"},"scenery-phet":{"sha":"f0fc9ae8","branch":"HEAD"},"sherpa":{"sha":"bcc28cd6","branch":"HEAD"},"sun":{"sha":"f2bd9d60","branch":"HEAD"},"tandem":{"sha":"d369b847","branch":"HEAD"},"vibe":{"sha":"b422db9c","branch":"HEAD"}}

pixelzoom commented 7 years ago

The fact that the 2 canvases are the same size (21.69 MB) looks suspicious, since the 2 CanvasNodes used in this sim (BallsNode, PegsNode) have different bounds. @jonathanolson is it possible that Safari 10 is ignoring canvas bounds?

pixelzoom commented 7 years ago

It would be interested to see what the sizes of these canvases is for Safari 9. I haven't figured out how to do this.

phet-steele commented 7 years ago

@pixelzoom I think the heap snapshot feature is new in Safari 10. We'll have to sleuth for a way to see memory in Safari 9. https://developer.apple.com/library/content/releasenotes/General/WhatsNewInSafari/Articles/Safari_10_0.html#//apple_ref/doc/uid/TP40014305-CH11-DontLinkElementID_27

phet-steele commented 7 years ago

@pixelzoom @jonathanolson I'm inclined to think this is not a new problem in Safari 10, we just now have updated dev tools to be able to notice. I compared activity monitors of a sim running in Safari 9 to one running in 10. They had (practically) identical CPU and memory usage. Seemingly this has always been happening without any sort of negative impact reported by our users.

jonathanolson commented 7 years ago

I currently don't have Safari 10 available to test with, but looking through the code and other browsers (Chrome, Safari 9), I'm really not sure what those Canvases from that Safari snapshot could be.

Scenery's two Canvases do appear to be sized properly (presumably around the canvasBounds). Additionally, those Canvases have ids that are like "scenery-canvas544", and definitely would never have id="canvas" like is shown in the screenshot. Additionally I can find no mention of paintRectsCanvas in our code. It seems like those Canvases shown in the heap snapshot are something completely different?

I'm mostly curious what the difference is between heap snapshots before and after visiting the screens.

pixelzoom commented 7 years ago

@jonathanolson said

I'm mostly curious what the difference is between heap snapshots before and after visiting the screens.

@phet-steele Is this something that you can provide?

phet-steele commented 7 years ago

@pixelzoom yes, next time I am at a Mac.

pixelzoom commented 7 years ago

@phet-steele Please take a look at a couple of other PhET sims, and see if there is a similar heap-size issue, and whether these 2 mystery canvases are present. That will tell us whether this is a general issue.

pixelzoom commented 7 years ago

Priority set to "top", since RC test 1.0.0-rc.1 is complete, and this is the only issue that has not been addressed or deferred.

jonathanolson commented 7 years ago

Priority set to "top"

Understood, let me know if there's anything else I can do now. I'm mostly just waiting on the Safari 10 snapshots and deltas.

Just briefly checked, Chrome goes from 12.3MB => 12.6MB and Firefox 159.18 => 159.90MB. Allocations are related to Scenery Instance objects being created, doesn't look like anything concerning or sim-specific.

phet-steele commented 7 years ago

take a look at a couple of other PhET sims, and see if there is a similar heap-size issue, and whether these 2 mystery canvases are present. That will tell us whether this is a general issue.

@pixelzoom it's hard to say what's happening. Last Thursday I would have said that every sim has this problem. Each sim I tried had huge snapshots with 2 duplicate canvases. Today, however, I can't seem to get the same results. Let's see what can be deduced from the below snapshots of Plinko first.

Snapshots 1, 2, & 3 go in the order of Home screen --> Intro Screen --> Lab Screen respectively.

Interesting fact: closing the dev tools after the sim has loaded, then re-opening the dev tools to take snapshots gives very different memory usage: 13.45 MB --> 31.36 --> 49.81 in the same order as above. In each snapshot, there do seem to be duplicate canvases, but not limited to just one pair (look at the last screenshot).

screen shot 2016-09-26 at 10 14 58 am screen shot 2016-09-26 at 10 16 17 am screen shot 2016-09-26 at 10 16 52 am screen shot 2016-09-26 at 10 24 09 am
pixelzoom commented 7 years ago

@ariel-phet would like 2 questions answered before proceeding with the next RC test cycle:

(1) Is there a problem here that's specific to plinko-probability? (2) Is there a general problem here that might affect all sims?

I am not familiar enough with the DOM structure to understand (or speculate on) what @phet-steele is reporting. @phet-steele and @jonathanolson can you please collaborate directly and report on the answers to the above questions?

phet-steele commented 7 years ago

I talked with @jonathanolson. To answer both questions at once, no this is not sim specific. It is a general problem with how Safari (old & new) is allocating the memory at sim start. Without myself really knowing the science behind this issue, the fact of the matter is that this memory is having no negative side effects. Furthermore, it's been happening all along in older Safari versions with no one ever reporting a problem. We just now have the tools to reveal the "problem" (can't take snapshots before Safari 10). Since the sim can run on iPad 2 without it crashing, we are probably fine. I'll let @jonathanolson comment in more detail.

ariel-phet commented 7 years ago

@pixelzoom, since not a blocking issue, lets forge ahead on Plinko

pixelzoom commented 7 years ago

Since this is not sim-specific, @ariel-phet has instructed me not to address it for 1.0.0 release.

Closing this issue, tracking as a general issue in https://github.com/phetsims/scenery/issues/570.