Three.js: GLTFLoader: Disable cache

[rpadovani]

We are working on an huge project with thousands of objects, and every object is unique. We load data from a gltf file, and the loader checks if any object is equal to something already created.

Disabling the check on the cache here (return null instead of calling isPrimitiveEqual over thousands of objects) cuts the loading time by a factor of 4, in our case.

I know this is a corner case, but would you be interested in an option to disable the cache? My company will sponsor my work on this, because we have to implement it anyway, and makes more sense to do it upstream than keeping our own version of the loader

[mrdoob]

How about creating new loader instance per object?

[donmccurdy]

First of all, thanks for opening the issue and sharing your performance numbers — that’s invaluable. :)

The geometry cache is kept at the parser level, and isn’t shared across .load() requests, so one-per-model loaders won’t help here unless I’ve missed something. Splitting your single glTF file into many files would help, but understandably you may not want to do that for other reasons.

I think there are two choices:

1. An option to disable the cache as you suggest.
2. Reimplement the cache to create a unique string key for each primitive, so cache lookups are O(1) rather than O(n).

My preference would be (2). Are you able to test that and see if the results are acceptable? If it’s still prohibitively slow I’m also open to (1), but I suspect the overhead of a faster cache would be negligible.

[rpadovani]

Hi, thanks for your answers :-)

Splitting the file isn't really an option - our customer target's browser doesn't support HTTP/2, so loading hundreds (or thousands) of different files have some overhead. TBH I haven't tested it, but it something I'd like to avoid: we should also keep a track of all the files and sending them to the client in some way.

I like the idea to make the cache lookups O(1), but I don't know how to implement it in a generic way: I can always add something like an id in our GLTF, but then I have to use our specific id.

At the moment a new object in the cache is added as:

cache.push( { primitive: primitive, promise: geometryPromise } );

There is any way to create an unique signature from the primitive?

The only generic implementation I can think of is using something like object-hash to create an unique hash of objects and use that as key.
Any other suggestion?

[donmccurdy]

Splitting the file isn't really an option...

Understood 👍

There is any way to create an unique signature from the primitive?

The list of properties a primitive can have is finite, just what we compare here:

https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/GLTFLoader.js#L1513-L1548

So essentially, rather than explicitly comparing each of those values, we should compute a unique primitive key from them. It can be as simple as, roughly:

var primitiveKey;
var dracoExtension = primitive.extensions && primitive.extensions[ EXTENSIONS.KHR_DRACO_MESH_COMPRESSION ];

if ( dracoExtenison ) {

  primitiveKey = 'draco:' + dracoExtension.bufferView
    + ':' + dracoExtension.indices
    + ':' + createAttributesKey( dracoExtension.attributes );

} else {

  primitiveKey = primitive.indices + ':' + createAttributesKey( primitive.attributes );

}

... where createAttributesKey just does some join operation like SEMANTIC:<accessor>,SEMANTIC:<accessor>,....

Obviously that's not as fully robust as object-hash, but it only matters if users are putting custom extensions or metadata into these primitives, in which case it's not unreasonable to expect them to modify the loader if necessary.

[takahirox]

FYI, Relating thread #12718

[donmccurdy]

@rpadovani if you don't have time to make a PR that's no problem, just let us know and we'll plan this for a future release.

@takahirox yeah, I think we should use the hash key approach you mentioned there.

[rpadovani]

Hi everyone, I am still available to work on this, we have disabled the cache at the moment but I'd prefer to have a solution upstream.

I took a look to #12718, and as far as I understood this proposal would fix the problem for Draco - but as far as I see this hasn't been merged in master.

Before starting working on this I'd like to understand which properties I should check to implement the the hash: primitives, attributes, targets, extensions, and extras. Since there are so many, does really make sense to implement a custom hashing function, instead of relay on an existing library - considering also you want to change the gltf specifications to add an id attribute, so this solution is temporary?

[donmccurdy]

#12718 is really just the motivation for why we have the cache.

I do not expect an id attribute to be added to glTF any time soon — it's a significant change in the schema and there are higher priorities at the moment. We should fix this in GLTFLoader without waiting for that.

I'd suggest hashing exactly the properties that are compared by the existing function. It's not fully robust against things like unknown extensions, but reusing geometries efficiently is the more common and important need IMO. Users with custom extensions can modify the loader if necessary.

An existing hashing library would make this easier, but there's no mechanism for code in examples/js/* to add dependencies except asking the user to copy/paste a script into their build, which I don't think is worthwhile here.

[rpadovani]

I gave it a try in #15431, let me know what you think

[takahirox]

It's not fully robust against things like unknown extensions, but reusing geometries efficiently is the more common and important need IMO. Users with custom extensions can modify the loader if necessary.

I always prefer robustness. Rare case happens someday. IMO we shouldn't ignore known issues.

But yeah it's impossible for GLTFLoader to (efficiently) determine whether geometry is cachable with extras and custom extensionis because they're user defined and they're unknown for the loader.

So I think we have two options.

1. If extras or custom extensions is non-empty, we don't reuse geometry
2. If extras or custom extensions is non-empty, we display console info or warning (once) indicating we ignored them for determining whether geometry is reusable

May be 1. is pessimistic too much. Probably 2 sounds reasonable. We can avoid the worst scenario that a geometry shouldn't be reusable because of its special extras or extensions, but it's reused, and users doesn't notice. What do you think?

[donmccurdy]

If robustness is the goal, then JSON.stringify( prim1 ) === JSON.stringify( prim2 ) is about as good anything...

To me the worst-case scenario here is that we upload the same geometry to the GPU twice and users don't notice, because that's hard to catch and debug. Failing to preserve extras is much less worrying — even ignoring primitive.extras _entirely_ and telling users to rely on mesh.extras and node.extras instead would not be so bad.

For that matter, I don't think it's likely that users putting extras on a primitive intend for that geometry to be duplicated in the GPU. Perhaps they'd rather have .userData contain an array with the .extras of all of the primitives used?

Other alternatives:

1. reuse geometry as before, but log a warning when extras or extensions may have been lost.
2. Ensure that three.js can avoid uploading duplicate data if two geometries share the same BufferAttributes, so that none of this matters. :)

But all of this gets complicated, and the proposed PR has only one effect on our current implementation — it makes it faster. So i'd prefer to merge the PR and consider any changes later.

[takahirox]

Perhaps they'd rather have .userData contain an array with the .extras of all of the primitives used?

I think that works, too.

What I wanna avoid is throwing user-defined or custom extension data away without any notice. I prefer warning or .userData containing primitives' .extras array.

[mrdoob]

Does #15431 solve this?

[donmccurdy]

I think this can be closed, yes. 👍