From ad5e1238bcfa839e85bd549e343f4d057548de29 Mon Sep 17 00:00:00 2001 From: okuznetsov Date: Mon, 26 Oct 2026 18:18:32 +0000 Subject: [PATCH] layersvt: Deduplicate DebugMarker object key fields DebugObjectName stored vk_device, object_type, and handle alongside the object name even though the map key already held the same three fields. Move those fields into a hashed DebugObjectKey struct and store names in std::unordered_map, and use a colliding handle across devices in PreDestroyDeviceCleanupTest to cover per-device cleanup on collision. Bug: Test: DebugMarkerTests#PreDestroyDeviceCleanupTest Change-Id: Iec04eb396077047c0be7df0e908041846a6a6964 --- layersvt/debug_marker/debug_marker.cpp | 20 ++++++++++---------- layersvt/debug_marker/debug_marker.h | 22 ++++++++++++++-------- layersvt/test/test_debugmarker.cpp | 8 ++++---- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index a08e5aa084..6fae093aac 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -151,12 +151,12 @@ void DebugMarker::PreDestroyInstance(VkInstance instance, const VkAllocationCall void DebugMarker::PreDestroyDevice(VkDevice device, const VkAllocationCallbacks* pAllocator) { (void)pAllocator; uint64_t dev_handle = (uint64_t)device; - std::vector removed_names; + std::vector removed_objects; { std::lock_guard lock(mutex_); for (auto it = debug_object_names_.begin(); it != debug_object_names_.end();) { - if (it->second.vk_device == dev_handle) { - removed_names.push_back(std::move(it->second)); + if (it->first.vk_device == dev_handle) { + removed_objects.push_back(it->first); it = debug_object_names_.erase(it); } else { ++it; @@ -166,7 +166,7 @@ void DebugMarker::PreDestroyDevice(VkDevice device, const VkAllocationCallbacks* // Tell trace consumers that the objects of this device are unnamed from here on. The packets are // emitted after mutex_ is released, because the session start observer takes mutex_ from the tracing // thread (EmitAllDebugMarkers). The device is still alive here, so its handles cannot be reused yet. - for (const auto& removed : removed_names) { + for (const auto& removed : removed_objects) { EmitDebugObjectNamePacket(removed.vk_device, removed.object_type, removed.handle, std::string()); } } @@ -180,13 +180,13 @@ void DebugMarker::SetDebugObjectName(uint64_t device, int32_t type, uint64_t han std::lock_guard lock(mutex_); std::string name_str = name; - debug_object_names_[std::make_tuple(device, type, handle)] = DebugObjectName(device, type, handle, name_str); + debug_object_names_[{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; + if (debug_object_names_.erase({device, type, handle}) == 0) return; // An empty name tells trace consumers the object is unnamed from here on. EmitDebugObjectNamePacket(device, type, handle, std::string()); } @@ -194,16 +194,16 @@ void DebugMarker::RemoveDebugObjectName(uint64_t device, int32_t type, uint64_t void DebugMarker::EmitAllDebugMarkers() { std::lock_guard lock(mutex_); for (const auto& entry : debug_object_names_) { - const auto& marker = entry.second; - EmitDebugObjectNamePacket(marker.vk_device, marker.object_type, marker.handle, marker.name); + const DebugObjectKey& key = entry.first; + EmitDebugObjectNamePacket(key.vk_device, key.object_type, key.handle, entry.second); } } 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)); + auto it = debug_object_names_.find({device, type, handle}); if (it == debug_object_names_.end()) return std::nullopt; - return it->second.name; + return it->second; } 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 10e26dc07c..de30fe259c 100644 --- a/layersvt/debug_marker/debug_marker.h +++ b/layersvt/debug_marker/debug_marker.h @@ -22,7 +22,7 @@ #include #include #include -#include +#include #include #include "common/layer_base.h" @@ -119,23 +119,29 @@ class DebugMarker : public layersvt::LayerBase { PFN_vkVoidFunction GetLayerDeviceCommand(VkDevice device, const char* name) override; private: - struct DebugObjectName { + struct DebugObjectKey { uint64_t vk_device; int32_t object_type; uint64_t handle; - std::string name; - DebugObjectName() = default; - DebugObjectName(uint64_t dev, int32_t type, uint64_t h, const std::string& n) - : vk_device(dev), object_type(type), handle(h), name(n) {} + bool operator==(const DebugObjectKey& other) const = default; + + struct Hash { + size_t operator()(const DebugObjectKey& key) const noexcept { + size_t h1 = std::hash{}(key.vk_device); + size_t h2 = std::hash{}(key.object_type); + size_t h3 = std::hash{}(key.handle); + return h1 ^ (h2 << 1) ^ (h3 << 2); + } + }; }; mutable std::mutex mutex_; /** - * @brief Maps (vk_device, object_type, object_handle) to its debug name information. + * @brief Maps (vk_device, object_type, object_handle) to its debug name. * Non-dispatchable handles are not guaranteed to be unique across object types or devices. */ - std::map, DebugObjectName> debug_object_names_; + std::unordered_map debug_object_names_; struct EmulatedMessenger { VkInstance instance; diff --git a/layersvt/test/test_debugmarker.cpp b/layersvt/test/test_debugmarker.cpp index 23a8f413a9..b68fbf9746 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -159,20 +159,20 @@ 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"); + DebugMarker::Get().SetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x1111, "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"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x1111), "Buffer2"); // Destroy dev1 - should remove Buffer1 but keep Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); 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"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x1111), "Buffer2"); // Destroy dev2 - should remove Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); - EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x1111), std::nullopt); } TEST_F(DebugMarkerTests, NullOrEmptyNameRemovesNameTest) {