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

fix: 912-helpers-disposal-issue #913

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

alvarosabu
Copy link
Member

@alvarosabu alvarosabu commented Feb 1, 2025

  • Updated memory demo to use requestAnimationFrame instead of interval for more accurate results across multiple devices running the test
  • Refactor disposeObject3D method to remove deletes. Improved different type of object disposal (BufferGeometries, Scenes, Lights etc)
  • Removed unnecessary camera warning
  • Added extra step on TresCanvas disposal to clear WebGL context.
  • ForceContextLoss for temporarily created three renderer.

Closes #912

Tests

Test were done using MemoryTresObjects.vue with 2000 objects and 400 TresCanvas instances (instead of 1000 since no-disposal tests were hanging the browser at 500)

Browser: Chrome incognito mode.

Performance

Disposal Method Time to Complete JS Heap
No disposal > 30s 3,929 MB
Current disposal (delete) 17.46s 503MB
New disposal (no delete) 17.46s 232MB

No Disposal

Performance no disposal

Comments: We can see how the JS Heap grows on an upward trend that reaches almost 4 GB. This is the baseline

Current disposal (using delete)

Performance current disposal

Comments:

  • JS Heap has a Sawtooth pattern indicating garbage collection but still there is a gradual upward trend
  • This suggests some memory is being retained between cycles (memory leak)
  • These spikes (7.7MB - 503MB according to the metrics) suggest bulk object creation

New disposal

Performance new disposal

JS Heap is now much more stable (7.9MB - 232MB vs previous 7.7MB - 503MB)
The sawtooth pattern in the heap graph is more regular and controlled
The baseline doesn't show the same concerning upward trend
Memory spikes are smaller and more consistent

Memory allocation timeline

Disposal Method Time to Complete JS Heap
No disposal Couldn't complete ? (We assume big)
Current disposal (delete) 71.312s 40.6MB
New disposal (no delete) 94.313 79.0MB

No disposal

Browser crashed.

Current disposal

Memory allocation timeline - current disposal

Memory allocation timeline objects - current disposal

New Disposal

Memory Allocation timeline - new disposal
Memory Allocation timeline objects - new disposal

Comments: in the second screenshot, you will notice if compared with current implementation the presence of the properties of BoxGeometry, that indicates that the objects aren't being fully garbage collected - their internal methods and properties are still being retained in memory. This is different from what we'd expect in a fully disposed object, where these internal details should not be visible anymore.

Summary

Performance Metrics

Metric Current (delete) New Implementation Difference
Total Time 71.312s 94.313s +23.001s
Scripting 16,563ms 16,706ms +143ms
Rendering 182ms 186ms +4ms
Painting 67ms 69ms +2ms
System 204ms 214ms +10ms
JS Heap Range 7.7MB - 503MB 7.9MB - 232MB -271MB peak

Memory Allocation

Component Current (delete) New Implementation Difference
System/Context 38.8MB (96%) 40.9MB (52%) +2.1MB (-44%)
RefImpl 17.5MB (43%) 17.4MB (22%) -0.1MB (-21%)
WebGLRenderer 16.7MB (41%) 16.7MB (21%) 0MB (-20%)
Function/compiled code 14.4MB (36%) 28.7MB (36%) +14.3MB (0%)
Object 11.3MB (15%) 11.3MB (14%) 0MB (-1%)
Array 4.9MB (12%) 9.9MB (13%) +5MB (+1%)
WebGLTextures 4.7MB (12%) 4.6MB (6%) -0.1MB (-6%)
WebGLRManager 2.9MB (7%) 2.9MB (4%) 0MB (-3%)
Scene 2.6MB (6%) 2.6MB (3%) 0MB (-3%)
Total Selected Size 38.3MB 74.2MB +35.9MB

Key Observations

Performance: New implementation takes ~32% longer but shows more stable memory patterns
Memory Peak: New implementation reduces maximum memory spike by 271MB
Memory Distribution: New implementation shows better percentage distributions across all categories
Resource Management: Better cleanup of WebGL resources despite higher total selected size
Memory Tracking: More detailed memory tracking visible in new implementation (BoxGeometry properties visible)

Copy link

netlify bot commented Feb 1, 2025

Deploy Preview for tresjs-docs ready!

Name Link
🔨 Latest commit 5fcd0bc
🔍 Latest deploy log https://app.netlify.com/sites/tresjs-docs/deploys/679e0d02c068840008a5ec9d
😎 Deploy Preview https://deploy-preview-913--tresjs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@alvarosabu alvarosabu added the p4-important-bug Violate documented behavior or significantly improve performance (priority) label Feb 1, 2025
Copy link

pkg-pr-new bot commented Feb 1, 2025

Open in Stackblitz

npm i https://pkg.pr.new/@tresjs/core@913

commit: 5fcd0bc

@alvarosabu alvarosabu self-assigned this Feb 1, 2025
@andretchen0
Copy link
Contributor

No disposal | 30s | 3.929 MB

Guessing this is a typo. Should be GB?

@alvarosabu
Copy link
Member Author

No disposal | 30s | 3.929 MB

Guessing this is a typo. Should be GB?

Yes, is 3,929 MB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-important-bug Violate documented behavior or significantly improve performance (priority)
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Helpers disposal issue
2 participants