Dispose of loaders

[brianchirls]

Is your feature request related to a problem? Please describe.

Often, loaders for various file formats (e.g. GLTFLoader, ImageLoader, etc.) are used early in a site life cycle and then not needed anymore, but sometimes they leave resources in use that could be cleaned up. Specifically, loaders that use web workers will leave those threads running in the background.

Describe the solution you'd like

I'd like to see modern and widely used loaders have a dispose method that cleans up any existing resources, like running web workers or files cached in memory. Ideally, any downloads in progress will be aborted. I see FileLoader still uses XMLHttpRequest, which has broad support for .abort.

Describe alternatives you've considered

We could expose underlying resources on the different loader objects, but that would be unwieldy and brittle since each loader would have different resources.

Additional context

This seems related to #18234 and possibly dependent upon #19650. I don't see this specific ability mentioned in that issue or in that pull request in progress, so I figured I'd file it separately.

Thank you.

[donmccurdy]

DRACOLoader does have a dispose() method that cleans up its Web Workers. GLTFLoader doesn't create Web Workers itself, just indirectly when Draco is used. We may use Web Workers elsewhere in the future though, such as for transcoding KTX2/Basis textures embedded in a glTF file.

Given that the usage pattern is...

var loader = new GLTFLoader().setDRACOLoader( dracoLoader );

... do you think GLTFLoader should still have a dispose() method that disposes of its DRACOLoader instance? I feel like that doesn't match up with how dispose() is implemented elsewhere in the library, where reusable resources need to be disposed directly. But it's also not ideal to expect users to know which loaders create Web Workers and which don't... I wonder if this could be part of LoadingManager somehow, or the TaskManager proposed in #18234.

[brianchirls]

Didn't know that about DRACOLoader - thanks. Draco is challenging to deal with because of the way its multiple script assets are configured and loaded.

But it's also not ideal to expect users to know which loaders create Web Workers and which don't...

Yes, I think this is the key point here. There are potentially resources besides workers that need to be cleaned up, and those will shift over time. The cleanest, clearest and safest way is to encapsulate that into a common dispose method that we can count on to be there. I think it's not such a stretch, since it already exists on many other objects in this library, like materials and geometries.

As a general rule, I think it's good practice that an object should be responsible for objects that it creates. You shouldn't have to dig inside to clean up after it.

There's an assumption that resource management isn't something that makes sense in a garbage collected language like JS, but that's not practical in reality when you're working with something as stateful as WebGL and a big framework like three.js. Dispose/destroy is a pattern that I've been relying on more and more in my own code and I'm loving it.

[gkjohnson]

There are potentially resources besides workers that need to be cleaned up

Do you have specific examples of what resources you're expecting to be cleaned up? As far as I know OBJLoader2Parallel (which automatically terminates its worker) and DRACOLoader are the only two that create objects that can potentially be cleaned up (Web Workers / WASM). Everything else should naturally fall out of scope and be collected by the GC -- there wouldn't be value anywhere else.

I see FileLoader still uses XMLHttpRequest, which has broad support for .abort.

I think it would be nice to cancel requests, as well, but disposing of a Loader doesn't feel like the right way to do this to me. Maybe setting an AbortSignal on the loaders would be a good way to enable this and be future proof to if / when FileLoader switches to use fetch.

[donmccurdy]

I think it's good practice that an object should be responsible for objects that it creates.

Agreed, but GLTFLoader instances don't create their DRACOLoader and KTX2Loader dependencies, so the user would need to dispose all three in this scenario. That's still probably for the best I think.

As far as I know OBJLoader2Parallel (which automatically terminates its worker) and DRACOLoader are the only two that create objects that can potentially be cleaned up (Web Workers / WASM).

BasisTextureLoader also creates web workers, and KTX2Loader eventually will (but doesn't yet).

[donmccurdy]

I'm looking at refactoring KTX2Loader and DRACOLoader a bit. Both would have a WorkerPool, which manages Web Worker tasks. Because the two loaders execute different tasks on the worker, they can't currently share a WorkerPool instance. The harder question seems to be:

If multiple instances of e.g. KTX2Loader are created, should they share a WorkerPool?

• Pros: Users create multiple loaders without realizing there are web workers involved. A performance penalty (by not sharing web workers) would catch many by surprise.
• Cons: To support .dispose() properly we need to internally keep track of how many KTX2Loaders exist and destroy the worker pool when the last loader is gone. This is a little awkward since methods like .dispose() and .setDecoderPath( ... ) are instance methods, but with a shared worker pool they'll affect all loaders of a type.

I'm tempted to just show a warning when multiple KTX2Loader or DRACOLoader instances are active (i.e. not disposed) at the same time. This way no one is caught by surprise by the performance penalty, and there isn't a lot of weird shared state behind the scenes. The only real use case I can think of that requires multiple KTX2Loader instances would be comparing different transcoder versions, and that's just a debugging task where warnings can be safely ignored.

Related: https://github.com/mrdoob/three.js/issues/22445

[donmccurdy]

Proposal for KTX2Loader: https://github.com/mrdoob/three.js/pull/22621

[gkjohnson]

To support .dispose() properly we need to internally keep track of how many KTX2Loaders exist and destroy the worker pool when the last loader is gone. This is a little awkward since methods like .dispose() and .setDecoderPath( ... ) are instance methods, but with a shared worker pool they'll affect all loaders of a type.

I think if WorkerPool were going to become a core concept I'd expect some of these methods to move onto the WorkerPool instance. Similar to LoadingManager a DefaultWorkerPool instance could be shared and the user would have to be aware of that when making changes to loader.manager but optionally they have the ability create their own WorkerPool instance with custom initialized settings that can be shared between loaders.

One thought that comes to mind is whether we need to require the user to completely "dispose" of the WorkerPool at all? Can worker instances be created as needed (up to some user-settable cap) and be automatically terminated as the number of jobs goes down? Or be terminated after a user-settable number of seconds passes and they haven't been used? Or of course the user can explicitly terminate the workers on command if needed. WeakRef comes to mind, as well, but I'm not sure if that would afford anything unique with GC that the browser isn't already doing with WebWorkers.

The 3DTilesRendererJS project has a related pattern maybe worth mentioning. In order to manage many downloads and tile file parsing every TilesRenderer instance creates its own download / parse queues and LRU cache that are not shared. So by default if you have many tiles loaders they won't share a cache or priority queues but it's recommended that they do and can easily be done like so:

const tilesRenderer = new TilesRenderer( './path/to/tileset.json' );
const tilesRenderer2 = new TilesRenderer( './path/to/tileset2.json' );

// set the second renderer to share the cache and queues from the first
tilesRenderer2.lruCache = tilesRenderer.lruCache;
tilesRenderer2.downloadQueue = tilesRenderer.downloadQueue;
tilesRenderer2.parseQueue = tilesRenderer.parseQueue;