Use WeakMap for geometries, render lists, and render states

[JoelEinbinder]

Today I learned that you had to dispose geometries otherwise they would leak. I was curious as to why that was, and it lead me to an object being used as a map, where a WeakMap looks like it would also work fine. Swapping it out for a WeakMap seems to have fixed the memory leak in my game, without me having to manually track geometries and call .dispose on them. I found a few other places where the same pattern was being used, and changed them there too.

This assumes that the JavaScript wrappers for Object3Ds are never reused with new ids. It doesn't look to be the case, but I couldn't find any specific documentation on this.

Assuming this works as I think it does and is merged, it raises the question of why does the manual geometry.dispose exist at all? Maybe it has some use for avoiding some gc funkiness, but it looks like the only side effect of not calling dispose would be WebGLInfo having the wrong count.

[mrdoob]

This would sure help on the js memory side, but we still need to call geometry.dispose() to deallocate the data from the gpu memory. In WebGL-land we have to do the garbage collection ourselves...

It would be awesome if javascript runtimes dispatched events when collecting garbage so we could check if we can remove the related data in the GPU.

[JoelEinbinder]

This would sure help on the js memory side, but we still need to call geometry.dispose() to deallocate the data from the gpu memory. In WebGL-land we have to do the garbage collection ourselves...

Are you sure about this? From my tests, it looks like things are being cleaned up somehow, at least as far as gpu memory isn't growing. Can you point me to the place in threejs where it does some webgl stuff when a geometry is disposed. It looks to me like all it does is clean up the JavaScript memory.

It would be awesome if javascript runtimes dispatched events when collecting garbage so we could check if we can remove the related data in the GPU.

WeakRefs and FinalizationGroups look like they are coming, but I wouldn't recommend using them for anything mission critical.

[JoelEinbinder]

This would sure help on the js memory side, but we still need to call geometry.dispose() to deallocate the data from the gpu memory. In WebGL-land we have to do the garbage collection ourselves...

Are you sure about this? From my tests, it looks like things are being cleaned up somehow, at least as far as gpu memory isn't growing. Can you point me to the place in threejs where it does some webgl stuff when a geometry is disposed. It looks to me like all it does is clean up the JavaScript memory.

Ah I see, gl.deleteBuffer in WebGLAttributes.js. I'll do some more investigating and get back to you.

[JoelEinbinder]

This would sure help on the js memory side, but we still need to call geometry.dispose() to deallocate the data from the gpu memory. In WebGL-land we have to do the garbage collection ourselves...

Are you sure about this? From my tests, it looks like things are being cleaned up somehow, at least as far as gpu memory isn't growing. Can you point me to the place in threejs where it does some webgl stuff when a geometry is disposed. It looks to me like all it does is clean up the JavaScript memory.

Ah I see, gl.deleteBuffer in WebGLAttributes.js. I'll do some more investigating and get back to you.

I happened to have a copy of webkit in front of me, so I added some printfs and webgl cleanup is indeed happening when the javascript wrapper is collected. This appears to be the specced behavior:

Mark for deletion the buffer object contained in the passed WebGLBuffer, as if by calling glDeleteBuffers. If the object has already been marked for deletion, the call has no effect. Note that underlying GL object will be automatically marked for deletion when the JS object is destroyed, however this method allows authors to mark an object for deletion early.

This does make the case for dispose clear though. Manually calling dispose would be a good idea if you had very large geometries and very small js memory usage. There could be a mismatch in the memory pressure in the JS process and the GPU process, causing the js process to never feel the need to garbage collect and the GPU process to go out of memory. I don't think thats very realistic, because the arraybuffers on geometries are being cached on the JS side as well, so memory usage should be pretty symmetrical. But yeah, its good to have a manual override for this case.

I am more confident in this PR now. I think it aligns threejs with how gl.deleteBuffer works. Dispose is optional, unless you want to be smarter than the JS garbage collector in which case you have a manual override.

[mrdoob]

I happened to have a copy of webkit in front of me, so I added some printfs and webgl cleanup is indeed happening when the javascript wrapper is collected. This appears to be the specced behavior:

Mark for deletion the buffer object contained in the passed WebGLBuffer, as if by calling glDeleteBuffers. If the object has already been marked for deletion, the call has no effect. Note that underlying GL object will be automatically marked for deletion when the JS object is destroyed, however this method allows authors to mark an object for deletion early.

@kenrussell Can we rely on that behaviour? Are the uploaded buffers/textures deleted from GPU memory when the javascript references are deleted? Can we rely on WeakMap?

[kenrussell]

Yes, it is guaranteed that when a WebGL object like a buffer or texture is garbage collected, the associated GPU resources will be deallocated.

However - JavaScript's garbage collector doesn't run on a guaranteed schedule. It is easy to get into situations where an application has a severe resource leak, or runs out of GPU memory, because it rapidly allocates and loses all references to WebGLBuffers or WebGLTextures which hold on to large GPU memory allocations.

Different browsers implement different heuristics for telling their respective JavaScript engines about memory pressure induced by large GPU allocations. Feedback from the V8 team has been that they don't like the notifications about "external memory allocations" - they only cause the garbage collector to run more often than normal, which usually results in degraded application performance.

In sum - please keep the dispose() methods in Three.js, and encourage developers to call them in order to promptly reclaim the GPU memory associated with objects that are no longer needed. Switching to a WeakMap internally for these various lists sounds fine, and a good change.

[mrdoob]

Thanks for the info @kenrussell!

I also remember we tried to implement this many years ago but browser compatibility for WeakMap was not there yet.

Lets give a go then! 👌

[mrdoob]

Thanks!

[aardgoose]

@JoelEinbinder

A quick warning: your observation that JS memory usage and GPU usage is roughly symmetrical breaks down if you use the BufferAttribute.onUploadCallback() to drop the JS side data after this has been sent to the GPU. (I use this a lot to avoid excessive JS heap, which is why the callback was added).

Aside: when I added WeakMaps for attributes originally, tests showed that WeakMap was slightly faster than just using an {}, which is nice.

[WestLangley]

@aardgoose wrote

use the BufferAttribute.onUploadCallback() to drop the JS side data after this has been sent to the GPU.

Hmm... I think this is what BufferAttribute.dynamic = false was originally used for -- to automatically free the JS data -- but something happened along the way to change the purpose of the property. It is a shame, because its original purpose made so much sense...

Do you remember?

[aardgoose]

@WestLangley
I don't think so. When I added the callback the dynamic flag was still just the pass through to the GL buffer allocation hint.

The dynamic flag got overloaded in contradictory ways shortly after because the preservation of the ArrayBuffer's properties collided with changing the ArrayBuffers length via setArray().

[WestLangley]

@aardgoose FYI: this is what I now remember from earlier: #3088 (comment).

Looks like I'm still struggling with #14730. :-)

What code pattern do you use in BufferAttribute.onUploadCallback() now?

[aardgoose]

I think to untangle things,

1. deprecate resizing BufferAttributes via .setArray()

Point users to replace a geometry's existing BufferAttribute instead, we don't have to worry about disposal of the existing object too much since its now only held by a weak map, as a result the internal renderer object referencing the webGL buffer object will be unreferenced and hence any GPU allocations will be deleted with GC (as discussed above). It might be good to have a BufferAttribute.dispose() method for full control, but not essential.

2. remove overloading of dynamic in WebGLAtttributes.

Just use gl.bufferSubData() in all cases in WebGLAttrributes.update(), which appears to be the recommended pattern, which is now safe because we don't support resizing.

Then .dynamic flag is purely a hint as originally defined. you are free to remove/rename dynamic if wanted.

I think it will be difficult to benchmark its usefulness because implementation of the hint is GPU/driver specific.

The code I use

function onUploadDropBuffer() {
	this.array = null;
}

And within the mesh constructor, something along these lines:

var attributes = this.geometry.attributes;

for ( var name in attributes ) attributes[ name ].onUpload( onUploadDropBuffer );

this.geometry.index.onUpload( onUploadDropBuffer );

[EliasHasle]

@aardgoose

The code I use

For what? All static geometries, to save memory?

[aardgoose]

@EliasHasle

Exactly, I use multiple large meshes for accurate real word terrain, and dropping the JS heap copies of the geometries allows more complex models without crashing the browser.

[gkjohnson]

Just to clarify what I'm getting in this thread is that using WeakRef to automatically dispose of geometry wouldn't buy us anything that WeakMap isn't already getting us. And that while you should still call dispose when you can you're not going to indefinitely accumulate GPU memory in your application if you forget to call it as long as you lose all the geometry references you're carrying around. Is that right?

[Mugen87]

This is also my understanding (see #17242 (comment)).