[dawn] Refactor ApiObject label to be thread-safe. This change replaces the raw std::string label in ApiObjectBase with a lazily-initialized ObjectLabel that holds a mutex-protected string. This ensures that SetLabel() and GetLabel() can be called safely from multiple threads without a device-wide lock. ApiObjectBase stores an atomic pointer to ObjectLabel (initialized once via std::once_flag on the first non-empty SetLabel call). GetLabel() returns std::string by value. Bug: 479457809 Change-Id: Ib9d14414d012bb984307fd391ba86bcff78bf820 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/297395 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Quyen Le <lehoangquyen@chromium.org> Reviewed-by: Kyle Charbonneau <kylechar@google.com>
diff --git a/include/dawn/native/DawnNative.h b/include/dawn/native/DawnNative.h index b2360d6..4ddbfd5 100644 --- a/include/dawn/native/DawnNative.h +++ b/include/dawn/native/DawnNative.h
@@ -282,7 +282,7 @@ DAWN_NATIVE_EXPORT bool CheckIsErrorForTesting(void* objectHandle); -DAWN_NATIVE_EXPORT const char* GetObjectLabelForTesting(void* objectHandle); +DAWN_NATIVE_EXPORT std::string GetObjectLabelForTesting(void* objectHandle); DAWN_NATIVE_EXPORT uint64_t GetAllocatedSizeForTesting(WGPUBuffer buffer);
diff --git a/src/dawn/native/BUILD.gn b/src/dawn/native/BUILD.gn index 933b41e..5a2f61c 100644 --- a/src/dawn/native/BUILD.gn +++ b/src/dawn/native/BUILD.gn
@@ -316,6 +316,7 @@ "ObjectBase.h", "ObjectContentHasher.cpp", "ObjectContentHasher.h", + "ObjectLabel.h", "PassResourceUsage.h", "PassResourceUsageTracker.cpp", "PassResourceUsageTracker.h",
diff --git a/src/dawn/native/CMakeLists.txt b/src/dawn/native/CMakeLists.txt index 7db4724..0ec901e 100644 --- a/src/dawn/native/CMakeLists.txt +++ b/src/dawn/native/CMakeLists.txt
@@ -109,6 +109,7 @@ "LogEmitter.h" "ObjectBase.h" "ObjectContentHasher.h" + "ObjectLabel.h" "PassResourceUsage.h" "PassResourceUsageTracker.h" "PerStage.h"
diff --git a/src/dawn/native/CreatePipelineAsyncEvent.cpp b/src/dawn/native/CreatePipelineAsyncEvent.cpp index 3fdc988..5d1e69f 100644 --- a/src/dawn/native/CreatePipelineAsyncEvent.cpp +++ b/src/dawn/native/CreatePipelineAsyncEvent.cpp
@@ -136,7 +136,8 @@ void CreatePipelineAsyncEvent<PipelineType, CreatePipelineAsyncCallbackInfo>::InitializeImpl( bool isAsync) { DeviceBase* device = mPipeline->GetDevice(); - const char* eventLabel = utils::GetLabelForTrace(mPipeline->GetLabel()); + const std::string& label = mPipeline->GetLabel(); + const char* eventLabel = utils::GetLabelForTrace(label); if (isAsync) { TRACE_EVENT_FLOW_END1(device->GetPlatform(), General, "CreatePipelineAsyncEvent::InitializeAsync", this, "label", @@ -169,7 +170,8 @@ template <typename PipelineType, typename CreatePipelineAsyncCallbackInfo> void CreatePipelineAsyncEvent<PipelineType, CreatePipelineAsyncCallbackInfo>::InitializeAsync() { DeviceBase* device = mPipeline->GetDevice(); - const char* eventLabel = utils::GetLabelForTrace(mPipeline->GetLabel()); + const std::string& label = mPipeline->GetLabel(); + const char* eventLabel = utils::GetLabelForTrace(label); TRACE_EVENT_FLOW_BEGIN1(device->GetPlatform(), General, "CreatePipelineAsyncEvent::InitializeAsync", this, "label", eventLabel); @@ -208,7 +210,8 @@ if (device->IsLost()) { // Invalid async creation should "succeed" if the device is already lost. if (!mPipeline->IsError()) { - mPipeline = PipelineType::MakeError(device.Get(), mPipeline->GetLabel().c_str()); + const std::string& label = mPipeline->GetLabel(); + mPipeline = PipelineType::MakeError(device.Get(), label.c_str()); } pipeline = std::move(mPipeline); } else if (mError != nullptr) {
diff --git a/src/dawn/native/DawnNative.cpp b/src/dawn/native/DawnNative.cpp index 216e7c1..f4406f0 100644 --- a/src/dawn/native/DawnNative.cpp +++ b/src/dawn/native/DawnNative.cpp
@@ -27,6 +27,7 @@ #include "dawn/native/DawnNative.h" +#include <string> #include <vector> #include "dawn/common/Log.h" @@ -258,9 +259,9 @@ return reinterpret_cast<ErrorMonad*>(objectHandle)->IsError(); } -const char* GetObjectLabelForTesting(void* objectHandle) { +std::string GetObjectLabelForTesting(void* objectHandle) { ApiObjectBase* object = reinterpret_cast<ApiObjectBase*>(objectHandle); - return object->GetLabel().c_str(); + return object->GetLabel(); } uint64_t GetAllocatedSizeForTesting(WGPUBuffer buffer) {
diff --git a/src/dawn/native/ObjectBase.cpp b/src/dawn/native/ObjectBase.cpp index c2504c7..66d6c72 100644 --- a/src/dawn/native/ObjectBase.cpp +++ b/src/dawn/native/ObjectBase.cpp
@@ -27,14 +27,13 @@ #include "dawn/native/ObjectBase.h" -#include <atomic> -#include <mutex> #include <utility> #include <vector> #include "absl/strings/str_format.h" #include "dawn/native/Adapter.h" #include "dawn/native/Device.h" +#include "dawn/native/ObjectLabel.h" #include "dawn/native/ObjectType_autogen.h" #include "dawn/native/utils/WGPUHelpers.h" @@ -146,23 +145,30 @@ } ApiObjectBase::ApiObjectBase(DeviceBase* device, StringView label) : ObjectBase(device) { - mLabel = std::string(label); + if (!label.IsUndefined()) { + SetLabel(std::string(label)); + } } ApiObjectBase::ApiObjectBase(DeviceBase* device, ErrorTag tag, StringView label) : ObjectBase(device, tag) { - mLabel = std::string(label); + if (!label.IsUndefined()) { + SetLabel(std::string(label)); + } } ApiObjectBase::ApiObjectBase(DeviceBase* device, DelayedInitializationTag tag, StringView label) : ObjectBase(device, tag) { - mLabel = std::string(label); + if (!label.IsUndefined()) { + SetLabel(std::string(label)); + } } ApiObjectBase::ApiObjectBase(DeviceBase* device, LabelNotImplementedTag tag) : ObjectBase(device) {} ApiObjectBase::~ApiObjectBase() { DAWN_ASSERT(!IsAlive()); + delete mLabel.load(std::memory_order_acquire); } void ApiObjectBase::APISetLabel(StringView label) { @@ -170,18 +176,35 @@ } void ApiObjectBase::SetLabel(std::string label) { - mLabel = std::move(label); + auto* labelObj = mLabel.load(std::memory_order_acquire); + if (labelObj == nullptr) { + auto* newLabel = new ObjectLabel(); + ObjectLabel* expected = nullptr; + if (mLabel.compare_exchange_strong(expected, newLabel, std::memory_order_acq_rel, + std::memory_order_acquire)) { + labelObj = newLabel; + } else { + delete newLabel; + labelObj = expected; + } + } + labelObj->SetValue(std::move(label)); SetLabelImpl(); } -const std::string& ApiObjectBase::GetLabel() const { - return mLabel; +std::string ApiObjectBase::GetLabel() const { + auto* label = mLabel.load(std::memory_order_acquire); + if (label == nullptr) { + return ""; + } + return label->GetValue(); } void ApiObjectBase::FormatLabel(absl::FormatSink* s) const { s->Append(ObjectTypeAsString(GetType())); - if (!mLabel.empty()) { - s->Append(absl::StrFormat(" \"%s\"", mLabel)); + const std::string& label = GetLabel(); + if (!label.empty()) { + s->Append(absl::StrFormat(" \"%s\"", label)); } else { s->Append(" (unlabeled)"); }
diff --git a/src/dawn/native/ObjectBase.h b/src/dawn/native/ObjectBase.h index 3e3365c..f19054f 100644 --- a/src/dawn/native/ObjectBase.h +++ b/src/dawn/native/ObjectBase.h
@@ -28,6 +28,7 @@ #ifndef SRC_DAWN_NATIVE_OBJECTBASE_H_ #define SRC_DAWN_NATIVE_OBJECTBASE_H_ +#include <atomic> #include <mutex> #include <optional> #include <string> @@ -42,6 +43,8 @@ namespace dawn::native { +class ObjectLabel; + namespace detail { template <typename T> @@ -156,7 +159,7 @@ virtual ObjectType GetType() const = 0; void SetLabel(std::string label); - const std::string& GetLabel() const; + std::string GetLabel() const; virtual void FormatLabel(absl::FormatSink* s) const; @@ -201,7 +204,7 @@ private: friend class ApiObjectList; - std::string mLabel; + std::atomic<ObjectLabel*> mLabel{nullptr}; }; template <typename T>
diff --git a/src/dawn/native/ObjectLabel.h b/src/dawn/native/ObjectLabel.h new file mode 100644 index 0000000..9ae00e9 --- /dev/null +++ b/src/dawn/native/ObjectLabel.h
@@ -0,0 +1,55 @@ +// Copyright 2026 The Dawn & Tint Authors +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this +// list of conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, +// this list of conditions and the following disclaimer in the documentation +// and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#ifndef SRC_DAWN_NATIVE_OBJECTLABEL_H_ +#define SRC_DAWN_NATIVE_OBJECTLABEL_H_ + +#include <string> +#include <utility> + +#include "dawn/common/MutexProtected.h" + +namespace dawn::native { + +// Holds a label string protected by a mutex for safe concurrent access. +class ObjectLabel final { + public: + void SetValue(std::string label) { + mLabel.Use([&label](auto guard) { *guard = std::move(label); }); + } + + std::string GetValue() const { + return mLabel.Use([](auto guard) { return std::string(*guard); }); + } + + private: + dawn::MutexProtected<std::string> mLabel; +}; + +} // namespace dawn::native + +#endif // SRC_DAWN_NATIVE_OBJECTLABEL_H_
diff --git a/src/dawn/native/webgpu/ComputePipelineWGPU.cpp b/src/dawn/native/webgpu/ComputePipelineWGPU.cpp index 70593ba..2404a79 100644 --- a/src/dawn/native/webgpu/ComputePipelineWGPU.cpp +++ b/src/dawn/native/webgpu/ComputePipelineWGPU.cpp
@@ -56,9 +56,10 @@ ObjectWGPU(device->wgpu->computePipelineRelease) {} ResultOrError<Extent3D> ComputePipeline::InitializeImpl() { + std::string label = GetLabel(); WGPUComputePipelineDescriptor desc; desc.nextInChain = nullptr; - desc.label = ToOutputStringView(GetLabel()); + desc.label = ToOutputStringView(label); const PipelineLayoutBase* layout = GetLayout(); DAWN_ASSERT(layout != nullptr); desc.layout = ToBackend(layout)->GetInnerHandle();
diff --git a/src/dawn/native/webgpu/ExternalTextureWGPU.cpp b/src/dawn/native/webgpu/ExternalTextureWGPU.cpp index 93696fa..83a77fa 100644 --- a/src/dawn/native/webgpu/ExternalTextureWGPU.cpp +++ b/src/dawn/native/webgpu/ExternalTextureWGPU.cpp
@@ -27,6 +27,8 @@ #include "dawn/native/webgpu/ExternalTextureWGPU.h" +#include <string> + #include "dawn/common/StringViewUtils.h" #include "dawn/native/webgpu/BufferWGPU.h" #include "dawn/native/webgpu/DeviceWGPU.h" @@ -66,9 +68,10 @@ RecordableObject(schema::ObjectType::ExternalTexture), ObjectWGPU(device->wgpu->externalTextureRelease), mCreationParams(descriptor) { + std::string label = GetLabel(); WGPUExternalTextureDescriptor desc = { .nextInChain = nullptr, - .label = ToOutputStringView(GetLabel()), + .label = ToOutputStringView(label), .plane0 = ToBackend(descriptor->plane0)->GetInnerHandle(), .plane1 = descriptor->plane1 ? ToBackend(descriptor->plane1)->GetInnerHandle() : nullptr, .cropOrigin = ToWGPU(descriptor->cropOrigin),
diff --git a/src/dawn/native/webgpu/PipelineLayoutWGPU.cpp b/src/dawn/native/webgpu/PipelineLayoutWGPU.cpp index bdd7735..11b1f2a 100644 --- a/src/dawn/native/webgpu/PipelineLayoutWGPU.cpp +++ b/src/dawn/native/webgpu/PipelineLayoutWGPU.cpp
@@ -28,6 +28,7 @@ #include "dawn/native/webgpu/PipelineLayoutWGPU.h" #include <array> +#include <string> #include <vector> #include "dawn/common/Constants.h" @@ -50,9 +51,10 @@ : PipelineLayoutBase(device, descriptor), RecordableObject(schema::ObjectType::PipelineLayout), ObjectWGPU(device->wgpu->pipelineLayoutRelease) { + std::string label = GetLabel(); WGPUPipelineLayoutDescriptor desc; desc.nextInChain = nullptr; - desc.label = ToOutputStringView(GetLabel()); + desc.label = ToOutputStringView(label); std::array<WGPUBindGroupLayout, kMaxBindGroups> bindGroupLayouts; bindGroupLayouts.fill(nullptr);
diff --git a/src/dawn/native/webgpu/RenderPipelineWGPU.cpp b/src/dawn/native/webgpu/RenderPipelineWGPU.cpp index 4482f64..a113fc6 100644 --- a/src/dawn/native/webgpu/RenderPipelineWGPU.cpp +++ b/src/dawn/native/webgpu/RenderPipelineWGPU.cpp
@@ -71,8 +71,9 @@ PerColorAttachment<WGPUColorTargetStateExpandResolveTextureDawn> colorTargetStateExpandResolveTextureDawnExtensions = {}; + std::string label = GetLabel(); desc.nextInChain = nullptr; - desc.label = ToOutputStringView(GetLabel()); + desc.label = ToOutputStringView(label); auto layout = GetLayout(); DAWN_ASSERT(layout != nullptr); desc.layout = ToBackend(layout)->GetInnerHandle();
diff --git a/src/dawn/native/webgpu/TextureWGPU.cpp b/src/dawn/native/webgpu/TextureWGPU.cpp index 5353a89..501568c 100644 --- a/src/dawn/native/webgpu/TextureWGPU.cpp +++ b/src/dawn/native/webgpu/TextureWGPU.cpp
@@ -76,9 +76,10 @@ viewFormats.push_back(ToAPI(device->GetValidInternalFormat(i).format)); } + std::string label = GetLabel(); WGPUTextureDescriptor desc = { .nextInChain = nullptr, - .label = ToOutputStringView(GetLabel()), + .label = ToOutputStringView(label), .usage = ToAPI(actualUsage), .dimension = ToAPI(GetDimension()), .size = ToWGPU(GetBaseSize()),