Three.js: WebGLBindingStates: Memory leak in needsUpdate().

[Usnul]

Describe the bug

There's a very interesting memory leak introduced at some point

https://github.com/mrdoob/three.js/blob/43c3d98af25afbbd1baa9d945cb8594e558422f7/src/renderers/webgl/WebGLBindingStates.js#L172

What it does is crease 2 arrays every time the method is called (via Object.keys) just to compare their lengths. In my basic performance tests - I saw this little gem contribute to about 35% of GC overhead in my application or around 1kb per frame.

To Reproduce

Steps to reproduce the behavior:

1. Go to https://threejs.org/examples/#webgl_instancing_performance
2. Click on the mouse
3. See error in usage of allocations, after you run an allocation recording

Code

if ( Object.keys( cachedAttributes ).length !== Object.keys( geometryAttributes ).length ) return true;

Live example

• *-latest-release

Expected behavior

No allocations expected to be performed

Screenshots

If applicable, add screenshots to help explain your problem (drag and drop the image).

Platform:

• Device: [Desktop]
• OS: [Windows]
• Browser: [Chrome, Firefox]
• Three.js version: [r??? - yes, 122]

[mrdoob]

Please use the provided bug report template. I'll delete this otherwise.

[makc]

@Usnul blame says this line is there since April 2019 (fe2533d7) when WebGLBindingStates.js was initially created

[takahirox]

Hi @Usnul, thanks for the report. I think this report is meaningful. Would you please update the issue comment with the bug report template because I don't want this report to be deleted?

In my basic performance tests - I saw this little gem contribute to about 35% of GC overhead in my application or around 1kb per frame.

I came up with a solution. Would you please share the test with me to check if it works? Or could you test in your side for me if I make a PR?

[makc]

@takahirox btw how faster it is (and is it, actually) having this check there vs not having it at all?

[Mugen87]

vs not having it at all?

The check is required.

[makc]

@Mugen87 isnt it just to quit early before checking values

[Usnul]

Hi @Usnul, thanks for the report. I think this report is meaningful. Would you please update the issue comment with the bug report template because I don't want this report to be deleted?

In my basic performance tests - I saw this little gem contribute to about 35% of GC overhead in my application or around 1kb per frame.

I came up with a solution. Would you please share the test with me to check if it works? Or could you test in your side for me if I make a PR?

Sure, I can test it out if you make a PR. I can review it as well if you ping me.
I use FF for memory testing, they have a pretty nifty allocation recording feature.