Skip to content

fix: local scene dev reload memory leak #3595

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

Merged
merged 7 commits into from
Mar 17, 2025

Conversation

pravusjif
Copy link
Member

@pravusjif pravusjif commented Mar 15, 2025

WHY

During Local Sccene Development mode, RAW GLTFs are being used instead of asset bundles. With every SDK scene code change or file change, the scene gets automatically reloaded (known as "hot-reload")

It has been detected that GLTF textures are not being disposed at all when the scene is reloading, infinitely accumulating in memory, potentially making the app crash or the hot reload unreliable

Issue: decentraland/creator-hub#271

Image Image Image

WHAT

  • Fixed GLTFData not being disposed of correctly
  • Improved cache and unreferenced assets unload during LocalSceneDevelopment scene reload

TEST INSTRUCTIONS

  1. Download this test scene
  2. Enter the scene root folder and run npm i and then npm run start -- --explorer-alpha
  3. Close the Explorer that auto-opened. Leave the scene running in the console/terminal.
  4. Download the build from this PR and connect it to the running scene using the console command according to your OS
  5. Once the scene loads, open the "Memory" section of the Debug Panel and keep it in sight, take note of the approximate value you see in "system used memory" for that 1st scene load.
  6. Run the "/reload" chat command in the chat window and confirm that the scene is reloaded
  7. Do 10 more reloads, and confirm that after the last one, your memory hasn't increased more than 500MB (maybe it increased even less) since the 1st scene load memory value, making it a lot more stable.

Note: It's OK if you see the chat message saying "Error running command." that's an unrelated (inoffensive) problem

VIDEO COMPARISON DEMO

PROD:

After 11 scene reloads (without changing anything on the scene) the memory is almost doubled.

LSD-memory-lead-PROD.mp4

THIS PR:

After 11 scene reloads (without changing anything on the scene) the memory is kept pretty stable, only increasing a total of about 300~ MBs and never surpassing that.

LSD-memory-lead-FIX.mp4

@pravusjif pravusjif added bug Something isn't working sdk memory-issue labels Mar 15, 2025
@pravusjif pravusjif self-assigned this Mar 15, 2025
Copy link
Contributor

github-actions bot commented Mar 15, 2025

@pravusjif
Copy link
Member Author

pravusjif commented Mar 15, 2025

Up to this point (c9cf4a6), now that GLTF textures are being cleaned up correctly they are not longer leaking but I've detected GLTF materials are still leaking (185~ added on every reload, they never get disposed...):

Image Image

@pravusjif
Copy link
Member Author

With 0eba597 commit I clean up the leaked materials on LSD reload:

Screenshot 2025-03-15 at 6 44 03 PM Screenshot 2025-03-15 at 6 44 48 PM

@pravusjif
Copy link
Member Author

There appears to still be a small leak on reloads but unrelated to Local Scene Development:

Screenshot 2025-03-15 at 7 27 10 PM Screenshot 2025-03-15 at 7 27 50 PM Screenshot 2025-03-15 at 7 28 00 PM

@pravusjif pravusjif added the force-build Used to trigger a build on draft PR label Mar 15, 2025
@pravusjif pravusjif removed the force-build Used to trigger a build on draft PR label Mar 17, 2025
@pravusjif pravusjif marked this pull request as ready for review March 17, 2025 09:52
@pravusjif pravusjif requested review from a team as code owners March 17, 2025 09:52
@pravusjif pravusjif moved this to QA in Creators Tools Mar 17, 2025
@pravusjif pravusjif moved this from Todo to With QA / Awaiting Review in Explorer Alpha Mar 17, 2025
@pravusjif pravusjif removed request for a team and mikhail-dcl March 17, 2025 09:53
Copy link
Collaborator

@popuz popuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup 👍

Copy link
Contributor

@anicalbano anicalbano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️ PR reviewed and approved by QA. Didn't find any issues while reloading the scenes and the memory remained stable with no significant increments.

17.03.2025_08.53.26_REC.mp4

@github-project-automation github-project-automation bot moved this from With QA / Awaiting Review to In Progress in Explorer Alpha Mar 17, 2025
@m3taphysics m3taphysics enabled auto-merge (squash) March 17, 2025 12:02
@pravusjif
Copy link
Member Author

Sorry Ani, can you re-check with this other scene?
https://github.com/decentraland/sdk7-test-scenes/tree/main/scenes/54%2C-55-Testing-3d-models

I just updated the original description, my bad =S

@m3taphysics m3taphysics merged commit a2162cf into dev Mar 17, 2025
6 checks passed
@m3taphysics m3taphysics deleted the fix/local-scene-dev-reload-memory-leak branch March 17, 2025 12:26
@github-project-automation github-project-automation bot moved this from In Progress to Done in Explorer Alpha Mar 17, 2025
@github-project-automation github-project-automation bot moved this from QA to Done in Creators Tools Mar 17, 2025
@anicalbano
Copy link
Contributor

@pravusjif all good with that scene too!

17.03.2025_09.35.29_REC.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working memory-issue sdk
Projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants