Loading gltf on background thread very slow when using "Separate" rendering thread model due to lock contention

[brycehutchings]

Tested versions

v4.5.1

System information

Godot v4.5.1.mono (eb5a059c3) - Windows 11 (build 26200) - Multi-window, 2 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4090 (NVIDIA; 32.0.15.7680) - 13th Gen Intel(R) Core(TM) i9-13900K (32 threads) - 127.7 GiB memory

Issue description

My team is working on loading user-supplied glTF models at runtime for an XR application. We are experimenting with loading the glTF model on a background thread to reduce frame stalling.

We are also experimenting with the "Separate" rendering model (despite the warnings) to understand what problems we might have.

With the "Separate" thread model, we see load times that are roughly 30x slower than if we use a background thread with "Safe" thread model. I have instrumented the Godot engine to isolate the problem and have found it is due to lock contention in CommandQueueMT. Specifically this is what I see:

• GLTFDocument's generate_scene will import each node.
  • As part of import, convert_importer_mesh_instance_3d will create MeshInstance3D objects and set their mesh.
  • Creating the MeshInstance3D object often takes 10+ milliseconds. Setting the mesh also often takes 10+ milliseconds. With "Safe" thread model these operations are instant. More on what causes this later...
  • So if there are tens of thousands of nodes (we're working with complex industrial models that can be hundreds of megabytes) we can see this take 17 minutes for one of our models. BTW, Godot does wonderfully rendering with good performance once it is loaded, and does wonderfully loading quickly if we use "Safe" thread model.
• When the "Separate" thread model is used, the RenderingServer's _draw() function will be queued on the CommandQueueMT and execute on the RenderingServer's render pump thread.
  • _draw() is very slow because it includes the swapchain synchronization. If I turn off vsync (which we do for XR), then it is "only" 2-3x slower than when using "Safe" thread model.
  • While _draw() is running (or any queued work for that matter), a mutex in the CommandQueueMT is acquired which prevents new work from being queued.

I instrumented Godot and have this timeline which shows what is happening:

I drew a box around a single node/mesh being imported and there are four places where it pushes into the CommandQueue. Each of these four places block on acquiring a mutex because the _draw() method (shown on a separate lower track in that screenshot) is being invoked out of the Command Queue which is holding that lock.

I am still new to Godot and its architecture but one possible solution comes to mind: CommandQueueMT gets smarter around locking. Perhaps it could have separate pending and flushing queues with separate locks? Flush would swap the queues atomically to minimize contention with queuing work.

Safe thread model doesn't have this problem because the _draw() command does not run via CommandQueueMT.

Steps to reproduce

Here is the loading code I use, which I invoke from a button press:

public async void _on_button_pressed()
{
    Node3D sceneNode = null!;
    await Task.Run(() =>
    {
        var state = new GltfState();
        var doc = new GltfDocument();
        Error readErr = doc.AppendFromFile("path to very large GLB file", state);
        sceneNode = (Node3D)doc.GenerateScene(state);
    });

    await ToSignal(GetTree(), SceneTree.SignalName.ProcessFrame); // hop back to main thread

    AddChild(sceneNode);
}

Minimal reproduction project (MRP)

separate-thread-model-slow.zip

Note this doesn't include the GLB file that I am testing with. The "PATH_TO_GLB_GOES_HERE.glb" line of code in node_3d.gd will need to be changed to something.

[clayjohn]

Very nice investigation!

I guess this is a special case for the CommandQueueMT implementation. It's designed to allow you to queue up work while you are processing the queue on another thread. However, the GLTF code needs values that are returned by the RenderingServer which requires a full flush and sync of the CommandQueue.

Your mention of Vsync reminds me that we had a similar issue a couple of years ago with the background thread loader that was around 30x slower than it should be, but only when vsync was enabled. Basically what was happening was the lock was being held when we started waiting on vsync, so all loading tasks would get blocked for the full vertical refresh time. I'll need to search for what we did there since a similar solution is likely needed. I may even be the case that the solution just didn't work for XR and needs to be duplicated.

[brycehutchings]

I just implemented a solution which does an atomic swap of the command_mem buffer (essentially it becomes double buffered), and it fixes the performance issue for me. For reference, this is the (only lightly tested/reviewed) change that I tried: brycehutchings@dce53a7

I guess this is a special case for the CommandQueueMT implementation. It's designed to allow you to queue up work while you are processing the queue on another thread.

I don't see it allowing work to queue up while processing the queue. If it did then there'd be no performance problem.

https://github.com/godotengine/godot/blob/master/core/templates/command_queue_mt.h#L134 Here is where the lock has to be acquired to push into the queue.

https://github.com/godotengine/godot/blob/master/core/templates/command_queue_mt.h#L162 Here is where the lock is acquired while draining the queue. So while cmd->call() is happening (such as _draw() being invoked), the lock is held.

However, the GLTF code needs values that are returned by the RenderingServer which requires a full flush and sync of the CommandQueue.

I don't think I'm seeing this, just a bunch of queued work without flush/sync.

Your mention of Vsync reminds me that we had a similar issue a couple of years ago with the background thread loader that was around 30x slower than it should be, but only when vsync was enabled. Basically what was happening was the lock was being held when we started waiting on vsync, so all loading tasks would get blocked for the full vertical refresh time. I'll need to search for what we did there since a similar solution is likely needed. I may even be the case that the solution just didn't work for XR and needs to be duplicated.

This sounds exactly like what I'm seeing here with glTF loading on a background thread.

[clayjohn]

I just implemented a solution which does an atomic swap of the command_mem buffer (essentially it becomes double buffered), and it fixes the performance issue for me. For reference, this is the (only lightly tested/reviewed) change that I tried: brycehutchings@dce53a7

Very nice. I suspect that this is exactly what we need to do.

I don't see it allowing work to queue up while processing the queue. If it did then there'd be no performance problem.

I misremembered. In my mind, we only needed to lock the mutex when NeedsSync is true.

I don't think I'm seeing this, just a bunch of queued work without flush/sync.

Looking deeper I don't see any obvious flush and syncs. I thought that creating a MeshInstance3D did a flush and sync since most rendering commands that return a value require a flush and creating a MeshInstance3D requires returning an RID for the instance. But we have a lockless API for allocating an RID, so there is no flush and sync needed.

This sounds exactly like what I'm seeing here with glTF loading on a background thread.

Here is the PR with the solution when we ran into this problem before #87590 (the changes were eventually incorporated in #90400 and merged).

Reading the description, it seems like the problem there came from locking in the RenderingDevice. So it seems like a very similar symptom, but ultimately with a different root cause.

This is possibly the same fundamental problem behind #56524 and #70325

[brycehutchings]

@clayjohn Thanks for all the feedback and context. Do you think it is worth me putting forward a pull request with my fix (after I can spend more time validating and polishing it)? The upside is it resolves the lock contention. The downside is it will result in 2x the memory for however much LocalVector<uint8_t> command_mem; uses. Will this be acceptable? This little class seems pretty central and I don't yet know the ins and outs of every use case and permutation of how Godot is being used to say if this is the right fix.

[clayjohn]

My gut feeling is that it is worth making a pull request.

Like you, I am not totally sure I understand all the potential risks involved. But I am pretty sure that we need to be able to push commands to the queue while the queue is flushing, especially if we are waiting on vsync while the queue is flushing.

If we unlocked the mutex while waiting on vsync, then maybe the problem wouldn't be as pronounced. But that sounds a lot riskier than the solution you came up with.

At any rate, we should test this more and validate that it works well. I would like to run some profiles on a complex project that makes heavy use of background loading as I suspect this might resolve some of the thread contention we see during background loading.

[RandomShaper]

Good findings indeed. I've made a PR with a different approach because I think the issue can be solved at a more fundamental level. As I say in the description, I don't have time to test or benchmark it at this moment, so I would be very grateful to get some help in that regard. The PR: #112506