Memory test - Githubissues

phetsims / build-a-nucleus

"Build a Nucleus" is an educational simulation in HTML5, by PhET Interactive Simulations.

GNU General Public License v3.0

0 stars 5 forks source link

Memory test #119

Closed Luisav1 closed 1 year ago

Luisav1 commented 1 year ago

From self code review https://github.com/phetsims/build-a-nucleus/issues/112.

Luisav1 commented 1 year ago

On-hold because of CT issue #129.

zepumph commented 1 year ago

https://github.com/phetsims/build-a-nucleus/issues/129 is closed. let's do it!

jbphet commented 1 year ago

As part of the code review in https://github.com/phetsims/build-a-nucleus/issues/165 I ran a memory test. There is a screenshot of the results below. The bottom line is that there may be a slow leak, but nothing egregious, and what we do see could be explainable by pooling in Scenery.

Everything was done in a Chrome incognito window using a freshly built version of build-a-nucleus_all_phet_debug.html.

Explanation:

Snapshot 1 - loaded, no interaction
Snapshot 2 - after a minute of basic interaction, then a reset on both screens and a return to the home screen
Snapshot 3 - after a reload with the fuzz parameter added and one minute of fuzzing
Snapshots 4 thru 12 - snapshots at one minute intervals while fuzzing
Snapshot 13 - Stopped fuzzing using phet.chipper.queryParameter.fuzz = false, reset both screens, went to the home screen

zepumph commented 1 year ago

When I create 10 of each nucleon and then reset, the comparison memory tool doesn't show any obvious memory leaks. Yay!

zepumph commented 1 year ago

We found one problem with how we were recreating a Text, PatternStringProperty, AND a DerivedProperty each time the mass number changed. Oops. Fixed here.

zepumph commented 1 year ago

After fuzzing for 45 minutes and capturing snapshots intermittently, I don't see problems anymore. Woo! Closing. Thanks @Luisav1 for all your help.

zepumph commented 1 year ago

Arg! Found another nested listener adding:

https://github.com/phetsims/build-a-nucleus/blob/1475dedc07ee8c6227610432c6faa485ea105afb/js/common/view/NucleonNumberPanel.ts#L110-L121

zepumph commented 1 year ago

?listenerLimit=1000 has been helpful for this nesting issue

zepumph commented 1 year ago

After fuzzing for a while I saw that the maxListenerCount was 783, then when I restarted the sim with ?listenerLimit=782, it failed on startup. No further work to be done here.