Memory leak when destroying GL::Mesh

[bz-next]

There may be something I am not understanding properly about using the library, but I've found that GL::Mesh doesn't seem to clear memory associated with the mesh when it is destroyed.

As a test, I wrote the following code in a loop:

    for (int i = 0; i < 1000000; ++i) {
        GL::Mesh test = MeshTools::compile(Primitives::cubeSolid());
    }

From what I can tell, when the mesh goes out of scope, it should free any memory that it is using, but this example uses around 5GiB of memory on my machine.

Update: I've also tested this by adding the above code to the primitives example and the same thing happens: a lot of persistent memory use.

[mosra]

Hi, thanks a lot for the report and the to-the-point repro case!

This is unfortunately something that recently slipped through when doing a relatively large cleanup of mesh internals in f7a6d79 (on Nov 22, 2023), and you're the first to notice. I checked with apitrace and it was the VAO not getting destroyed properly but the index/vertex buffers were, so it's probably the GL spec mandating they're kept for as long as any VAO referencing them, or something, causing the GPU memory usage endlessly growing.

In any case, this should be fixed with b1ba1f0 (in the next branch). Can you confirm? Once you do, I'll push that commit to master.

Oh, and I subscribed to your project, will be watching closely (if that's okay, haha) 👍

[bz-next]

Appears fixed, thank you for the quick response!

I first noticed this bug when re-compiling meshes for a game map with a few million vertices, it'd cause a leak of a few hundred MiB per recompile. Now, it's clear that the memory is being freed, and memory use is consistent.

Always glad to have another watcher :) Thanks for providing this super slick library!

[mosra]

Awesome, thanks for confirmation -- the commit is in master now.

For a mesh that has its contents modified very often but the layout stays the same, MeshTools::compile() (and MeshData in general) might be a bit of an overkill, a simpler approach would be to have just some Containers::Array<Vertex> that's repopulated and reuploaded to an already existing vertex / index buffer, together with adjusting setCount() on the mesh if needed.

Or, alternatively, depending on how the inputs to the map recompiling are structured and how generic they are, you could store Trade::MeshData instances for the parts and then pass them all to MeshTools::concatenate(). That won't avoid the GL object recreation per se, but could at least simplify the overall logic and make it easier to modify / extend later (for example if you'd ever need additional attributes like vertex color, or would want to pack them for smaller memory use etc.).

GL object recreation could then be avoided by creating the vertex/index buffers explicitly with this compile() overload and subsequently just calling setData() on those without creating them anew, and (assuming the data layout didn't change) calling just mesh.setCount(meshData.indexCount()) on the pre-existing mesh instead of the whole compile() operation again.

But also, if this is an operation that isn't done that often and the data size isn't too massive, the existing way is probably fine and won't hurt perf. So I'm just listing the possibilities here :) Have fun!