[native] Deduplicate BindGroup usage tracking per pass In a render pass, render bundle or compute pass, rebinding the same BindGroup across draws or dispatches (e.g. with varying dynamic offsets) repeatedly walks the bind group layout and records identical whole-resource usage on every SetBindGroup call. Add a 64-bit PassTrackerID TypedInteger generated atomically by DeviceBase (GetNextPassTrackerID) and give one to each pass-level tracker (RenderPassResourceUsageTracker and ComputePassResourceUsageTracker). Track the last pass tracker a BindGroup was recorded in intrusively on BindGroupBase via an atomic<PassTrackerID> checked in ShouldAddToPassTracker() with relaxed atomics. Duplicate additions caused by concurrent use in multiple encoders are benign. - Render passes and bundles: RenderPassResourceUsageTracker::AddBindGroup skips bind groups already recorded in the pass. - Compute passes: ComputePassResourceUsageTracker::AddResourcesReferencedByBindGroup skips bind groups already referenced in the pass. Per-dispatch synchronization scopes still record every bind group since each dispatch needs its own SyncScopeResourceUsage. To keep a single, type-safe code path, the non-deduplicated walk becomes the protected SyncScopeUsageTracker::AddBindGroupUsages(), and a new DispatchResourceUsageTracker exposes it as AddBindGroup() for the per-dispatch scopes of compute passes. On Animometer (10,000 draws/frame), this reduces AddBindGroup CPU cycle cost from 7.00% to 0.71% (~220 us saved per frame) on Adreno 840. Gained 8.8% CPU FPS. With 1,000 to 10,000 dispatches per frame that rebind the same bind group with dynamic offsets, this reduces Dawn CPU instructions per frame by 4-6% on Pixel 11. Bug: b/564314926, b/570398140 Change-Id: I682bacceafa6d1de156e0b61669d8f814303d17a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/342775 Reviewed-by: Brandon Jones <bajones@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Sanjeevi Suresh <sanjeevisuresh@google.com>
diff --git a/src/dawn/native/BindGroup.h b/src/dawn/native/BindGroup.h index 6464cc6..7e4f860 100644 --- a/src/dawn/native/BindGroup.h +++ b/src/dawn/native/BindGroup.h
@@ -29,6 +29,7 @@ #define SRC_DAWN_NATIVE_BINDGROUP_H_ #include <array> +#include <atomic> #include <optional> #include <span> #include <vector> @@ -96,6 +97,16 @@ void ForEachUnverifiedBufferBindingIndex(std::function<void(BindingIndex, uint32_t)> fn) const; + // Returns true and updates the last recorded pass tracker ID if this bind group has not just + // been recorded in the pass tracker identified by `id`. + bool ShouldAddToPassTracker(PassTrackerID id) { + if (mLastPassTrackerRecordedIn.load(std::memory_order_relaxed) == id) { + return false; + } + mLastPassTrackerRecordedIn.store(id, std::memory_order_relaxed); + return true; + } + protected: // To save memory, the size of a bind group is dynamically determined and the bind group is // placement-allocated into memory big enough to hold the bind group with its @@ -131,6 +142,7 @@ Ref<BindGroupLayoutBase> mLayout; BindGroupLayoutInternalBase::BindingDataPointers mBindingData; + std::atomic<PassTrackerID> mLastPassTrackerRecordedIn = kInvalidPassTrackerID; // This vector hosts the bound external textures of the bind group of each external texture // binding entry.
diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp index 3c9396f..31a056a 100644 --- a/src/dawn/native/CommandEncoder.cpp +++ b/src/dawn/native/CommandEncoder.cpp
@@ -1468,7 +1468,7 @@ DeviceBase* device = GetDevice(); DAWN_ASSERT(device->IsLockedByCurrentThreadIfNeeded()); - RenderPassResourceUsageTracker usageTracker; + RenderPassResourceUsageTracker usageTracker(device->GetNextPassTrackerID()); bool depthReadOnly = false; bool stencilReadOnly = false;
diff --git a/src/dawn/native/ComputePassEncoder.cpp b/src/dawn/native/ComputePassEncoder.cpp index 5583c69..963dfe3 100644 --- a/src/dawn/native/ComputePassEncoder.cpp +++ b/src/dawn/native/ComputePassEncoder.cpp
@@ -151,6 +151,7 @@ CommandEncoder* commandEncoder, EncodingContext* encodingContext) : ProgrammableEncoder(device, descriptor->label, encodingContext), + mUsageTracker(device->GetNextPassTrackerID()), mCommandEncoder(commandEncoder) { GetObjectTrackingList()->Track(this); } @@ -173,6 +174,7 @@ ErrorTag errorTag, StringView label) : ProgrammableEncoder(device, encodingContext, errorTag, label), + mUsageTracker(kInvalidPassTrackerID), mCommandEncoder(commandEncoder) {} // static @@ -186,7 +188,7 @@ void ComputePassEncoder::DestroyImpl(DestroyReason reason) { mCommandBufferState.End(); - mUsageTracker = {}; + mUsageTracker = ComputePassResourceUsageTracker(kInvalidPassTrackerID); // Ensure that the pass has exited. This is done for passes only since validation requires // they exit before destruction while bundles do not. @@ -440,7 +442,7 @@ } } - SyncScopeUsageTracker scope; + DispatchResourceUsageTracker scope; mUsageTracker.AddReferencedBuffer(indirectBuffer); Ref<BufferBase> indirectBufferRef = indirectBuffer; @@ -606,7 +608,7 @@ "encoding %s.WriteTimestamp(%s, %u).", this, querySet, queryIndex); } -void ComputePassEncoder::AddDispatchSyncScope(SyncScopeUsageTracker scope) { +void ComputePassEncoder::AddDispatchSyncScope(DispatchResourceUsageTracker scope) { PipelineLayoutBase* layout = mCommandBufferState.GetPipelineLayout(); for (BindGroupIndex i : layout->GetBindGroupLayoutsMask()) { scope.AddBindGroup(mCommandBufferState.GetBindGroup(i));
diff --git a/src/dawn/native/ComputePassEncoder.h b/src/dawn/native/ComputePassEncoder.h index bf2728c..fe6bd49 100644 --- a/src/dawn/native/ComputePassEncoder.h +++ b/src/dawn/native/ComputePassEncoder.h
@@ -41,8 +41,6 @@ namespace dawn::native { -class SyncScopeUsageTracker; - class ComputePassEncoder final : public ProgrammableEncoder { public: static Ref<ComputePassEncoder> Create(DeviceBase* device, @@ -105,7 +103,7 @@ // Adds the bindgroups used for the current dispatch to the SyncScopeResourceUsage and // records it in mUsageTracker. - void AddDispatchSyncScope(SyncScopeUsageTracker scope = {}); + void AddDispatchSyncScope(DispatchResourceUsageTracker scope = {}); ComputePassResourceUsageTracker mUsageTracker; // For render and compute passes, the encoding context is borrowed from the command encoder.
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index c4bf94c..6163a11 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp
@@ -2525,6 +2525,10 @@ return PipelineCompatibilityToken(mNextPipelineCompatibilityToken++); } +PassTrackerID DeviceBase::GetNextPassTrackerID() { + return PassTrackerID(mNextPassTrackerID++); +} + const CacheKey& DeviceBase::GetCacheKey() const { return mDeviceCacheKey; }
diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h index 20c0786..f53b9df 100644 --- a/src/dawn/native/Device.h +++ b/src/dawn/native/Device.h
@@ -453,6 +453,7 @@ dawn::platform::WorkerTaskPool* GetWorkerTaskPool() const; PipelineCompatibilityToken GetNextPipelineCompatibilityToken(); + PassTrackerID GetNextPassTrackerID(); const CacheKey& GetCacheKey() const; const std::string& GetLabel() const; @@ -686,6 +687,7 @@ std::atomic_uint64_t mLazyClearCountForTesting = 0; std::atomic_uint64_t mNextPipelineCompatibilityToken; + std::atomic_uint64_t mNextPassTrackerID = 1; CombinedLimits mLimits; FeaturesSet mEnabledFeatures;
diff --git a/src/dawn/native/IntegerTypes.h b/src/dawn/native/IntegerTypes.h index e8e0c58..950372e 100644 --- a/src/dawn/native/IntegerTypes.h +++ b/src/dawn/native/IntegerTypes.h
@@ -143,6 +143,12 @@ // Used to look up additional information related to the pass, such a resource usages. using PassIndex = TypedInteger<struct PassIndexT, uint32_t>; +// A unique identifier for a pass-level resource usage tracker (the RenderPassResourceUsageTracker +// of a render pass or render bundle, or the ComputePassResourceUsageTracker of a compute pass) used +// to intrusively deduplicate BindGroup resource usage tracking within the pass. +using PassTrackerID = TypedInteger<struct PassTrackerIDT, uint64_t>; +constexpr PassTrackerID kInvalidPassTrackerID = PassTrackerID(0u); + } // namespace dawn::native #endif // SRC_DAWN_NATIVE_INTEGERTYPES_H_
diff --git a/src/dawn/native/PassResourceUsageTracker.cpp b/src/dawn/native/PassResourceUsageTracker.cpp index 6b6a6df..4d41eef 100644 --- a/src/dawn/native/PassResourceUsageTracker.cpp +++ b/src/dawn/native/PassResourceUsageTracker.cpp
@@ -121,7 +121,7 @@ DAWN_ASSERT(usages.resourceTable == nullptr); } -void SyncScopeUsageTracker::AddBindGroup(BindGroupBase* group) { +void SyncScopeUsageTracker::AddBindGroupUsages(BindGroupBase* group) { const auto* layout = group->GetLayout(); for (BindingIndex i : layout->GetBufferIndices()) { @@ -263,7 +263,12 @@ return result; } -ComputePassResourceUsageTracker::ComputePassResourceUsageTracker() = default; +void DispatchResourceUsageTracker::AddBindGroup(BindGroupBase* group) { + AddBindGroupUsages(group); +} + +ComputePassResourceUsageTracker::ComputePassResourceUsageTracker(PassTrackerID passTrackerID) + : mPassTrackerID(passTrackerID) {} ComputePassResourceUsageTracker::ComputePassResourceUsageTracker( ComputePassResourceUsageTracker&&) = default; @@ -286,6 +291,14 @@ } void ComputePassResourceUsageTracker::AddResourcesReferencedByBindGroup(BindGroupBase* group) { + DAWN_ASSERT(mPassTrackerID != kInvalidPassTrackerID); + // The referenced resources only depend on the bind group, so they only need to be added once + // per compute pass. Per-dispatch sync scopes are tracked separately. If the bind group is + // concurrently added in multiple encoders at the same time, duplicate additions are benign. + if (!group->ShouldAddToPassTracker(mPassTrackerID)) { + return; + } + const auto* layout = group->GetLayout(); for (BindingIndex i : layout->GetBufferIndices()) { @@ -309,10 +322,12 @@ } ComputePassResourceUsage ComputePassResourceUsageTracker::AcquireResourceUsage() { + mPassTrackerID = kInvalidPassTrackerID; return std::move(mUsage); } -RenderPassResourceUsageTracker::RenderPassResourceUsageTracker() = default; +RenderPassResourceUsageTracker::RenderPassResourceUsageTracker(PassTrackerID passTrackerID) + : mPassTrackerID(passTrackerID) {} RenderPassResourceUsageTracker::RenderPassResourceUsageTracker(RenderPassResourceUsageTracker&&) = default; @@ -322,6 +337,18 @@ RenderPassResourceUsageTracker& RenderPassResourceUsageTracker::operator=( RenderPassResourceUsageTracker&&) = default; +void RenderPassResourceUsageTracker::AddBindGroup(BindGroupBase* group) { + DAWN_ASSERT(mPassTrackerID != kInvalidPassTrackerID); + // Intrusive PassTrackerID tracking on BindGroupBase suffices to detect duplicates because usage + // is tracked per entire resource, and dynamic offsets are handled separately. If the bind group + // is concurrently added in multiple encoders at the same time, duplicate additions are benign. + if (!group->ShouldAddToPassTracker(mPassTrackerID)) { + return; + } + + AddBindGroupUsages(group); +} + void RenderPassResourceUsageTracker::MarkFramebufferFetchUsed() { mFramebufferFetchUsed = true; } @@ -339,6 +366,7 @@ } mQueryAvailabilities.clear(); + mPassTrackerID = kInvalidPassTrackerID; result.usesFramebufferFetch = mFramebufferFetchUsed;
diff --git a/src/dawn/native/PassResourceUsageTracker.h b/src/dawn/native/PassResourceUsageTracker.h index 19dda3e..fbcd4e5 100644 --- a/src/dawn/native/PassResourceUsageTracker.h +++ b/src/dawn/native/PassResourceUsageTracker.h
@@ -70,14 +70,16 @@ // Add all usages referenced to this tracker. void MergeResourceUsages(const SyncScopeResourceUsage& usages); - // Walks the bind groups and tracks all its resources. - void AddBindGroup(BindGroupBase* group); - void SetUsedResourceTable(ResourceTableBase* table); // Returns the per-pass usage for use by backends for APIs with explicit barriers. SyncScopeResourceUsage AcquireSyncScopeUsage(); + protected: + // Walks the bind group and tracks all its resources. Subclasses expose this through their own + // AddBindGroup() so that callers can't bypass deduplication where it is expected. + void AddBindGroupUsages(BindGroupBase* group); + private: void MergeTextureUsage(TextureBase* texture, const TextureSubresourceSyncInfo& textureSyncInfo); @@ -88,10 +90,17 @@ raw_ptr<ResourceTableBase> mUsedResourceTable = nullptr; }; +// Helper class to build the SyncScopeResourceUsage of a single dispatch in a compute pass. +class DispatchResourceUsageTracker : public SyncScopeUsageTracker { + public: + // Walks the bind group and tracks all its resources. + void AddBindGroup(BindGroupBase* group); +}; + // Helper class to build ComputePassResourceUsages class ComputePassResourceUsageTracker { public: - ComputePassResourceUsageTracker(); + explicit ComputePassResourceUsageTracker(PassTrackerID passTrackerID); ComputePassResourceUsageTracker(ComputePassResourceUsageTracker&&); ~ComputePassResourceUsageTracker(); @@ -106,17 +115,24 @@ private: ComputePassResourceUsage mUsage; + + // Unique identifier for this compute pass used by AddResourcesReferencedByBindGroup() to + // skip bind groups already referenced in this pass. + PassTrackerID mPassTrackerID = kInvalidPassTrackerID; }; // Helper class to build RenderPassResourceUsages class RenderPassResourceUsageTracker : public SyncScopeUsageTracker { public: - RenderPassResourceUsageTracker(); + explicit RenderPassResourceUsageTracker(PassTrackerID passTrackerID); RenderPassResourceUsageTracker(RenderPassResourceUsageTracker&&); ~RenderPassResourceUsageTracker(); RenderPassResourceUsageTracker& operator=(RenderPassResourceUsageTracker&&); + // Walks the bind group and tracks all its resources. + void AddBindGroup(BindGroupBase* group); + void TrackQueryAvailability(QuerySetBase* querySet, QueryIndex queryIndex); const QueryAvailabilityMap& GetQueryAvailabilityMap() const; @@ -132,6 +148,10 @@ // Tracks queries used in the render pass to validate that they aren't written twice. QueryAvailabilityMap mQueryAvailabilities; + // Unique identifier for this render pass tracker used by AddBindGroup() to intrusively + // deduplicate BindGroup usage tracking without a hash table lookup. + PassTrackerID mPassTrackerID = kInvalidPassTrackerID; + bool mFramebufferFetchUsed = false; };
diff --git a/src/dawn/native/RenderBundleEncoder.cpp b/src/dawn/native/RenderBundleEncoder.cpp index eb14ab8..961abc9 100644 --- a/src/dawn/native/RenderBundleEncoder.cpp +++ b/src/dawn/native/RenderBundleEncoder.cpp
@@ -125,6 +125,7 @@ : RenderEncoderBase(device, descriptor->label, &mBundleEncodingContext, + RenderPassResourceUsageTracker(device->GetNextPassTrackerID()), device->GetOrCreateAttachmentState(descriptor), descriptor->depthReadOnly, descriptor->stencilReadOnly),
diff --git a/src/dawn/native/RenderEncoderBase.cpp b/src/dawn/native/RenderEncoderBase.cpp index 1b8af79..d08527e 100644 --- a/src/dawn/native/RenderEncoderBase.cpp +++ b/src/dawn/native/RenderEncoderBase.cpp
@@ -48,10 +48,12 @@ RenderEncoderBase::RenderEncoderBase(DeviceBase* device, StringView label, EncodingContext* encodingContext, + RenderPassResourceUsageTracker usageTracker, Ref<AttachmentState> attachmentState, bool depthReadOnly, bool stencilReadOnly) : ProgrammableEncoder(device, label, encodingContext), + mUsageTracker(std::move(usageTracker)), mIndirectDrawMetadata(device->GetLimits()), mAttachmentState(std::move(attachmentState)), mDisableBaseVertex(device->IsToggleEnabled(Toggle::DisableBaseVertex)), @@ -65,12 +67,15 @@ ErrorTag errorTag, StringView label) : ProgrammableEncoder(device, encodingContext, errorTag, label), + mUsageTracker(kInvalidPassTrackerID), mIndirectDrawMetadata(device->GetLimits()), mDisableBaseVertex(device->IsToggleEnabled(Toggle::DisableBaseVertex)), mDisableBaseInstance(device->IsToggleEnabled(Toggle::DisableBaseInstance)) {} void RenderEncoderBase::DestroyImpl(DestroyReason reason) { - mUsageTracker = {}; + // Release all tracked usages. No more commands can be encoded after destruction, so the + // tracker is reset with an invalid pass tracker ID. + mUsageTracker = RenderPassResourceUsageTracker(kInvalidPassTrackerID); // Remove reference to the attachment state so that we don't have lingering references to // it preventing it from being uncached in the device.
diff --git a/src/dawn/native/RenderEncoderBase.h b/src/dawn/native/RenderEncoderBase.h index 4b1fcad..4179ee9 100644 --- a/src/dawn/native/RenderEncoderBase.h +++ b/src/dawn/native/RenderEncoderBase.h
@@ -44,6 +44,7 @@ RenderEncoderBase(DeviceBase* device, StringView label, EncodingContext* encodingContext, + RenderPassResourceUsageTracker usageTracker, Ref<AttachmentState> attachmentState, bool depthReadOnly, bool stencilReadOnly);
diff --git a/src/dawn/native/RenderPassEncoder.cpp b/src/dawn/native/RenderPassEncoder.cpp index 45de178..3907be6 100644 --- a/src/dawn/native/RenderPassEncoder.cpp +++ b/src/dawn/native/RenderPassEncoder.cpp
@@ -82,6 +82,7 @@ : RenderEncoderBase(device, descriptor->label, encodingContext, + std::move(usageTracker), std::move(attachmentState), depthReadOnly, stencilReadOnly), @@ -89,7 +90,6 @@ mRenderArea(renderArea), mOcclusionQuerySet(descriptor->occlusionQuerySet), mEndCallback(std::move(endCallback)) { - mUsageTracker = std::move(usageTracker); if (auto* maxDrawCountInfo = descriptor.Get<RenderPassMaxDrawCount>()) { mMaxDrawCount = maxDrawCountInfo->maxDrawCount; }
diff --git a/src/dawn/tests/unittests/validation/QueueSubmitValidationTests.cpp b/src/dawn/tests/unittests/validation/QueueSubmitValidationTests.cpp index cff348e..7d273bb 100644 --- a/src/dawn/tests/unittests/validation/QueueSubmitValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/QueueSubmitValidationTests.cpp
@@ -277,6 +277,56 @@ } } +// Test that buffers in compute pass bindgroups that are set multiple times, in the same pass and +// in passes of different command buffers, are checked in Queue::Submit validation for every +// command buffer. This covers deduplication of SetBindGroup tracking within a compute pass. +TEST_F(QueueSubmitValidationTest, SubmitWithComputeBindGroupSetInMultiplePasses) { + wgpu::Queue queue = device.GetQueue(); + + wgpu::BindGroupLayout testBGL = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Compute, wgpu::BufferBindingType::Storage}}); + + wgpu::BufferDescriptor bufDesc; + bufDesc.size = 4; + bufDesc.usage = wgpu::BufferUsage::Storage; + + for (bool destroy : {true, false}) { + wgpu::Buffer buffer = device.CreateBuffer(&bufDesc); + wgpu::BindGroup bg = utils::MakeBindGroup(device, testBGL, {{0, buffer}}); + + // Set the same bind group multiple times in a pass, including at another index. + wgpu::CommandBuffer commands0; + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); + pass.SetBindGroup(0, bg); + pass.SetBindGroup(0, bg); + pass.SetBindGroup(1, bg); + pass.End(); + commands0 = encoder.Finish(); + } + + // Set the same bind group again in a pass of another command buffer. + wgpu::CommandBuffer commands1; + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); + pass.SetBindGroup(0, bg); + pass.End(); + commands1 = encoder.Finish(); + } + + if (destroy) { + buffer.Destroy(); + ASSERT_DEVICE_ERROR(queue.Submit(1, &commands1)); + ASSERT_DEVICE_ERROR(queue.Submit(1, &commands0)); + } else { + queue.Submit(1, &commands1); + queue.Submit(1, &commands0); + } + } +} + // Test that textures in unused compute pass bindgroups are still checked for in // Queue::Submit validation. TEST_F(QueueSubmitValidationTest, SubmitWithUnusedComputeTextures) {