phetsims / my-solar-system

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

poor performance in safari with dev tools open #310

Open Nancy-Salpepi opened 10 months ago

Nancy-Salpepi commented 10 months ago

Test device MacBook Air M1 chip

Operating System 14.1.2

Browser Safari 17.1.2

Problem description For https://github.com/phetsims/qa/issues/1008, when dev tools are open in the PhET brand sim or wrappers (ex. Data: colorized, Data:JSON, the standalone sim), the performance in safari is greatly affected. I'm not sure if it something about safari or perhaps settings I have on in my dev tools, but the movement is very choppy. I don't see this with mac + chrome. --I don't see this with the published sim.

Steps to reproduce

  1. Using safari, open the PhET sim and go to either screen
  2. Open Web Inspector under "Develop"
  3. Press Play

Visuals

https://github.com/phetsims/my-solar-system/assets/87318828/60ce9624-1ac6-4fd3-8aed-c6ed4df23238

pixelzoom commented 10 months ago

Reproduced in main with phetmarks URL http://localhost:8080/my-solar-system/my-solar-system_en.html?brand=phet&ea&debugger.

I tried another sim that has animation, gas-properties, and I do not see this problem.

KatieWoe commented 10 months ago

Saw this in Win 11 Chrome with more complicated orbits (Hyperbolic in this case)

pixelzoom commented 10 months ago

I asked in Slack#developer if anyone had any insights into this. It was reproducable, but no insights. @jonathanolson asked "Does it happen for all safari versions?"

@Nancy-Salpepi @KatieWoe Do you have access to any other versions of Safari besides 17.12? One hypothesis is that this may be a new problem in Safari, so it would be helpful to compare with older versions.

pixelzoom commented 10 months ago

Ah... I think I know what this is -- it's the large amount of console output, courtesy of the PhET-iO data stream. @AgustinVallejo is working on decreasing that output, so we'll see if that solves this issue.

pixelzoom commented 10 months ago

But @Nancy-Salpepi said:

... when dev tools are open in the PhET brand sim or wrappers

"PhET brand" would not be impacted by the PhET-iO data stream. And I've reproduced this in PhET brand.

Nancy-Salpepi commented 10 months ago

I tried the PhET brand sim with macOS 10.15.7 and safari 15.6.1 and got the same results.

pixelzoom commented 10 months ago

Thanks @Nancy-Salpepi. Any chance that you'd have a time to identify one or more other sims that have this problem? I tried gas-propertie, but it was fine.

Nancy-Salpepi commented 10 months ago

Sure @pixelzoom. I'll take a look.

pixelzoom commented 10 months ago

Performance changed between these 2 dev versions:

pixelzoom commented 10 months ago

This change was a possible candidate in MySolarSystemModel on 2023-10-31, but reverting had no improvement. @AgustinVallejo is still suspicious of this though.

-    // Number of steps is an arbitrary function of adjustedDT, where bigger adjustedDT results in more steps.
-    const desiredFrameDuration = 1 / 5000; // 5000 fps
-
-    // If dt is too big, we need to split it into smaller steps
-    const numberOfSteps = Math.ceil( dt / desiredFrameDuration );
+    const desiredStepsPerSecond = 300;

Reverting this change in NumericalEngine improves performance significantly. But this change was made before 1.3.0-dev.3 on 2023-10-18, so we don't understand why. And we really don't want to change this back.

-    const iterationCount = 400 / this.bodies.length; // 400 is an arbitrary number of iterations that worked well
+    const iterationCount = 4000 / this.bodies.length; // 4000 is an arbitrary number of iterations that worked well
Nancy-Salpepi commented 10 months ago

Thanks @Nancy-Salpepi. Any chance that you'd have a time to identify one or more other sims that have this problem? I tried gas-properties, but it was fine.

I looked at several sims and I don't see this issue anywhere else. On main I looked at: Greenhouse, Energy Skate Park, Energy Forms and Changes, Masses and Springs, Gravity and Orbits, Center and Variability, and Pendulum Lab.

pixelzoom commented 10 months ago

Thanks @Nancy-Salpepi.

pixelzoom commented 10 months ago

Since this only occurs when dev tools are open, this does not block creation of the 1.3 release branch and proceeding with RC testing. That said, it would be good to understand why such a performance decrease occurs, in case it's symptomatic of something that could bite us later, or impact other sims.

pixelzoom commented 9 months ago

No progress on this. It's not a prerequisite for 1.3 release, and we're ready to create the 1.3 release branch. So labeling as deferred, to be investigated in a future release.