From 06f55a3387b255240c8b9ffda0f1b2f4b9e4bce3 Mon Sep 17 00:00:00 2001 From: okuznetsov Date: Mon, 12 Oct 2026 20:20:59 +0100 Subject: [PATCH] layersvt: Remove DebugMarker object names on null or empty name VK_EXT_debug_utils defines a null or empty pObjectName as removing the object's name. DebugMarker stored and emitted the literal "NULL" instead, so cleared objects appeared as "NULL" in traces and were replayed under that name on every later session start. Erase the stored name instead and emit an empty name, so trace consumers treat the object as unnamed from that point on. The removal is a separate method so object destruction can reuse it. Bug: Test: new tests - DebugMarkerTests#NullOrEmptyNameRemovesNameTest Change-Id: Ia6d7df2386b911a0b50911ebc7c5e9176a6a6964 --- layersvt/debug_marker/debug_marker.cpp | 22 ++++++++++--- layersvt/debug_marker/debug_marker.h | 23 ++++++++------ layersvt/test/test_debugmarker.cpp | 44 +++++++++++++++++++------- 3 files changed, 63 insertions(+), 26 deletions(-) diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index 2857455d9c..45fa52af62 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -24,6 +24,8 @@ namespace { // The packet shares the track event sequence, whose defaults declare an incremental clock, so the // absolute timestamp must carry an explicit clock id to not be read as a delta. +// An empty object_name, emitted by RemoveDebugObjectName, means the object is unnamed from this +// packet's timestamp on. void EmitDebugObjectNamePacket(uint64_t device, int32_t type, uint64_t handle, const std::string& name) { perfetto::TrackEvent::Trace([device, type, handle, &name](perfetto::TrackEvent::TraceContext ctx) { auto packet = ctx.NewTracePacket(); @@ -94,13 +96,25 @@ void DebugMarker::PreDestroyDevice(VkDevice device, const VkAllocationCallbacks* } void DebugMarker::SetDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const char* name) { + if (name == nullptr || name[0] == '\0') { + RemoveDebugObjectName(device, type, handle); + return; + } + std::lock_guard lock(mutex_); - std::string name_str = name ? name : "NULL"; + std::string name_str = name; debug_object_names_[std::make_tuple(device, type, handle)] = DebugObjectName(device, type, handle, name_str); EmitDebugObjectNamePacket(device, type, handle, name_str); } +void DebugMarker::RemoveDebugObjectName(uint64_t device, int32_t type, uint64_t handle) { + std::lock_guard lock(mutex_); + if (debug_object_names_.erase(std::make_tuple(device, type, handle)) == 0) return; + // An empty name tells trace consumers the object is unnamed from here on. + EmitDebugObjectNamePacket(device, type, handle, std::string()); +} + void DebugMarker::EmitAllDebugMarkers() { std::lock_guard lock(mutex_); for (const auto& entry : debug_object_names_) { @@ -109,11 +123,11 @@ void DebugMarker::EmitAllDebugMarkers() { } } -bool DebugMarker::HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name) { +std::optional DebugMarker::GetDebugObjectName(uint64_t device, int32_t type, uint64_t handle) const { std::lock_guard lock(mutex_); auto it = debug_object_names_.find(std::make_tuple(device, type, handle)); - if (it == debug_object_names_.end()) return false; - return it->second.name == name; + if (it == debug_object_names_.end()) return std::nullopt; + return it->second.name; } VkResult DebugMarker::CreateEmulatedMessenger(VkInstance instance, const VkDebugUtilsMessengerCreateInfoEXT* pCreateInfo, diff --git a/layersvt/debug_marker/debug_marker.h b/layersvt/debug_marker/debug_marker.h index fc811bed88..10e26dc07c 100644 --- a/layersvt/debug_marker/debug_marker.h +++ b/layersvt/debug_marker/debug_marker.h @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -63,24 +64,26 @@ class DebugMarker : public layersvt::LayerBase { } /** - * @brief Sets or updates the name associated with a Vulkan object. - * @param device The handle of the Vulkan device that owns the object. - * @param type The type of the Vulkan object (represented as int32_t). - * @param handle The handle of the Vulkan object. - * @param name The name to associate with the object. + * Sets or updates the name associated with a Vulkan object. A null or empty name + * removes the object's name. */ void SetDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const char* name); /** - * @brief Emits all stored debug markers to the tracing system. + * Removes the name associated with a Vulkan object (no-op if unnamed). + */ + void RemoveDebugObjectName(uint64_t device, int32_t type, uint64_t handle); + + /** + * Emits all stored debug markers to the tracing system. */ void EmitAllDebugMarkers(); /** - * @brief Checks if a debug name is stored for a given object. - * @note This function is for testing only. + * Returns the name stored for a Vulkan object, or std::nullopt if the object has no name. + * For testing only. */ - bool HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name); + std::optional GetDebugObjectName(uint64_t device, int32_t type, uint64_t handle) const; /** * @brief Emulated VK_EXT_debug_utils messengers, used for instances whose downstream lacks the @@ -127,7 +130,7 @@ class DebugMarker : public layersvt::LayerBase { : vk_device(dev), object_type(type), handle(h), name(n) {} }; - std::mutex mutex_; + mutable std::mutex mutex_; /** * @brief Maps (vk_device, object_type, object_handle) to its debug name information. * Non-dispatchable handles are not guaranteed to be unique across object types or devices. diff --git a/layersvt/test/test_debugmarker.cpp b/layersvt/test/test_debugmarker.cpp index 62e11549cd..3605bf4c1b 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -60,16 +60,15 @@ TEST_F(DebugMarkerTests, CombinedTest) { // 1. Set instance name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), "MyInstance"); // 2. Override new name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), "MyInstanceRenamed"); // 3. Reset layer_test::ResetLayer(); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), std::nullopt); } TEST_F(DebugMarkerTests, ManifestTest) { @@ -160,18 +159,39 @@ TEST_F(DebugMarkerTests, PreDestroyDeviceCleanupTest) { DebugMarker::Get().SetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1"); DebugMarker::Get().SetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111), "Buffer1"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), "Buffer2"); // Destroy dev1 - should remove Buffer1 but keep Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), "Buffer2"); // Destroy dev2 - should remove Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), std::nullopt); +} + +TEST_F(DebugMarkerTests, NullOrEmptyNameRemovesNameTest) { + TEST_DESCRIPTION("Verify a null or empty object name removes the stored name instead of storing it"); + + const uint64_t device = 0x1000; + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1, "Buffer"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1), "Buffer"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1, nullptr); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1), std::nullopt); + + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2, "Image"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2), "Image"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2, ""); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2), std::nullopt); + + // Removing the name of an object that was never named leaves other names untouched. + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x3, "Other"); + DebugMarker::Get().RemoveDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x3), "Other"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4), std::nullopt); } TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { @@ -182,8 +202,8 @@ TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { DebugMarker::Get().SetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer"); DebugMarker::Get().SetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1), "Device1Buffer"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1), "Device2Buffer"); } TEST_F(DebugMarkerTests, TemplateMethodDispatchTest) { @@ -265,7 +285,7 @@ TEST_F(DebugMarkerTests, MissingDownstreamExtensionTest) { name_info.objectHandle = 0x4444; name_info.pObjectName = "Buffer"; EXPECT_EQ(set_object_name(mock_device, &name_info), VK_SUCCESS); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)mock_device, VK_OBJECT_TYPE_BUFFER, 0x4444, "Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)mock_device, VK_OBJECT_TYPE_BUFFER, 0x4444), "Buffer"); } namespace {