Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

does this sim do any memory management?

Open
  • Is something wrong?
#51 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
15/100
Issue type
Refactor
Clarity
Needs clarification
Activity status
Stale
Tech stack
javascript
Domain
frontend, performance

Research direction

Start by reading the memory-leak testing work in #32 and the documentation request in implementation-notes.md. Audit sim-specific uses of link, DerivedProperty, Multilink, Events.on, Emitter.addListener, and Node.on against their cleanup calls. Done means the relevant leaks and dispose requirements are addressed or documented, with the overall memory-management strategy recorded.

Written by the indexing model from the issue text.

Description

dev:code-review

Related to #2 (code review), there are 3 items related to memory management:

  • For each common-code component (sun, scenery-phet, vegas, …) that opaquely registers observers or listeners, is there a call to that component’s dispose function, or is it obvious why it isn't necessary, or is there documentation about why dispose isn't called? An example of why no call to dispose is needed is if the component is used in a ScreenView that would never be removed from the scene graph.
  • Are there leaks due to registering observers or listeners? The following guidelines should be followed unless there it is obviously no need to unlink, or documentation (in-line or in the implementation nodes)added about why following them is not necessary. Unlink is not needed for properties contained in classes that are never disposed of,
    such as primary model and view classes that exist for the duration of the sim.
    - [ ] AXON: Property.link is accompanied by Property.unlink.
    - [ ] AXON: Creation of DerivedProperty is accompanied by dispose.
    - [ ] AXON: Creation of Multilink is accompanied by dispose.
    - [ ] AXON: Events.on is accompanied by Events.off.
    - [ ] AXON: Emitter.addListener is accompanied by Emitter.removeListener.
    - [ ] SCENERY: Node.on is accompanied by Node.off
    - [ ] TANDEM: PhET-iO instrumented PhetioObject instances should be disposed.
  • Do all types that require a dispose function have one? This should expose a public dispose function that calls this.disposeMyType(), where disposeMyType is a private function declared in the constructor. MyType should exactly match the filename.

There are zero definitions of dispose in sim-specific code, and zero calls to dispose of common-code. That may be OK, or it may point to memory leaks.

Things to do:

  • Test for memory leaks, see #32
  • For each call to the functions mentioned above (e.g. link), have an associated cleanup call (eg. unlink) or document why one is not needed.
  • Document overall memory management strategy in implementation-notes.md, see #8
Dominant language
JavaScript
Stars
0
Forks
2
No merged PRs in 30d

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from phetsims/normal-modes

All issues in phetsims/normal-modes

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.