-
Notifications
You must be signed in to change notification settings - Fork 2
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
reduce memory usage of sims #969
Comments
Sounds like a good question for @ariel-phet in general. It sounds like the biggest concern is crashes due to memory usage. Patterns like those mentioned above ALSO improve cpu performance, so it seems generally nice to do for common code. I've generally been trying to write the most readable/maintainable code up front, handling performance/memory issues where problems arise, but we could definitely be more proactive. |
I want to look at this from a many-year perspective. We want to maintain our simulations for many years, and make it easy to add accessibility, phet-io and other features as desired. Our supported platforms are gaining more memory and more CPU power. In Energy Skate Park: Basics, we sacrificed readability/maintainability in order to reduce allocations--see for example the commits in phetsims/energy-skate-park-basics#50 or search for the term "allocation" in energy-skate-park repo. However, after we subsequently (a) added WebGL and (b) dropped support for iPad 2, many of those performance optimizations are likely unnecessary and hassles that should probably be removed to simplify maintenance and development going forward. We have an Android app, but do not regularly performance optimize on Android phones or tablets (though we do test on chromebooks). If one day we need to attain maximum performance on older Android phones or tablets, it would be better to do this via a systemic approach like phetsims/scenery#303 rather than rewriting disparate bits and pieces to save a few MB of memory. On the other hand, changes like the one in phetsims/tandem#71 afforded a huge advantage at a very small change in readability and little or no impact on future accessibility or phet-io support, so that is a good change. |
1/3/19 dev meeting notes @ariel-phet: It is a natural part of PhET evolution that our software uses more memory and CPU as we move forward. We will keep an eye on it, but we don't have plans to suddenly support older/slower platforms. I'm generally OK with our current policy: dropping older, slower platforms as their usage drops off the chart. Most of our software still has good performance on our most used platforms, and we don't receive a significant number of reports of performance issues. OK to improve memory usage as long it does not damage readability/maintainability, but we should not "mess up" our code in order to save 1-2MB or such. Accessibility and PhET-iO would need to be more settled before we have a gestalt understanding of where memory is spent. @zepumph: phetsims/tandem#71 was somewhat a surprise and was a good opportunity for us to do our first significant memory testing for PhET-iO. @pixelzoom: The pattern we saw in phetsims/tandem#71 . May be good to use that policy in many sims. @ariel-phet: Should this be captured in our phet-software-design-patterns.md ? Should we set aside core hours time for reviewing this doc? @jonathanolson: Code that is running once or a handful of times should be written in most maintainable/readable form. Code that is running many times per frame in an inner loop may require performance/memory optimization. |
This came up in discussion of phetsims/tandem#71 with @samreid. After mitigating the memory consumption due to IO types, we noticed other opportunities to reduce memory usage.
Some questions for discussion at dev meeting:
Should we reduce memory usage of sims? Is the effort worth the cost?
What patterns should devs be aware of?
How aggressively should we apply patterns like factor out constants for option default values #968?
The text was updated successfully, but these errors were encountered: