-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Startup time is long #117
Comments
This sim is on hold for a while, so I'm unassigning this issue until work resumes. |
I just built version 1.0.0-dev.14 (available here: http://www.colorado.edu/physics/phet/dev/html/arithmetic/1.0.0-dev.14/arithmetic_en.html) and measured the startup time on several devices and browsers. It seems to have gotten a bit better, but it still quite long on iPads. My test on each device was to open a new tab, load the sim, then reload it twice, recording the elapsed time from click (or touch) to appearance of the home screen. The collected data is shown in the table below.
The load time seems pretty decent for all tested cases except for the two iPads. If I can reduce this load time significantly without negatively impacting the other platforms, we should be good to go. |
…nimator so that it doesn't have to copy the whole object for the level selection and workspace nodes, see #117
…onts instead of recreating a font for each cell, see #117
Note, mostly to myself: I think the reason that startup time seemed shorter when I started measuring it is that mipmaps are computed on load time in RequireJS mode, but are pre-computed for built versions. For sims (like this one) that use a number of mipmaps, RequireJS load times are quite a bit longer than load times for built sims. |
…cell only be created when needed, see #117
…racCell be a zero length string to save on bounds computation time, see #117
I tested with on the two iPads listed in the table above after making the optimizations reflected in the commits above. The result was that Tycho was down to 10.66 seconds on average and Disessa was at 10.23 seconds. This is a substantial improvement - over 30% - but still not enough. Profiling on Chrome+Win7 shows that the creation of all the text nodes in the cells of the multiplication tables is requiring a substantial amount of time. I'm going to try creating them lazily instead of eagerly. |
…see phetsims/arithmetic#117. On several trials on OSX, this reduced the creation time to about 1/4 of what it had been. This was tested on Arithmetic, Balancing Chemical Equations, Fraction Matcher, and Making Tens and no ill effects were observed.
…leNode in favor of basic layout, see #117. Also did some code cleanup.
…single pointer hand in the MultiplicationTableFactorNode, see #117
… use in the cells, part of the optimization cleanup, see #117
A lot of optimization has been done, and I'm rerunning the startup time tests to see where we are at. Note that the iPad 2 was upgraded from iOS 9.0 to 9.1 since the first round of testing was performed. The version being tested is 1.0.0-dev.15, and can be found at http://www.colorado.edu/physics/phet/dev/html/arithmetic/1.0.0-dev.15/. The time values were obtained by using a stopwatch app on an iPhone, so they aren't super accurate. This is the same method that was used for the previous data set.
|
For comparison, I just ran the same load test using the recently released Hooke's Law simulation (v1.0.0) on the iPad 2 "Tycho" and got an average of 7.50 seconds, so this sim is very close. The optimizations have reduced the load times on iPad to around 1/2 of what they were in v 1.0.0-dev.14. I had @ariel-phet do a subjective test on 1.0.0-dev.15, and he felt like the load time was acceptable. @jonathanolson said that he might look at making some changes in Scenery that would improve load times, since this sim creates a lot of nodes and thus provides a good opportunity to optimize the per-node creation time. I'm assigning the issue to him for now. If he doesn't have time or is unable to find any good optimizations, this issue will be closed since the load time is now in the acceptable range. |
Initial ~200ms for preloads, ~200ms more for loading require.js modules (and executing them), ~1000ms (1s) for sim object load, ~300ms for Scenery initial display. I have a ton of notes on potential performance improvements, so I'll be looking at the big offenders (text bounds and stroked shape bounds) and the low-hanging fruit. |
@jbphet, a small optimization would be further reducing the number of fonts that the sim uses. It's not terribly significant, so don't worry about it too much. Current list:
|
onStatic gets called 45290 times on a normal startup, so a Scenery pattern like phetsims/scenery#490 or possibly even an instance-notification pattern would work. |
After the above optimizations, Node's constructor's SELF time is 116ms on my Win7/Chrome. Might be creating many hidden-class chains? |
I've started to hit diminishing returns. Further investigation would probably be more into how our inheritance pattern might affect hidden classes, and the amount/quality of JS optimization. Many types of AbstractCell are created (and many of them). It does cause a disabled-optimization on getFillRendererBitmask (along with many deoptimizations). It may be possible to restructure some UI so that we don't display all of the rectangles (but instead one larger rectangle BEHIND grid lines), etc., and that may reduce the Node count significantly (and help with performance). Current testing: If that's accurate, it's close to 2 seconds off of the previous iPad 3 numbers. Assigning back to @jbphet for testing. Let me know if you want to work further on improvements. |
@jbphet if the above numbers hold true (~7 seconds on iPad 3) no need for further optimization for sure. |
I just did three trials on an iPad 3 (Disessa) running iOS 9.2, and all trials were less than 8 seconds, average at 7.23 seconds. Closing. |
The load time for this sim strikes me as excessively long for as simple as it is. This should be profiled and examined and, if possible, the load time should be reduced.
The text was updated successfully, but these errors were encountered: