search

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.
http://scenerystack.org/
MIT License
9 stars 6 forks source link

Dispose Preferences content #878

Closed zepumph closed 2 years ago

zepumph commented 2 years ago

Over in https://github.com/phetsims/scenery-phet/issues/769 we realized that for PhET-iO, all content in the preferences needs to be disposable. This is mostly because of how now all strings are string Properties. @samreid will take a first pass.

zepumph commented 2 years ago

I hit a couple more of the tabs today for this. Instead of 100 preferences dialogs + disposals taking 240MB (starting from 35 in friction), it is now 70. Nothing else struck me as specific to preferences.

zepumph commented 2 years ago

This morning I loaded Ratio and Proportion and ran a memory snapshot on startup. It was 60MB. Then after creating and disposing the preferences dialog 100 times, it was 80MB. 300 times was 107MB. 500 times was 134MB

This is an obvious correlation, but when I look at the memory comparison, there are no obvious preferences-related items.

zepumph commented 2 years ago

When creating 100 preferences dialogs and disposing them, we used to have 717 Nodes in the heap in RAP on startup. After https://github.com/phetsims/sun/commit/30257d517560364f5f19d88cc039b39beece6fd6, we now only have 322. That came out to 10MB in the heap for my case!

zepumph commented 2 years ago

Ok. After the above commit, creating 100 preferences dialogs (and disposing) in RAP goes from starup of 60.9MB to 64.2.

zepumph commented 2 years ago

Arg! I wasn't testing with the input or locale screen. Currently it goes from 65MB -> 117MB for 100 preferences dialog create/disposes.

zepumph commented 2 years ago

http://localhost:8080/ratio-and-proportion/ratio-and-proportion_en.html?brand=phet&ea&debugger&locales=*&supportsGestureControl=true

zepumph commented 2 years ago

Oh boy, sorry to scare, looks like it was a false memory snapshot by chrome. It is consistently 65.5MB -> 70MB, which is about what I saw without those tabs. I also glanced through the two Panel nodes and didn't see anything scary.

@samreid can you please review and feel free to close.

samreid commented 2 years ago

I read through the commits but did not run any memory tests. But everything seemed good, closing.