Skip to content

device_memory_report: distinguish driver and application unbound memory - #35

Merged
olehkuznetsov merged 6 commits into
android-graphics:mainfrom
jimblacklercorp:bugfix-device-memory-report-unbound-memory-attribution
Sep 16, 2026
Merged

olehkuznetsov merged 6 commits into
android-graphics:mainfrom
jimblacklercorp:bugfix-device-memory-report-unbound-memory-attribution

Conversation

@jimblacklercorp

Copy link
Copy Markdown

Only inspect associated virtual resources for driver allocations when determining unbound track name, preventing handle collisions between VkDeviceMemory and previously registered VkBuffer/VkImage handles from misclassifying application unbound headroom.

Also free the driver allocation at the end of EmitEventsAndSubCounters. DeviceMemoryReport is a process-wide singleton, so the previously leaked 2048 byte driver allocation stayed on
vulkan.mem.driver.usage.unbound_memory and leaked into subsequent tests in the same binary, breaking the new attribution test.

Only inspect associated virtual resources for driver allocations when
determining unbound track name, preventing handle collisions between
VkDeviceMemory and previously registered VkBuffer/VkImage handles from
misclassifying application unbound headroom.

Also free the driver allocation at the end of EmitEventsAndSubCounters.
DeviceMemoryReport is a process-wide singleton, so the previously leaked
2048 byte driver allocation stayed on
vulkan.mem.driver.usage.unbound_memory and leaked into subsequent tests
in the same binary, breaking the new attribution test.

@olehkuznetsov olehkuznetsov left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Distinguish driver and application unbound memory

Thanks for this fix! The core change is mathematically sound: guarding resources_.find(allocation.object_handle) with if (allocation.is_driver) correctly resolves the issue where application allocations (VkDeviceMemory) whose handles coincided with tracked resource handles were misattributed to driver cluster tracks.

The review identified a few areas to refine before merging:

  1. Handle collision across driver object types: Driver allocations for non-image/buffer objects (e.g. VkPipeline, VkDescriptorPool) could still collide with resources_ handles. Restricting the lookup to VK_OBJECT_TYPE_IMAGE and VK_OBJECT_TYPE_BUFFER will make this fully watertight.
  2. Re-attribution test coverage: The new test sets up the resource before the allocation. In real driver workflows, allocation callbacks often arrive before OnCreateImage / OnCreateBuffer. Adding a test case for this sequence ensures the re-attribution loops updated in lines 329 and 342 remain covered.
  3. API doc contract & test naming hygiene: Minor refinements to document the fallback return of GetUsageCounterBytes and use descriptive variable names in the unit tests.

FYI / Follow-up Note (Pre-existing issue outside this diff):
In layersvt/device_memory_report/device_memory_report.cpp:359:

uint64_t key = (objectType == VK_OBJECT_TYPE_DEVICE_MEMORY) ? objectHandle : memoryObjectId;

For driver internal allocations where objectType == VK_OBJECT_TYPE_DEVICE_MEMORY, drivers often set objectHandle = 0. This causes multiple driver device memory allocations to collide on key 0 in memory_allocations_. Because this is a pre-existing issue in the layer's keying logic, it shouldn't block this PR, but we should track and address it in a separate change.

Comment thread layersvt/device_memory_report/device_memory_report.cpp
Comment thread layersvt/device_memory_report/device_memory_report.h
Comment thread layersvt/test/test_devicememoryreport.cpp
Comment thread layersvt/test/test_devicememoryreport.cpp Outdated
Comment thread layersvt/test/test_devicememoryreport.cpp Outdated
Comment thread layersvt/test/test_devicememoryreport.cpp Outdated
Comment thread layersvt/test/test_devicememoryreport.cpp
@olehkuznetsov
olehkuznetsov merged commit 2ba92d5 into android-graphics:main Sep 16, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants