Skip to content

Loaders: Use unique cache keys per loader type. #31315

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 2 commits into from
Jun 26, 2025
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jun 26, 2025

Fixed #27301.

Description

THREE.Cache is currently used by three core loaders: FileLoader, ImageLoader and ImageBitmapLoader.

Since the cache is global, loaders potentially share cache entries if the same URL is loaded by different loaders. Granted, that is an edge case but the OP of #27301 described a potential use case in #27301 (comment).

And since the cached results are incompatible between loaders anyway, it makes sense like described in #27301 (comment) to make the cache entries unique per loader. This is done by adding an additional loader-specific key part to the actual cache key.

Copy link

github-actions bot commented Jun 26, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 337.65
78.76
337.65
78.76
+0 B
+0 B
WebGPU 557.6
154.28
557.6
154.28
+0 B
+0 B
WebGPU Nodes 556.52
154.07
556.52
154.07
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 468.89
113.45
468.89
113.45
+0 B
+0 B
WebGPU 632.96
171.3
632.96
171.3
+0 B
+0 B
WebGPU Nodes 588.01
160.61
588.01
160.61
+0 B
+0 B

@Mugen87 Mugen87 added this to the r178 milestone Jun 26, 2025
@Mugen87 Mugen87 merged commit cdd6c5b into mrdoob:dev Jun 26, 2025
12 checks passed
@Mugen87 Mugen87 changed the title Loaders: Use unique cache keys per loader. Loaders: Use unique cache keys per loader type. Jun 26, 2025
@mrdoob
Copy link
Owner

mrdoob commented Jun 27, 2025

Hmm... Would it make sense to do this all the loaders instead? objloader:, gltfloader:, etc?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 27, 2025

No other loaders are directly using THREE.Cache.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 27, 2025

Ah wait..I see what you mean. Honoring the owner of the current file or image (bitmap) loader in the key instead of always using a name per loader type.

However, I'm not sure there is a benefit of doing this. I can understand if an image is loaded differently like described in #27301 (comment) but I don't think there is a use case for loading a glTF (or OBJ, FBX etc.) asset with a different kind of asset loader.

Besides, the "owner" information does not exists in the three basic loaders so we would need to pass additional name information from GLTFLoader or OBJLoader when creating file or image loaders.

@mrdoob
Copy link
Owner

mrdoob commented Jun 27, 2025

Okay, sounds good to me!

Made some clean up: ee327fd ac07a8a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache clobbering when loading same file through FileLoader, ImageBitmapLoader, or ImageLoader
2 participants