Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Leak #127

Closed
KatieWoe opened this issue Jan 28, 2022 · 5 comments
Closed

Memory Leak #127

KatieWoe opened this issue Jan 28, 2022 · 5 comments

Comments

@KatieWoe
Copy link

For phetsims/qa#771. This seems to indicate a memory leak.
Start: 25.4 MB
1 min: 127
2 min: 200
3 min: 250
4 min: 297
5 min: 343

@KatieWoe
Copy link
Author

This looks similar in dev.45
newmem

@chrisklus
Copy link
Contributor

Oops, I forgot to update this issue on Friday. @jonathanolson helped me look into the memory status of this sim and we found that it had been greatly improved since the last tests. Initial size was around 50 MB and after 5-7 min of testing we never saw it go above 56 MB.

There was one dragListener to dispose that I am about to commit. I will do a new memory test on 1.0 prototype to confirm no maintenance release is needed for this issue

@marlitas marlitas self-assigned this Jan 4, 2023
@marlitas
Copy link
Contributor

marlitas commented Jan 5, 2023

Did a memory test on each individual screen today and got this:

Screen 1:
Screenshot 2023-01-05 at 1 10 15 PM

Screen 2:
Screenshot 2023-01-05 at 1 24 39 PM

Screen 3:
Screenshot 2023-01-05 at 1 30 09 PM

Screen 4:
Screenshot 2023-01-05 at 1 33 25 PM

It's pretty apparent that Screen 4 has a memory leak. Going to address that first, and then test again.

marlitas added a commit to phetsims/number-suite-common that referenced this issue Jan 5, 2023
marlitas added a commit to phetsims/number-suite-common that referenced this issue Jan 10, 2023
@marlitas
Copy link
Contributor

@zepumph and I addressed the leaky code in the Lab Screen. Another full sim memory test indicates there are no other leaks. Assign to @chrisklus for review.
Screenshot 2023-01-10 at 11 52 06 AM

@chrisklus
Copy link
Contributor

@marlitas and I co-reviewed and everything looks great, thanks! Closing.

chrisklus added a commit to phetsims/number-suite-common that referenced this issue Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants