phetsims / number-line-integers

"Number Line: Integers" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 4 forks source link

FPS drop when dragging thermometer #71

Closed loganbraywork closed 4 years ago

loganbraywork commented 4 years ago

Test device Dirac Operating System MacOs 10.11 Browser Safari Problem description When the thermometer is moved around on the climate map, FPS drops from an average of about 56 to 27. Steps to reproduce

  1. Go to the Explore screen
  2. Switch to the climate map
  3. Pick up a thermometer and move it around the map rapidly Visuals 2019-12-11FPSLagNmbrLne

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Number Line: Integers‬ URL: https://phet-dev.colorado.edu/html/number-line-integers/1.0.0-dev.10/phet/number-line-integers_all_phet.html Version: 1.0.0-dev.10 2019-12-11 16:22:53 UTC Features missing: touch User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.108 Safari/537.36 Language: en-US Window: 1366x625 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: 32 varying: 32 uniform: 256 Texture: size: 8192 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}

jbphet commented 4 years ago

For the purposes of having a repeatable process for this, I'm defining the test as follows:

When I run this test on Chrome on my Win 10 machine, the motion is smooth and the FPS readout varies from roughly 51 fps to 55 fps.

jbphet commented 4 years ago

I profiled this test in Chrome on my Win 10 machine, and it looks like the biggest opportunities for optimization are with the updates to the comparison statement node, at least in this particular case. Below is a screenshot of the flame chart for one frame time.

image

jbphet commented 4 years ago

I just tried the "circle Africa at 40 bpm" test on the machine named "Dirac", which was the machine that was used when this issue was initially reported. While I did see the FPS indicator drop to as low as 33, it mostly stayed in the 40s and didn't seem to affect usability at all as far as I could tell. I'll spend a little time trying to improve this, but it doesn't seem like it's worth a ton of time.

If I drag the thermometer around really quickly for a while, which isn't a particularly realistic use case, I do see the frame rate drop into the 20s.

jbphet commented 4 years ago

I've implemented an optimization where Scenery nodes in the comparison statement that were previously being removed, recreated and re-added are now kept in the scene graph all the time, with their visibility and values updated as needed. This saves the time that was being consumed in the constructors and in the process of adding the nodes to the scene graph. The results in Chrome were fairly dramatic - the methods for updating the comparison statement got so much faster that they often don't even appear in the flame chart at all anymore. Below is a screen shot at roughly the same scale as the previous one where only one of the two calls to updateComparisonStatement is visible, and it is also significantly shorter than it used to be.

image

I re-ran the "circle Africa at 40 bpm" test on Dirac+Safari with a built version that incorporated this change, and it seems somewhat improved, but the difference wasn't huge. The lowest reading I saw was 39 FPS, which is better than the 33 FPS that I saw before, but I'm not sure how repeatable these results are or whether the user would notice a difference of this magnitude. Nevertheless, I think this is a win, and the performance seems reasonable to me at this point.

I'm going to assign this back to @KatieWoe to decide whether this should be tested again when the RC comes out for this sim or whether to just call it it good and close this issue.

KatieWoe commented 4 years ago

I would say this is good. Perhaps a note in the rc to keep an eye on performance when doing impacted tasks, but not a specific issue.

jbphet commented 4 years ago

Sounds good, thanks. Closing.