From 0bcd87373748159fdcaa575fc5bf485b34b609cf Mon Sep 17 00:00:00 2001 From: Jim Blackler Date: Tue, 27 Oct 2026 15:22:51 +0000 Subject: [PATCH 1/3] device_memory_report: clamp bound resource sizes to backing allocation Clamp bound suballocation sizes and BIND trace events to the backing VkDeviceMemory allocation size both at bind time and when the allocation size arrives later via VK_EXT_device_memory_report or vkAllocateMemory. Bindings recorded against a placeholder that extend past the allocation are unbound and re-bound through the regular bind path once the size is known, after the CREATE event, so trace consumers see a balancing DESTROY followed by the clamped BIND. Stored bindings now always lie within their allocation; assert this when computing the unbound counter while still clamping in release builds. BUG=b/578430447 --- .../device_memory_report.cpp | 100 +++++++++++++++--- .../device_memory_report.h | 16 ++- layersvt/test/test_devicememoryreport.cpp | 18 ++++ 3 files changed, 118 insertions(+), 16 deletions(-) diff --git a/layersvt/device_memory_report/device_memory_report.cpp b/layersvt/device_memory_report/device_memory_report.cpp index b59dfc98bd..4ba5ed3df5 100644 --- a/layersvt/device_memory_report/device_memory_report.cpp +++ b/layersvt/device_memory_report/device_memory_report.cpp @@ -17,10 +17,20 @@ #include "device_memory_report_perfetto.h" #include "perfetto/perfetto.h" #include +#include #include #include #include +namespace { + +// Returns the number of bytes of [offset, offset + size) that lie within an allocation of total_size bytes. +VkDeviceSize ClampToAllocation(VkDeviceSize offset, VkDeviceSize size, VkDeviceSize total_size) { + return offset < total_size ? std::min(size, total_size - offset) : 0; +} + +} // namespace + DeviceMemoryReport& DeviceMemoryReport::Get() { static DeviceMemoryReport instance; return instance; @@ -182,12 +192,14 @@ void DeviceMemoryReport::UpdateAllocationUnboundCounter(const AllocationKey& key std::vector> intervals; intervals.reserve(allocation.sub_allocations.size()); for (const auto& suballocation : allocation.sub_allocations) { - // Only the part of a binding that lies within the allocation is bound memory of it; an - // out-of-range binding must not hide unbound bytes. Clamping here, not at bind time, means - // bindings recorded against a placeholder (total_size == 0) are counted once the size is known. - if (suballocation.size > 0 && suballocation.offset < allocation.total_size) { - VkDeviceSize end = suballocation.offset + std::min(suballocation.size, allocation.total_size - suballocation.offset); - intervals.emplace_back(suballocation.offset, end); + // Sub-allocations are clamped to the allocation when bound and by ClampSubAllocationsToAllocation + // once a placeholder's size is known, so a stored binding never extends past total_size. + assert(allocation.total_size == 0 || suballocation.offset + suballocation.size <= allocation.total_size); + // Clamp anyway in release builds: a binding past the end would otherwise make the unsigned + // unbound size below wrap around. + const VkDeviceSize bound = ClampToAllocation(suballocation.offset, suballocation.size, allocation.total_size); + if (bound > 0) { + intervals.emplace_back(suballocation.offset, suballocation.offset + bound); } } if (!intervals.empty()) { @@ -286,7 +298,7 @@ void EmitDebugObjectName(VkObjectType object_type, uint64_t object_handle, std:: } // namespace -void DeviceMemoryReport::RemoveResourceBinding(const ObjectKey& resource_key, Resource& resource) { +void DeviceMemoryReport::RemoveResourceBinding(const ObjectKey& resource_key, Resource& resource, bool update_unbound_counter) { if (resource.bound_memory == 0) return; const uint64_t memory_handle = resource.bound_memory; @@ -309,7 +321,9 @@ void DeviceMemoryReport::RemoveResourceBinding(const ObjectKey& resource_key, Re VkDeviceSize suballocation_offset = it->offset; SubtractCounterBytes(it->usage_track, suballocation_size); suballocations.erase(it); - UpdateAllocationUnboundCounter(AllocationKey::App(memory_handle)); + if (update_unbound_counter) { + UpdateAllocationUnboundCounter(AllocationKey::App(memory_handle)); + } EmitAllocationTraceEvent({ .operation = "DESTROY", @@ -334,24 +348,35 @@ void DeviceMemoryReport::BindResourceMemory(VkObjectType object_type, uint64_t r // If the same resource handle is passed more than once, remove stale bindings first. RemoveResourceBinding(resource_key, res_it->second); + BindResourceMemoryLocked(resource_key, res_it->second, memory_handle, memory_offset); +} +void DeviceMemoryReport::BindResourceMemoryLocked(const ObjectKey& resource_key, Resource& resource, uint64_t memory_handle, VkDeviceSize memory_offset) { + const auto& [object_type, resource_handle] = resource_key; const AllocationKey key = AllocationKey::App(memory_handle); auto& allocation = memory_allocations_[key]; - VkDeviceSize res_size = res_it->second.size; - std::string new_usage_track = GetUsageTrackName(allocation.is_driver, res_it->second.GetCluster(allocation.mem_flags)); + VkDeviceSize res_size = resource.size; + // Clamp the bound resource size to the backing VkDeviceMemory allocation when its size is known, + // so drivers returning a worst-case requirement (e.g. for imported AHardwareBuffers) or out-of-range + // bindings do not inflate usage counters or snapshot BIND records beyond the physical allocation (b/578430447). + if (allocation.total_size > 0) { + res_size = ClampToAllocation(memory_offset, res_size, allocation.total_size); + if (res_size == 0) return; + } + std::string new_usage_track = GetUsageTrackName(allocation.is_driver, resource.GetCluster(allocation.mem_flags)); // Suballocations represent individual resources (like buffers or images) that are bound // to specific offset regions within a single large memory allocation. // We add a record here to track this specific resource's footprint within the larger memory block. allocation.sub_allocations.push_back({ object_type, resource_handle, memory_offset, res_size, new_usage_track }); - res_it->second.bound_memory = memory_handle; + resource.bound_memory = memory_handle; - // Each distinct virtual resource handle adds its virtual size to its specific category track upon binding. + // Each distinct virtual resource handle adds its clamped size to its specific category track upon binding. AddCounterBytes(new_usage_track, res_size); UpdateAllocationUnboundCounter(key); bool is_image = (object_type == VK_OBJECT_TYPE_IMAGE); - const char* cluster_name = res_it->second.GetCluster(allocation.mem_flags); + const char* cluster_name = resource.GetCluster(allocation.mem_flags); EmitAllocationTraceEvent({ .operation = "BIND", .source = is_image ? "IMAGE" : "BUFFER", @@ -364,6 +389,37 @@ void DeviceMemoryReport::BindResourceMemory(VkObjectType object_type, uint64_t r }); } +void DeviceMemoryReport::ClampSubAllocationsToAllocation(const AllocationKey& key) { + auto allocation_it = memory_allocations_.find(key); + if (allocation_it == memory_allocations_.end() || allocation_it->second.total_size == 0) return; + const MemoryAllocation& allocation = allocation_it->second; + + struct Overhang { + ObjectKey resource_key; + Resource* resource; + VkDeviceSize offset; + }; + std::vector overhangs; + for (const auto& suballocation : allocation.sub_allocations) { + if (ClampToAllocation(suballocation.offset, suballocation.size, allocation.total_size) == suballocation.size) continue; + const ObjectKey resource_key = {suballocation.object_type, suballocation.resource_handle}; + auto res_it = resources_.find(resource_key); + // Resources are unbound before they are erased, so every binding has a tracked resource. + assert(res_it != resources_.end()); + if (res_it != resources_.end()) { + overhangs.push_back({resource_key, &res_it->second, suballocation.offset}); + } + } + // Unbind every overhanging resource before re-binding any, so the unbound counter is only + // recomputed once all remaining bindings lie within the allocation. + for (const auto& overhang : overhangs) { + RemoveResourceBinding(overhang.resource_key, *overhang.resource, /*update_unbound_counter=*/false); + } + for (const auto& overhang : overhangs) { + BindResourceMemoryLocked(overhang.resource_key, *overhang.resource, key.id, overhang.offset); + } +} + void DeviceMemoryReport::OnBindBufferMemory(uint64_t buffer_handle, uint64_t memory_handle, VkDeviceSize memory_offset) { BindResourceMemory(VK_OBJECT_TYPE_BUFFER, buffer_handle, memory_handle, memory_offset); } @@ -665,7 +721,10 @@ void DeviceMemoryReport::OnMemoryReportEvent(const VkDeviceMemoryReportCallbackD allocation.is_driver = is_driver; allocation.object_type = pCallbackData->objectType; allocation.object_handle = pCallbackData->objectHandle; - UpdateAllocationUnboundCounter(key); + if (is_driver) { + // Also resolves cluster_name for the CREATE event. App allocations are updated below. + UpdateAllocationUnboundCounter(key); + } operation_name = "CREATE"; memory_type = is_driver ? allocation.cluster_name : "unbound_memory"; } else if (pCallbackData->type == VK_DEVICE_MEMORY_REPORT_EVENT_TYPE_FREE_EXT || @@ -694,6 +753,14 @@ void DeviceMemoryReport::OnMemoryReportEvent(const VkDeviceMemoryReportCallbackD }); } + if (!is_driver && (pCallbackData->type == VK_DEVICE_MEMORY_REPORT_EVENT_TYPE_ALLOCATE_EXT || + pCallbackData->type == VK_DEVICE_MEMORY_REPORT_EVENT_TYPE_IMPORT_EXT)) { + // Bindings recorded against a placeholder before its size was known are clamped after CREATE, + // so the corrective DESTROY/BIND events follow it in the trace. + ClampSubAllocationsToAllocation(key); + UpdateAllocationUnboundCounter(key); + } + const bool is_free = pCallbackData->type == VK_DEVICE_MEMORY_REPORT_EVENT_TYPE_FREE_EXT || pCallbackData->type == VK_DEVICE_MEMORY_REPORT_EVENT_TYPE_UNIMPORT_EXT; if (is_free && !is_driver && @@ -732,7 +799,6 @@ void DeviceMemoryReport::OnAllocateMemory(VkDevice device, VkDeviceMemory memory allocation.is_driver = false; allocation.object_type = VK_OBJECT_TYPE_DEVICE_MEMORY; allocation.object_handle = handle; - UpdateAllocationUnboundCounter(key); EmitAllocationTraceEvent({ .operation = "CREATE", @@ -744,6 +810,10 @@ void DeviceMemoryReport::OnAllocateMemory(VkDevice device, VkDeviceMemory memory .object_handle = handle, .memory_type = "unbound_memory", }); + + // Clamp bindings recorded against a placeholder after CREATE (see OnMemoryReportEvent). + ClampSubAllocationsToAllocation(key); + UpdateAllocationUnboundCounter(key); } } diff --git a/layersvt/device_memory_report/device_memory_report.h b/layersvt/device_memory_report/device_memory_report.h index faf06dd267..442f343abd 100644 --- a/layersvt/device_memory_report/device_memory_report.h +++ b/layersvt/device_memory_report/device_memory_report.h @@ -335,6 +335,19 @@ class DeviceMemoryReport { */ void BindResourceMemory(VkObjectType object_type, uint64_t resource_handle, uint64_t memory_handle, VkDeviceSize memory_offset); + /** + * @brief Records a binding of an already-unbound resource; caller must hold counter_mutex_. + * The bound size is clamped to the allocation when its size is known, and a binding that lies + * entirely outside the allocation is dropped. + */ + void BindResourceMemoryLocked(const ObjectKey& resource_key, Resource& resource, uint64_t memory_handle, VkDeviceSize memory_offset); + + /** + * @brief Re-binds sub-allocations recorded against a placeholder (before total_size was known) + * that extend past the allocation, so their sizes, counters, and trace events are clamped to it. + */ + void ClampSubAllocationsToAllocation(const AllocationKey& key); + /** * @brief Updates unbound memory category counter for a physical memory slab. * Unbound memory is allocated device memory not currently bound to any active resource (e.g. buffer or image). @@ -343,8 +356,9 @@ class DeviceMemoryReport { /** * @brief Removes a sub-allocation of a resource from a physical memory slab if bound. + * @param update_unbound_counter Whether to recompute the slab's unbound counter afterwards. */ - void RemoveResourceBinding(const ObjectKey& resource_key, Resource& resource); + void RemoveResourceBinding(const ObjectKey& resource_key, Resource& resource, bool update_unbound_counter = true); /** * @brief Removes all sub-allocations and tracking for a memory slab being freed. diff --git a/layersvt/test/test_devicememoryreport.cpp b/layersvt/test/test_devicememoryreport.cpp index fa673c5cf8..8429391422 100644 --- a/layersvt/test/test_devicememoryreport.cpp +++ b/layersvt/test/test_devicememoryreport.cpp @@ -370,6 +370,7 @@ TEST_F(DeviceMemoryReportTests, BindingsAreClampedToTheirAllocation) { InitializeDeviceMemoryReportPerfetto(); const std::string unbound_track = "vulkan.mem.app.usage.unbound_memory"; + const std::string mesh_track = "vulkan.mem.app.usage.geometry_mesh"; VkDeviceMemoryReportCallbackDataEXT cb_data = {}; cb_data.sType = VK_STRUCTURE_TYPE_DEVICE_MEMORY_REPORT_CALLBACK_DATA_EXT; @@ -385,21 +386,38 @@ TEST_F(DeviceMemoryReportTests, BindingsAreClampedToTheirAllocation) { DeviceMemoryReport::Get().OnCreateBuffer(0xE101, VK_BUFFER_USAGE_VERTEX_BUFFER_BIT, 8000); DeviceMemoryReport::Get().OnBindBufferMemory(0xE101, 0xE001, 12000); EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes(unbound_track), 12000u); + EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes(mesh_track), 4000u); + auto first_alloc = DeviceMemoryReportTestPeer::FindAllocation(0xE001); + ASSERT_TRUE(first_alloc.has_value()); + ASSERT_EQ(first_alloc->sub_allocations.size(), 1u); + EXPECT_EQ(first_alloc->sub_allocations[0].size, 4000u); // [20000, 21000) lies entirely outside the allocation, so nothing more is bound. DeviceMemoryReport::Get().OnCreateBuffer(0xE102, VK_BUFFER_USAGE_VERTEX_BUFFER_BIT, 1000); DeviceMemoryReport::Get().OnBindBufferMemory(0xE102, 0xE001, 20000); EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes(unbound_track), 12000u); + EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes(mesh_track), 4000u); + first_alloc = DeviceMemoryReportTestPeer::FindAllocation(0xE001); + ASSERT_TRUE(first_alloc.has_value()); + EXPECT_EQ(first_alloc->sub_allocations.size(), 1u); // A binding recorded before its allocation is reported is clamped once the allocation size is known. DeviceMemoryReport::Get().OnCreateBuffer(0xE103, VK_BUFFER_USAGE_VERTEX_BUFFER_BIT, 4000); DeviceMemoryReport::Get().OnBindBufferMemory(0xE103, 0xE002, 8000); + DeviceMemoryReport::Get().OnCreateBuffer(0xE104, VK_BUFFER_USAGE_VERTEX_BUFFER_BIT, 1000); + DeviceMemoryReport::Get().OnBindBufferMemory(0xE104, 0xE002, 12000); cb_data.memoryObjectId = 0x7001; cb_data.objectHandle = 0xE002; cb_data.size = 10000; DeviceMemoryReport::MemoryReportCallback(&cb_data, nullptr); // 12,000 B from the first allocation plus 10,000 - 2,000 B from the second. EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes(unbound_track), 20000u); + // 4,000 B from the first allocation plus 2,000 B from the clamped [8000, 10000) binding; [12000, 13000) is dropped. + EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes(mesh_track), 6000u); + auto second_alloc = DeviceMemoryReportTestPeer::FindAllocation(0xE002); + ASSERT_TRUE(second_alloc.has_value()); + ASSERT_EQ(second_alloc->sub_allocations.size(), 1u); + EXPECT_EQ(second_alloc->sub_allocations[0].size, 2000u); } TEST_F(DeviceMemoryReportTests, ClusterClassificationFunctions) { From f279a8b0b1130b93c2fe38d9508150861ae19cfc Mon Sep 17 00:00:00 2001 From: Jim Blackler Date: Tue, 27 Oct 2026 15:23:41 +0000 Subject: [PATCH 2/3] device_memory_report: query fallback resource sizes after vkBind*Memory Route vkBindBufferMemory and vkBindImageMemory through the same RecordBufferBindings / RecordImageBinds helpers as vkBindBufferMemory2 and vkBindImageMemory2, using a single-element bind info after the downstream bind succeeds. The fallback vkGet*MemoryRequirements query for resources with an unknown size now runs only after binding, and the query-after-bind ordering lives in one place per resource type. BUG=b/578430447 --- ...vice_memory_report_handwritten_functions.h | 66 ++++++++----------- .../test/test_devicememoryreport_dispatch.cpp | 52 ++++++++++++--- 2 files changed, 70 insertions(+), 48 deletions(-) diff --git a/layersvt/device_memory_report/device_memory_report_handwritten_functions.h b/layersvt/device_memory_report/device_memory_report_handwritten_functions.h index a666078fa4..114f74aed2 100644 --- a/layersvt/device_memory_report/device_memory_report_handwritten_functions.h +++ b/layersvt/device_memory_report/device_memory_report_handwritten_functions.h @@ -359,46 +359,6 @@ EXPORT_FUNCTION VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceExtensionPropert } -// Intercept memory binding to correlate buffer object handles with device memory allocations. -VKAPI_ATTR VkResult VKAPI_CALL vkBindBufferMemory(VkDevice device, VkBuffer buffer, VkDeviceMemory memory, VkDeviceSize memoryOffset) { - auto* table = device_dispatch_table(device); - assert(table->BindBufferMemory != nullptr); - assert(buffer != VK_NULL_HANDLE); - assert(memory != VK_NULL_HANDLE); - if (DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_BUFFER, reinterpret_cast(buffer)) == 0) { - if (table->GetBufferMemoryRequirements) { - VkMemoryRequirements mem_reqs; - table->GetBufferMemoryRequirements(device, buffer, &mem_reqs); - DeviceMemoryReport::Get().OnRecordResourceSize(VK_OBJECT_TYPE_BUFFER, reinterpret_cast(buffer), mem_reqs.size); - } - } - VkResult result = table->BindBufferMemory(device, buffer, memory, memoryOffset); - if (result == VK_SUCCESS) { - DeviceMemoryReport::Get().OnBindBufferMemory(reinterpret_cast(buffer), reinterpret_cast(memory), memoryOffset); - } - return result; -} - -// Intercept memory binding to correlate image object handles with device memory allocations. -VKAPI_ATTR VkResult VKAPI_CALL vkBindImageMemory(VkDevice device, VkImage image, VkDeviceMemory memory, VkDeviceSize memoryOffset) { - auto* table = device_dispatch_table(device); - assert(table->BindImageMemory != nullptr); - assert(image != VK_NULL_HANDLE); - assert(memory != VK_NULL_HANDLE); - if (DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_IMAGE, reinterpret_cast(image)) == 0) { - if (table->GetImageMemoryRequirements) { - VkMemoryRequirements mem_reqs; - table->GetImageMemoryRequirements(device, image, &mem_reqs); - DeviceMemoryReport::Get().OnRecordResourceSize(VK_OBJECT_TYPE_IMAGE, reinterpret_cast(image), mem_reqs.size); - } - } - VkResult result = table->BindImageMemory(device, image, memory, memoryOffset); - if (result == VK_SUCCESS) { - DeviceMemoryReport::Get().OnBindImageMemory(reinterpret_cast(image), reinterpret_cast(memory), memoryOffset); - } - return result; -} - static void RecordBufferBindings(VkDevice device, uint32_t bindInfoCount, const VkBindBufferMemoryInfo* pBindInfos) { auto* table = device_dispatch_table(device); for (uint32_t i = 0; i < bindInfoCount; ++i) { @@ -413,6 +373,19 @@ static void RecordBufferBindings(VkDevice device, uint32_t bindInfoCount, const } } +// Intercept memory binding to correlate buffer object handles with device memory allocations. +VKAPI_ATTR VkResult VKAPI_CALL vkBindBufferMemory(VkDevice device, VkBuffer buffer, VkDeviceMemory memory, VkDeviceSize memoryOffset) { + auto* table = device_dispatch_table(device); + assert(table->BindBufferMemory != nullptr); + VkResult result = table->BindBufferMemory(device, buffer, memory, memoryOffset); + if (result == VK_SUCCESS) { + const VkBindBufferMemoryInfo bind_info = { + .sType = VK_STRUCTURE_TYPE_BIND_BUFFER_MEMORY_INFO, .buffer = buffer, .memory = memory, .memoryOffset = memoryOffset}; + RecordBufferBindings(device, 1, &bind_info); + } + return result; +} + // Intercept memory binding via vkBindBufferMemory2 to correlate buffer object handles with device memory allocations. VKAPI_ATTR VkResult VKAPI_CALL vkBindBufferMemory2(VkDevice device, uint32_t bindInfoCount, const VkBindBufferMemoryInfo* pBindInfos) { auto* table = device_dispatch_table(device); @@ -459,6 +432,19 @@ static void RecordImageBinds(VkDevice device, uint32_t bindInfoCount, const VkBi } } +// Intercept memory binding to correlate image object handles with device memory allocations. +VKAPI_ATTR VkResult VKAPI_CALL vkBindImageMemory(VkDevice device, VkImage image, VkDeviceMemory memory, VkDeviceSize memoryOffset) { + auto* table = device_dispatch_table(device); + assert(table->BindImageMemory != nullptr); + VkResult result = table->BindImageMemory(device, image, memory, memoryOffset); + if (result == VK_SUCCESS) { + const VkBindImageMemoryInfo bind_info = { + .sType = VK_STRUCTURE_TYPE_BIND_IMAGE_MEMORY_INFO, .image = image, .memory = memory, .memoryOffset = memoryOffset}; + RecordImageBinds(device, 1, &bind_info); + } + return result; +} + // Intercept memory binding via vkBindImageMemory2 to correlate image object handles with device memory allocations. VKAPI_ATTR VkResult VKAPI_CALL vkBindImageMemory2(VkDevice device, uint32_t bindInfoCount, const VkBindImageMemoryInfo* pBindInfos) { auto* table = device_dispatch_table(device); diff --git a/layersvt/test/test_devicememoryreport_dispatch.cpp b/layersvt/test/test_devicememoryreport_dispatch.cpp index 008f6361c3..42fd13229f 100644 --- a/layersvt/test/test_devicememoryreport_dispatch.cpp +++ b/layersvt/test/test_devicememoryreport_dispatch.cpp @@ -30,6 +30,7 @@ #include #include +#include namespace { @@ -41,6 +42,13 @@ VkDeviceSize g_image_requirements_size = 0; int g_buffer_requirements_queries = 0; int g_image_requirements_queries = 0; +// Tracks whether the queried buffer/image had already been bound in the stub driver when +// vkGet*MemoryRequirements was invoked. +std::unordered_set g_bound_buffers; +std::unordered_set g_bound_images; +bool g_buffer_bound_at_last_requirements_query = false; +bool g_image_bound_at_last_requirements_query = false; + // Controls whether the stub driver implements VK_EXT_debug_utils / VK_EXT_debug_marker naming. bool g_stub_supports_debug_utils = false; bool g_stub_supports_debug_marker = false; @@ -66,7 +74,9 @@ VKAPI_ATTR VkResult VKAPI_CALL StubCreateBuffer(VkDevice, const VkBufferCreateIn return VK_SUCCESS; } -VKAPI_ATTR void VKAPI_CALL StubDestroyBuffer(VkDevice, VkBuffer, const VkAllocationCallbacks*) {} +VKAPI_ATTR void VKAPI_CALL StubDestroyBuffer(VkDevice, VkBuffer buffer, const VkAllocationCallbacks*) { + g_bound_buffers.erase(buffer); +} VKAPI_ATTR VkResult VKAPI_CALL StubCreateImage(VkDevice, const VkImageCreateInfo*, const VkAllocationCallbacks*, VkImage* pImage) { if (pImage != nullptr) { @@ -75,7 +85,9 @@ VKAPI_ATTR VkResult VKAPI_CALL StubCreateImage(VkDevice, const VkImageCreateInfo return VK_SUCCESS; } -VKAPI_ATTR void VKAPI_CALL StubDestroyImage(VkDevice, VkImage, const VkAllocationCallbacks*) {} +VKAPI_ATTR void VKAPI_CALL StubDestroyImage(VkDevice, VkImage image, const VkAllocationCallbacks*) { + g_bound_images.erase(image); +} VKAPI_ATTR VkResult VKAPI_CALL StubAllocateMemory(VkDevice, const VkMemoryAllocateInfo*, const VkAllocationCallbacks*, VkDeviceMemory* pMemory) { if (pMemory != nullptr) { @@ -86,23 +98,41 @@ VKAPI_ATTR VkResult VKAPI_CALL StubAllocateMemory(VkDevice, const VkMemoryAlloca VKAPI_ATTR void VKAPI_CALL StubFreeMemory(VkDevice, VkDeviceMemory, const VkAllocationCallbacks*) {} -VKAPI_ATTR VkResult VKAPI_CALL StubBindBufferMemory(VkDevice, VkBuffer, VkDeviceMemory, VkDeviceSize) { return VK_SUCCESS; } +VKAPI_ATTR VkResult VKAPI_CALL StubBindBufferMemory(VkDevice, VkBuffer buffer, VkDeviceMemory, VkDeviceSize) { + g_bound_buffers.insert(buffer); + return VK_SUCCESS; +} -VKAPI_ATTR VkResult VKAPI_CALL StubBindImageMemory(VkDevice, VkImage, VkDeviceMemory, VkDeviceSize) { return VK_SUCCESS; } +VKAPI_ATTR VkResult VKAPI_CALL StubBindImageMemory(VkDevice, VkImage image, VkDeviceMemory, VkDeviceSize) { + g_bound_images.insert(image); + return VK_SUCCESS; +} -VKAPI_ATTR VkResult VKAPI_CALL StubBindBufferMemory2(VkDevice, uint32_t, const VkBindBufferMemoryInfo*) { return VK_SUCCESS; } +VKAPI_ATTR VkResult VKAPI_CALL StubBindBufferMemory2(VkDevice, uint32_t bindInfoCount, const VkBindBufferMemoryInfo* pBindInfos) { + for (uint32_t i = 0; i < bindInfoCount && pBindInfos != nullptr; ++i) { + g_bound_buffers.insert(pBindInfos[i].buffer); + } + return VK_SUCCESS; +} -VKAPI_ATTR VkResult VKAPI_CALL StubBindImageMemory2(VkDevice, uint32_t, const VkBindImageMemoryInfo*) { return VK_SUCCESS; } +VKAPI_ATTR VkResult VKAPI_CALL StubBindImageMemory2(VkDevice, uint32_t bindInfoCount, const VkBindImageMemoryInfo* pBindInfos) { + for (uint32_t i = 0; i < bindInfoCount && pBindInfos != nullptr; ++i) { + g_bound_images.insert(pBindInfos[i].image); + } + return VK_SUCCESS; +} -VKAPI_ATTR void VKAPI_CALL StubGetBufferMemoryRequirements(VkDevice, VkBuffer, VkMemoryRequirements* pMemoryRequirements) { +VKAPI_ATTR void VKAPI_CALL StubGetBufferMemoryRequirements(VkDevice, VkBuffer buffer, VkMemoryRequirements* pMemoryRequirements) { ++g_buffer_requirements_queries; + g_buffer_bound_at_last_requirements_query = g_bound_buffers.count(buffer) > 0; pMemoryRequirements->size = g_buffer_requirements_size; pMemoryRequirements->alignment = 256; pMemoryRequirements->memoryTypeBits = 1; } -VKAPI_ATTR void VKAPI_CALL StubGetImageMemoryRequirements(VkDevice, VkImage, VkMemoryRequirements* pMemoryRequirements) { +VKAPI_ATTR void VKAPI_CALL StubGetImageMemoryRequirements(VkDevice, VkImage image, VkMemoryRequirements* pMemoryRequirements) { ++g_image_requirements_queries; + g_image_bound_at_last_requirements_query = g_bound_images.count(image) > 0; pMemoryRequirements->size = g_image_requirements_size; pMemoryRequirements->alignment = 256; pMemoryRequirements->memoryTypeBits = 1; @@ -178,6 +208,10 @@ class DeviceMemoryReportDispatchTests : public ::testing::Test { g_image_requirements_size = 0; g_buffer_requirements_queries = 0; g_image_requirements_queries = 0; + g_bound_buffers.clear(); + g_bound_images.clear(); + g_buffer_bound_at_last_requirements_query = false; + g_image_bound_at_last_requirements_query = false; g_stub_supports_debug_utils = false; g_stub_supports_debug_marker = false; g_set_debug_utils_name_calls = 0; @@ -241,6 +275,7 @@ TEST_F(DeviceMemoryReportDispatchTests, BindBufferMemoryQueriesUnknownResourceSi EXPECT_EQ(vkBindBufferMemory(device.handle(), buffer, memory, 0), VK_SUCCESS); EXPECT_EQ(g_buffer_requirements_queries, 1); + EXPECT_TRUE(g_buffer_bound_at_last_requirements_query); EXPECT_EQ(DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_BUFFER, AsObjectHandle(buffer)), 4096u); } @@ -255,6 +290,7 @@ TEST_F(DeviceMemoryReportDispatchTests, BindImageMemoryQueriesUnknownResourceSiz EXPECT_EQ(vkBindImageMemory(device.handle(), image, memory, 0), VK_SUCCESS); EXPECT_EQ(g_image_requirements_queries, 1); + EXPECT_TRUE(g_image_bound_at_last_requirements_query); EXPECT_EQ(DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_IMAGE, AsObjectHandle(image)), 65536u); } From 119943de6bb41ee21518a11c019ffde2fb38c445 Mon Sep 17 00:00:00 2001 From: Jim Blackler Date: Tue, 27 Oct 2026 15:23:42 +0000 Subject: [PATCH 3/3] device_memory_report: query image sizes at bind time instead of creation Stop calling vkGetImageMemoryRequirements from vkCreateImage. Images created with VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID must be bound before their memory requirements are queried (VUID-vkGetImageMemoryRequirements-image-04004), and drivers otherwise return a worst-case size. Every bind path already queries an unknown image size after a successful bind, and nothing reads the size between creation and binding, so the create-time query is unnecessary. BUG=b/578430447 --- ...vice_memory_report_handwritten_functions.h | 13 +--- .../test/test_devicememoryreport_dispatch.cpp | 76 +++++++++++++++++++ 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/layersvt/device_memory_report/device_memory_report_handwritten_functions.h b/layersvt/device_memory_report/device_memory_report_handwritten_functions.h index 114f74aed2..37f03c442f 100644 --- a/layersvt/device_memory_report/device_memory_report_handwritten_functions.h +++ b/layersvt/device_memory_report/device_memory_report_handwritten_functions.h @@ -473,15 +473,10 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateImage(VkDevice device, const VkImageCreat VkResult result = fpCreateImage(device, pCreateInfo, pAllocator, pImage); if (result == VK_SUCCESS && pCreateInfo != nullptr && pImage != nullptr && *pImage != VK_NULL_HANDLE) { DeviceMemoryReport::Get().OnCreateImage(reinterpret_cast(*pImage), pCreateInfo->usage); - - // Query and record memory requirements right after creation. - // This is needed because Vulkan 1.3 applications might use vkGetDeviceImageMemoryRequirements - // *before* creation, and subsequently skip calling vkGetImageMemoryRequirements, which would leave the tracked size as 0. - if (device_dispatch_table(device)->GetImageMemoryRequirements && (pCreateInfo->flags & VK_IMAGE_CREATE_DISJOINT_BIT) == 0) { - VkMemoryRequirements mem_reqs; - device_dispatch_table(device)->GetImageMemoryRequirements(device, *pImage, &mem_reqs); - DeviceMemoryReport::Get().OnRecordResourceSize(VK_OBJECT_TYPE_IMAGE, reinterpret_cast(*pImage), mem_reqs.size); - } + // The image size is not queried here: images created with Android hardware buffer external memory + // must be bound before vkGetImageMemoryRequirements is called (VUID-vkGetImageMemoryRequirements-image-04004), + // and otherwise report a worst-case size (b/578430447). An image whose size the application never + // queried (e.g. it used vkGetDeviceImageMemoryRequirements before creation) has it queried once bound. } return result; } diff --git a/layersvt/test/test_devicememoryreport_dispatch.cpp b/layersvt/test/test_devicememoryreport_dispatch.cpp index 42fd13229f..77b0805d9a 100644 --- a/layersvt/test/test_devicememoryreport_dispatch.cpp +++ b/layersvt/test/test_devicememoryreport_dispatch.cpp @@ -243,7 +243,14 @@ TEST_F(DeviceMemoryReportDispatchTests, ProactiveMemoryRequirementsQuery) { VkImage image = VK_NULL_HANDLE; ASSERT_EQ(vkCreateImage(device.handle(), &image_info, nullptr, &image), VK_SUCCESS); + // Image sizes are not queried at creation, but once the image is bound to memory. + EXPECT_EQ(g_image_requirements_queries, 0); + EXPECT_EQ(DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_IMAGE, AsObjectHandle(image)), 0u); + + VkDeviceMemory memory = MakeHandle(0xA1000); + EXPECT_EQ(vkBindImageMemory(device.handle(), image, memory, 0), VK_SUCCESS); EXPECT_EQ(g_image_requirements_queries, 1); + EXPECT_TRUE(g_image_bound_at_last_requirements_query); EXPECT_EQ(DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_IMAGE, AsObjectHandle(image)), 16384u); VkBufferCreateInfo buffer_info = {}; @@ -261,6 +268,75 @@ TEST_F(DeviceMemoryReportDispatchTests, ProactiveMemoryRequirementsQuery) { vkDestroyBuffer(device.handle(), buffer, nullptr); } +TEST_F(DeviceMemoryReportDispatchTests, CreateImageSkipsMemoryRequirementsForAndroidHardwareBufferUntilBound) { + // Calling vkGetImageMemoryRequirements on an AHardwareBuffer-backed VkImage before memory is + // bound violates VUID-vkGetImageMemoryRequirements-image-04004 and causes drivers to report a + // worst-case upper bound (b/578430447). The layer must defer the query until the image is bound. + FakeDevice device; + g_image_requirements_size = 33554432; + + VkExternalMemoryImageCreateInfo external_info = {}; + external_info.sType = VK_STRUCTURE_TYPE_EXTERNAL_MEMORY_IMAGE_CREATE_INFO; + external_info.handleTypes = VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID; + + VkImageCreateInfo image_info = {}; + image_info.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO; + image_info.pNext = &external_info; + image_info.imageType = VK_IMAGE_TYPE_2D; + image_info.format = VK_FORMAT_R8G8B8A8_UNORM; + image_info.extent = {1080, 2424, 1}; + image_info.mipLevels = 1; + image_info.arrayLayers = 1; + image_info.samples = VK_SAMPLE_COUNT_1_BIT; + image_info.tiling = VK_IMAGE_TILING_OPTIMAL; + image_info.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_SAMPLED_BIT; + image_info.sharingMode = VK_SHARING_MODE_EXCLUSIVE; + image_info.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; + + VkImage image = VK_NULL_HANDLE; + ASSERT_EQ(vkCreateImage(device.handle(), &image_info, nullptr, &image), VK_SUCCESS); + EXPECT_EQ(g_image_requirements_queries, 0); + EXPECT_EQ(DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_IMAGE, AsObjectHandle(image)), 0u); + + // Once the AHardwareBuffer memory is bound, the post-bind size is queried and clamped to the + // imported VkDeviceMemory allocation size. + VkMemoryAllocateInfo alloc_info = {}; + alloc_info.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO; + alloc_info.allocationSize = 10592256; + VkDeviceMemory memory = VK_NULL_HANDLE; + ASSERT_EQ(vkAllocateMemory(device.handle(), &alloc_info, nullptr, &memory), VK_SUCCESS); + + VkBindImageMemoryInfo bind_info = {}; + bind_info.sType = VK_STRUCTURE_TYPE_BIND_IMAGE_MEMORY_INFO; + bind_info.image = image; + bind_info.memory = memory; + bind_info.memoryOffset = 0; + EXPECT_EQ(vkBindImageMemory2(device.handle(), 1, &bind_info), VK_SUCCESS); + EXPECT_EQ(g_image_requirements_queries, 1); + EXPECT_TRUE(g_image_bound_at_last_requirements_query); + EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes("vulkan.mem.app.usage.color_render_target"), 10592256u); + EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes("vulkan.mem.app.usage.unbound_memory"), 0u); + + // Also verify the Vulkan 1.0 vkBindImageMemory path queries requirements only after binding. + VkImage image_v1 = VK_NULL_HANDLE; + ASSERT_EQ(vkCreateImage(device.handle(), &image_info, nullptr, &image_v1), VK_SUCCESS); + EXPECT_EQ(g_image_requirements_queries, 1); + EXPECT_EQ(DeviceMemoryReport::Get().GetRecordedResourceSize(VK_OBJECT_TYPE_IMAGE, AsObjectHandle(image_v1)), 0u); + + VkDeviceMemory memory_v1 = VK_NULL_HANDLE; + ASSERT_EQ(vkAllocateMemory(device.handle(), &alloc_info, nullptr, &memory_v1), VK_SUCCESS); + EXPECT_EQ(vkBindImageMemory(device.handle(), image_v1, memory_v1, 0), VK_SUCCESS); + EXPECT_EQ(g_image_requirements_queries, 2); + EXPECT_TRUE(g_image_bound_at_last_requirements_query); + EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes("vulkan.mem.app.usage.color_render_target"), 2u * 10592256u); + EXPECT_EQ(DeviceMemoryReport::Get().GetUsageCounterBytes("vulkan.mem.app.usage.unbound_memory"), 0u); + + vkDestroyImage(device.handle(), image_v1, nullptr); + vkFreeMemory(device.handle(), memory_v1, nullptr); + vkDestroyImage(device.handle(), image, nullptr); + vkFreeMemory(device.handle(), memory, nullptr); +} + TEST_F(DeviceMemoryReportDispatchTests, BindBufferMemoryQueriesUnknownResourceSize) { // A buffer whose size was never recorded (for example when the application created it before // the layer was active) must have its size queried from the driver at bind time, otherwise the