(high memory use issue) Setting a flag to not copy arrays

[magnus-poppe-adsk]

The issue:

I'm working on a piece of code which selectivly copies out features from a gltf model into a new gltf model. You can think of this usecase as stripping away unwanted features from a gltf model (joints, animations etc).

https://github.com/donmccurdy/glTF-Transform/blob/872ab7c9c0bb7ba561bad8c7f63a480f6a68c8ed/packages/core/src/properties/property.ts#L111-L115 When running the set() method above, array duplication is done for any new set array. This causes massive use of memory, which in turn makes the garbage collector run, spending more CPU time than runtime code.

Running @gtlf-transform/core v3.9.0	Self Time	Total Time	File
0.00ms	18,658.00ms	(root)
14,643.80ms	14,643.80ms	(garbage collector)

Modifying the snippet above, removing .slice(), we see a major performance increase:

 /** @hidden */
 protected set<K extends LiteralKeys<T>>(attribute: K, value: T[K]): this {
    if (Array.isArray(value)) value = value as T[K]; // copy vector, quat, color …
    return super.set(attribute, value);
 }

Self Time	Total Time	File
0.00ms	7,980.46ms	(root)
4,132.71ms	4,132.71ms	(garbage collector)

Proposed solution

I understand that stoping to copy this array will cause unexpected issues. I therefore suggest adding an opt-out flag for copying arrays.

The model used to produce these numbers is a real architecture model exported to glb with a ~500MB size

[donmccurdy]

Thanks for asking @magnus-poppe-adsk! I think we can figure something out here.

Even for a ~500mb file it's unexpected that this method would be in the hot code path... is that due to very large numbers of scene nodes perhaps? Any chance you can share an example file for benchmarking?

Perhaps updating the previous array "in place" would also be an option, rather than storing the passed array by reference. I'll think about both options some more.

I have some other changes planned for v4 that are intended to improve performance for these more complex files, as well.

[donmccurdy]

Related:

• 1141

• 1133

[magnus-poppe-adsk]

I'll look into if i can share the file or not. What causes this high memory is that when we are copying the GLB, we do this operation once per accessor in the file:

We also copy other features like textures, materials etc.

I'll look into those issue/PRs 💯

[donmccurdy]

I see! Never mind my suggestion of updating the array in-place then, that doesn't work here. But I do think we can improve the memory load with a flag, or similar.

Maybe also relevant:

• 924

This would allow you to transfer (or perhaps copy?) specific properties to a different document. Whereas now I assume you are cloning the entire document and then disposing of everything you don't need? Many calls to dispose() can also be a huge bottleneck for very complex models, which is what #1141 is intended to address.

[donmccurdy]

@magnus-poppe-adsk I've made some progress toward improving processing speed for large models, including adding a set of benchmarks, but I haven't been able to come up with a test case where these array copies are a meaningful cost. I'm hesitant to add flags without some way to measure that, to make sure I'm optimizing the right thing. If it would help at all with sharing a model for debugging, you can find my email address on my website (https://www.donmccurdy.com/).

[donmccurdy]

Without being able to reproduce a performance problem related to array copies, I'm afraid I am not comfortable maintaining an option that aims to avoid that problem. I'd be happy to investigate further with a test case, but for now I think I'll need to close this issue.

I would also recommend checking glTF Transform v4 — there some performance improvements in the alpha now, with more planned for the stable release.