Introducing a CommandListLifetimeTracker to enable work submission from multiple threads - #119
Merged
Conversation
…k submission from multiple threads
mcdelaney
added a commit
to mcdelaney/NVRHI
that referenced
this pull request
May 4, 2026
The Vulkan backend never had a real implementation of the ICommandListLifetimeTracker interface added in PR NVIDIA-RTX#119; the override returned nullptr with an error message. More importantly, Queue::submit was not safe to call from multiple threads — concurrent submitters raced on m_LastSubmittedID, the wait/signal accumulator vectors, and the underlying VkQueue (which Vulkan requires to be externally synchronized). Changes here: - Queue::submit holds m_Mutex for its entire body. This serializes vk::Queue.submit calls (Vulkan VkQueue external-sync), protects the wait/signal accumulator vectors, and guarantees tracking-semaphore signal-value monotonicity. Without it, two threads observing the same m_LastSubmittedID and signaling the trackingSemaphore at the same value produces validator VUID-VkSubmitInfo-pSignalSemaphores-03242 plus undefined behavior on the queue. - addWaitSemaphore / addSignalSemaphore lock the same m_Mutex. The addWait/addSignal/submit sequence is logically a single transaction per submitter; locking each individually + locking submit makes the transaction safe across threads. - retireCommandBuffers locks the same m_Mutex so concurrent retire calls (from multiple trackers) don't race on the in-flight list. - New nvrhi::vulkan::CommandListLifetimeTracker class implements the ICommandListLifetimeTracker interface. For now the tracker delegates runGarbageCollection to the queue's existing retire path — this is sufficient to make concurrent submission correct, which is the immediate goal. A future commit can move the in-flight list to be per-tracker for per-thread retire parallelism. - Device::createCommandListLifetimeTracker now returns a real tracker instead of nullptr+error. The application-side use case is multi-threaded async work submission (e.g. an arrival-upload worker submitting a compute-queue CL while the render thread submits graphics work), where the lack of internal synchronization here previously corrupted NVRHI state and produced device-removed crashes.
mcdelaney
added a commit
to mcdelaney/NVRHI
that referenced
this pull request
May 4, 2026
Brings the Vulkan CommandListLifetimeTracker to architectural parity with the D3D12 backend's tracker (PR NVIDIA-RTX#119). Previously the Vulkan tracker delegated runGarbageCollection back to a queue-wide retire path, which left all retire work serialized on Queue::m_Mutex even when callers used distinct trackers. The D3D12 design — per-tracker in-flight list — gives multiple submitting threads independent retire paths so they don't contend. Changes: - CommandListLifetimeTracker now owns its own m_CommandBuffersInFlight list and its own mutex. push() appends; runGarbageCollection drains that list, polls the queue's tracking semaphore via vkGetSemaphoreCounterValue, returns retired CBs to the queue's shared pool, and re-splices still-in-flight entries. Two threads with separate trackers do not contend on each other's mutex. - Each Queue creates a default tracker at Device construction and exposes it as Queue::defaultLifetimeTracker. CommandLists submitted without a per-CL tracker fall back to the queue's default — matches D3D12's CommandList::executed behavior. - CommandList stores params.lifetimeTracker as m_LifetimeTracker. CommandList::executed pushes the in-flight CB to the CL's tracker if set, otherwise to the queue's default tracker. - Queue::m_LastSubmittedID, m_LastRecordingID, m_LastFinishedID become std::atomic<uint64_t>. Mutated under m_Mutex during submit / updateLastFinishedID; readable lock-free elsewhere (e.g. pollCommandList, runGarbageCollection's completion check). - Queue::m_CommandBuffersInFlight removed (replaced by per-tracker list). m_CommandBuffersPool stays on the Queue (shared free list). Queue::returnCommandBufferToPool added so trackers can return retired CBs under m_Mutex. - Queue::retireCommandBuffers and Queue::getCommandBufferInFlight removed (former replaced by tracker; latter had no callers). - Device::runGarbageCollection now iterates each queue's default tracker rather than calling Queue::retireCommandBuffers. - Device::createCommandListLifetimeTracker now passes m_Context through to the new tracker constructor so tracker can drive RTXMU cleanup itself (matching D3D12's tracker layering). Net result: per-Queue submit serialization stays (Vulkan VkQueue external-sync requirement) but per-thread retire is now lock-free across threads. A worker that submits + collects garbage on its own tracker contends with the render thread only at the brief vk::Queue.submit window inside Queue::submit, not at retire time.
mcdelaney
added a commit
to mcdelaney/NVRHI
that referenced
this pull request
May 5, 2026
Vulkan requires external synchronization on a VkQueue across all queue ops, not just vkQueueSubmit. Two related gaps in our existing fork: 1. Queue::updateTextureTileMappings called m_Queue.bindSparse outside the mutex covering submit. Sparse residency uploads on a worker thread plus a render-thread submit on the same queue would race the VkQueue, producing validator THREADING_ERROR and (with sync chained off the bind via timeline semaphore) ErrorDeviceLost. Fix: lock m_Mutex around the bindSparse call. 2. vkQueuePresentKHR is called by the application directly on the raw VkQueue handle. NVRHI's queue mutex is private, so the app has no way to serialize present with submits/binds happening on worker threads. Fix: expose the per-queue std::mutex via a new IDevice::getQueueMutex(CommandQueue) method. The application takes a std::lock_guard on this mutex when calling vkQueuePresentKHR. Together these fixes close the last VkQueue external-sync gap. With PR NVIDIA-RTX#119 (per-tracker in-flight list), per-thread retire is lock-free across threads; only the actual VkQueue interaction serializes.
|
Thanks for doing these changes, and apologies about the delay - I was on leave. The changes look OK to me. I'm not entirely sure if this is a robust solution, but it seems to work. I ported the implementation to the Vulkan backend, see #133. To make your One thing that is bothering me is the removal of |
apanteleev
pushed a commit
to apanteleev/nvrhi
that referenced
this pull request
Jun 10, 2026
…ifetimeTracker to enable work submission from multiple threads": NVIDIA-RTX#119
|
@jambuttenshaw The changes are now in Do you mind submitting a pull request to Donut-Samples with your new async compute app? I don't think I can just pull your code into our repo without explicit approval or request. Below is a small patch with the changes to make it run on Vulkan and with the latest NVRHI. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an implementation of the solution described in #113
This introduces zero breaking API changes for existing code and identical behaviour for existing applications. All new behaviour to enable thread-safe work submission is entirely opt-in. A
ICommandListLifetimeTrackeris specified when creating a command list. Work is submitted as before. The details of lifetime tracking is handled by the backend; applications are required only to regularly callrunGarbageCollectionon the lifetime tracker (as is also required with the device). Command lists that do not specify a lifetime tracker will have their lifetimes managed by the Device, as before.I have only implemented this interface in D3D12 - I am not familiar with Vulkan to implement it myself currently but I believe this interface should be achievable to implement. This interface will not be implementable in D3D11 - but NVRHI does not allow for the creation of multiple command lists in D3D11 anyway, so there is no use for it there regardless.
I have implemented an async compute example in my fork of Donut-Samples to demonstrate this interface in action.