phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Investigate memory leaks for sims in general #890

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

Related to https://github.com/phetsims/joist/issues/889, https://github.com/phetsims/scenery/issues/1494 and https://github.com/phetsims/scenery-phet/issues/769, and also https://github.com/phetsims/geometric-optics/issues/373. There seems to be some real problems with memory recently. Geometric optics can easily get from a starting heap of 67MB to 127 after a few minutes of fuzzing in phet-io brand.

zepumph commented 1 year ago

I noticed that there are 38000 TinyOverrideProperties created after cycling through every single locale in the preferences menu (they are created lazily per locale). That only got me up to 84MB though, which includes creating the preferences dialog.

zepumph commented 1 year ago

Geometric optics phet-io brand is 90MB when you start the sim, and the open the about, keyboard, and preferences dialog. That is the best benchmark for memory testing I believe.

load GO with phet-io brand: 67. add preferences: 74 add keyboard help: 88MB add about: 89.8MB

I believe this has to do greatly with my decision in https://github.com/phetsims/joist/issues/874 because we are creating a new instance of the keyboard dialog per screen. In most cases this isn't necessary. Before we were just reusing the dialog for each in those cases. Hmmm. I think we can save at least 7mb on the creation of the help dialog. I'll take a look.

samreid commented 1 year ago

I feel we have had several recent RCs with no memory leaks. @zepumph can this issue be closed?

samreid commented 1 year ago

Oops wrong issue number on those commits, I'll comment in the commit threads.

zepumph commented 1 year ago

I believe you are correct. At one point I felt like it may be worth some time trying to hunt down smaller memory leaks that may be causing undo trouble in certain, uncommon cases, but that is most likely not worth the time at all. We should wait to find them during memory testing. Ready to close.