Skip to content

20% render() performance boost Part 2 (cache attribute names array): avoid 4x object allocation in Renderer._renderObjectDirect()#33111

Closed
PoseidonEnergy wants to merge 5 commits into
mrdoob:devfrom
PoseidonEnergy:fix/no-equals-object-allocations-1
Closed

20% render() performance boost Part 2 (cache attribute names array): avoid 4x object allocation in Renderer._renderObjectDirect()#33111
PoseidonEnergy wants to merge 5 commits into
mrdoob:devfrom
PoseidonEnergy:fix/no-equals-object-allocations-1

Conversation

@PoseidonEnergy
Copy link
Copy Markdown
Contributor

@PoseidonEnergy PoseidonEnergy commented Mar 2, 2026

Related issue: #33107

This is Part 2 of this PR.

This updates BufferGeometry such that attribute names are cached. This allows NodeMaterialObserver.equals() to avoid Object.keys(attributes) being called 2 times per mesh per render. Object.keys() is expensive in tight loops.

What I really think

BufferGeometry.attributes should really be a dedicated class, like class BufferGeometryAttributes { ... }. It would be much cleaner to maintain a cached attribute names array (like this PR is doing) internally inside a BufferGeometryAttributes class instead of wrapping the BufferGeometry.attributes object in a Proxy to do the same thing.

I only wrapped the BufferGeometry.attributes object in a Proxy for this PR to get the ball rolling on this, but I think a dedicated BufferGeometryAttributes class would be better. That way, direct access to the internal object would not be available, and attributes would be added/fetched via setAttribute() and getAttribute().

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 2, 2026

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 359.27
85.31
359.27
85.31
+0 B
+0 B
WebGPU 629.85
174.95
629.97
174.99
+125 B
+39 B
WebGPU Nodes 628.43
174.7
628.55
174.74
+125 B
+40 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 491.03
119.72
491.4
119.92
+366 B
+200 B
WebGPU 703.47
189.95
703.96
190.13
+491 B
+174 B
WebGPU Nodes 652.67
177.34
653.16
177.52
+491 B
+178 B

@Mugen87
Copy link
Copy Markdown
Collaborator

Mugen87 commented Mar 3, 2026

I agree we should get rid of Object.keys() in NodeMaterialObserver.equals(). But I'm not sure the suggested approach is the best choice. Let me investigate alternatives.

@Mugen87
Copy link
Copy Markdown
Collaborator

Mugen87 commented Mar 3, 2026

Closing in favor of #33113.

@Mugen87 Mugen87 closed this Mar 3, 2026
@Mugen87 Mugen87 modified the milestone: r184 Mar 3, 2026
@Mugen87
Copy link
Copy Markdown
Collaborator

Mugen87 commented Mar 3, 2026

I only wrapped the BufferGeometry.attributes object in a Proxy for this PR to get the ball rolling on this, but I think a dedicated BufferGeometryAttributes class would be better. That way, direct access to the internal object would not be available, and attributes would be added/fetched via setAttribute() and getAttribute().

Unfortunately, a lot of code does not use the set/get method interface but directly access the internals of BufferGeometry.

As an alternative, one could also consider a less dynamic solution and introduce needsUpdate on BufferGeometry level. Every time the attribute setup is structurally changed (attributes are added, replaced or removed), the app must set the flag to true. Since the app initializes the attribute change, it also knows the exact point in time to set the flag so the engine does not have to do any dynamic checks.

We could also go one step further and treat BufferGeometry as structurally immutable after the first render. But I understand this is maybe to strict and apps would like to reuse geometry instances.

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.

2 participants