search

phetsims / expression-exchange

"Expression Exchange" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 2 forks source link

memory usage seems high #91

Closed jbphet closed 7 years ago

jbphet commented 7 years ago

I was working on checking for memory leaks, and was seeing memory usage under Chrome at about 60-70 MB. I just did a bit more testing and found the usage to now be around 85 MB. The odd thing is that I went and checked a previously released dev version and found its memory usage to be about the same, so I'm wondering if something recently changed in Chrome that causes it to use more memory.

At any rate, I'd like to get a handle on this in order to figure out if I need to reduce the memory usage and, if so, how. I'll start by collecting some data.

jbphet commented 7 years ago

The latest dev release if v1.0.0-dev.23, available at http://www.colorado.edu/physics/phet/dev/html/expression-exchange/1.0.0-dev.23/expression-exchange_en.html. I'm first going to test the memory usage at startup. The procedure is:

  1. Open a new incognito window in Chrome so that we're starting from an unused tab
  2. Open the sim
  3. Go to the "Basics" screen
  4. Using the nav bar, go to each of the other screens
  5. Go to the home screen
  6. Take a heap snapshot

Chrome Version (from chrome://help/): Version 58.0.3029.110 (64-bit)

Two trials on the current master RequireJS version have the following values:

So, it's a bit higher in master+RequireJS, but less than 10% higher, so the high memory usage isn't due to some recent changes.

jbphet commented 7 years ago

I turned off the game screen using the screens query parameter and the memory usage went to 46.8 MB. For a point of comparison, I run Unit Rates since it is a recently released three screen sim and found that it used 42.2 MB. These are in the ballpark of one another, so perhaps Expression Exchange's memory usage isn't all that out of line for this size of sim.

jbphet commented 7 years ago

I just tried Function Builder, which is a recently released four screen sim and found that its memory usage is 78.6 MB (just one trial), so this also makes it seem like Expression Exchange is perhaps within reason in terms of its memory usage.

jbphet commented 7 years ago

Marking for dev meeting. Do we have any guidelines or rules of thumb for memory usage?

pixelzoom commented 7 years ago

Recent testing for Graphing Lines (https://github.com/phetsims/graphing-lines/issues/78#issuecomment-296837755) showed ~35MB for just the "Line Game" screen. So I don't think that these numbers are all that surprising for Expression Exchange or any of the other sims noted above.

samreid commented 7 years ago

Searching all of our repos for "MB" shows several hits that indicate prior memory test results: https://github.com/issues?utf8=%E2%9C%93&q=is%3Aissue+MB+user%3Aphetsims+sort%3Acreated-asc

jbphet commented 7 years ago

Discussed in the 5/18/2017 developer meeting and concluded that as long as it loads and runs on iPad 2, that should be a sufficient test.

jbphet commented 7 years ago

I just did several tests on a 16 GB iPad 2 running iOS 9.3.5 (the PhET device named "Witten"), and the sim loaded most of the time, but there were two instances (out of roughly 8) where Safari crashed. This makes me feel as though we are on the edge for memory usage. @jonathanolson has been assigned to do the code review, and he also offered to look for ways to reduce the number of nodes somewhat. Assigning to him for now, I'll take it back when he's done what he can do.

samreid commented 7 years ago

@jbphet can this be removed from "developer meeting" label?

jbphet commented 7 years ago

Yes (done).

jonathanolson commented 7 years ago

As noted in https://github.com/phetsims/expression-exchange/issues/88, I believe the creation of nodes (particularly images) for every coin term (if I'm reading things correctly) in CoinNodeFactory may be the culprit.

Using DAG patterns I believe should help a lot, as each Scenery Image node creates a DOM Image element, which depending on the browser may take up a sizable amount of memory.

RichText also creates a lot of nodes, which is also an issue for the Area Model simulations that I'm working on. A general improvement may be helpful.

Currently, by creating as many coins as I could on all screens, I was able to bump the number of nodes in the tree past 9000.

@jbphet, collaboration on this issue would probably be best.

jonathanolson commented 7 years ago

Also will plan to check performance with @jbphet when it doesn't crash.

phet-steele commented 7 years ago

@jbphet @jonathanolson, 1.1.0-dev.4 is not crashing on an iPad 2. Each screen was filled with max terms and the game was completed multiple time. That's not to say the sim didn't slow down, however (was still usable).

jbphet commented 7 years ago

Since this now works on iPad 2 and doesn't appear to leak memory, I'm closing this issue.