Change cgltf_extra to copy data during parsing?

[zeux]

I'm looking into optimizing the amount of memory gltfpack spends on processing very large glTF files (e.g. 400 MB .gltf 400 MB .bin). One thing that I'd like to explore is unloading the glTF data early.

The way gltfpack works is that it parses glTF file into memory, loads the buffers, then extracts the data for meshes and animations into separate data structures, and then proceeds to optimize the scene data and output the result.

During this process, it needs to keep cgltf_data* around - I'd like to continue doing that, since a lot of complex glTF structure, especially around materials, is easier to preserve that way.

However, it can in theory unload the JSON data very early (immediately after parsing) and unload the buffer data earlier (after extracting mesh/animation/image data).

Unfortunately, it's currently impossible to unload cgltf_data::json because cgltf_extras references offsets in that blob. Right now it's the only explicit reference to JSON data, although #114 adds this for extensions as well.

Would it be reasonable to change extras storage to store char* (or a pair of char* + size_t), and copy the data out of raw JSON storage during parsing? This will slightly increase the number of memory allocations and the peak memory usage for cases where the JSON data isn't unloaded, but it would allow dropping the entire buffer before application processing. For .glb parsing we will have to keep the allocation around since it stores the binary chunk, although there's probably a way to deal with this as well.

[prideout]

Seems fine to me.

[jkuhlmann]

Would it make sense for this to be optional? I suppose in that case the char* would either point to the extra allocation or into the JSON data.

[LxLasso]

I like this idea. I have already seen files this size from our users and will likely see worse. Unless the glTF is really peppered with large extras and extensions I would rather take the overhead of the allocs and copies and reduce peak memory use in the process.

I do suspect there's an argument for optionality though, yes.

[zeux]

I'm wondering how exactly we'd make this optional, and whether it's worth it? There's some complications with deallocation - we can probably check if the data is inside the JSON blob, or maybe add a separate char* data that starts out as NULL.

Some other data like names and URIs are allocated on the heap, does it make sense to special-case extras and extensions? On average I'd expect names & uris to be more common in files.

[zeux]

Oh, two additional notes:

• For extensions, there's already an allocation for the name of an extension anyway
• For both extras and extensions, specifying offsets into the JSON blob works fine for parsing but doesn't work well for cases where you need to create or modify the GLTF file and write it after that.

So it feels to me like the best route is to just change extras & extensions to not refer to JSON contents. We can always implement a string pool if we decide to reduce the allocation count, which would help all existing strings like name & uri as well.

I'm going to work on a PR for this unless there are objections to unconditionally changing this.

[zeux]

Started working on that and I'm having second thoughts about this. Specifically the destruction code becomes extremely complex with this approach, because almost all objects now contain their own allocations. This is manageable for most objects but particularly bad for texture views because there's no neat list for those, it's just a bunch of structs inside a material that are conditional on extension presence.

You can see this in #114 which has the same problem. I'm not clear yet if that's going to get merged; if it is, this change becomes a pretty trivial change on top of that. If that change gets a different solution to this problem then maybe it can be adapted to extras.

[jkuhlmann]

Oh, yes, #114 is kind of a hefty change. I feel it's whole lot of administrative overhead for a use case that's maybe not very common. I'm kind of thinking the same thing here. What do people normally do? Just load some geometry and be done with it? That's why I was asking about making it optional - we get a quite a few more allocations and copies if we extract extras and extensions. What do you think do most people who use this library look for?

[zeux]

That's why I was asking about making it optional

Ah, maybe I misinterpreted the request earlier - if that meant making the extras loading optional this could make sense to me. Although worth noting is that in most glTF files I see, there are no extras other than the top-level asset.extras. I could see the argument for extensions.

And yeah the dominant use case probably doesn't involve reading custom extensions or extras, or worrying about memory pressure either really.

[jkuhlmann]

I meant making the copying of the extras data to a new allocation optional.

My goal here is, of course, to make everyone happy. So, my preferred way would be to have these "advanced" features optional. However, I suppose you're also right that, if you're just looking to load simpler glTF data, you'll probably not have a lot of extras/extension data anyway.

[zeux]

Yeah I feel like in terms of allocation count, names are a much bigger problem. Whether we want to address it or not is unclear. cgltf doesn’t currently parse names for a number of objects that can have them either (e.g. accessors) so the impact is not as severe as it could be.

My main hesitation with this specific issue isn’t allocation count, as extras are rare, but rather deallocation complexity. It can be minimized if we remove extras from a few objects where it’s unlikely to matter (texture views and extras) but maybe that’s too drastic? Wonder if extensions can be treated similarly where we only add them to top level objects (nodes, meshes, materials, buffer views, buffers) to begin with to simplify.

[jkuhlmann]

What if we get rid of individual deallocation altogether? We could just allocate one big chunk of memory, then linearly allocate the individual bits from that and finally deallocate only the big chunk. The only problem would be that we'd need to determine the size of the big chunk first. However, maybe that's even the solution to making the whole thing optional? So, we'd load everything like we do now, then we have a new function that goes through the loaded glTF data once to determine size required for copying extras and extension data and does the copying into this chunk of memory that we can just free later in one go.

[zeux]

That's what I ended up doing in user space for now: https://github.com/zeux/meshoptimizer/commit/91011f44c1b758412ca34f91b976ad887f10610b

However, I don't think it's the full story really. Let's say we do that - do we force the users to similarly pack the extras into a single blob when creating or changing the tree?

[jkuhlmann]

And I thought I had a good idea. 😄 How would it work in other scenarios? Does the user call the alloc function they passed into cgltf when modifying the extras data? So that cgltf_free() can freely free the memory?

[zeux]

How would it work in other scenarios? Does the user call the alloc function they passed into cgltf when modifying the extras data? So that cgltf_free() can freely free the memory?

Yeah, that's what I'd expect - I'm not sure how cgltf_write would be used otherwise.

Re: packing, I guess maybe the key question to answer is how should extensions work. If they require separate allocations for the extensions array and extension name, then allocating extension contents and extras contents seems appropriate - it's similar complexity. If they don't, then the question is how should they work?

[zeitt]

Extensions could easily be done the same way as extras. I just thought the "Dictionary" approach would be more user friendly. However, if #114 would cause "a whole lot of administrative overhead" and/or complicate other things; going with a simpler solution where extensions would be exposed the same way as extras can also be done.

[jkuhlmann]

Honestly, I'm not sure what the best choice is here. I think we should go ahead with the additional allocations. We can always then later optimize it with optional packing. Does that make sense to you?

[zeitt]

Yes, it does make sense and I should be able to convert #114 to allocate/free contents fairly easily.

[jkuhlmann]

@zeux Is the state in #114 close to the approach that would also work for your extras data?

[zeux]

@jkuhlmann Yup, I believe so!

[jkuhlmann]

FYI, #114 is merged now.