Repository navigation
fix(agc): place the presentation source in host memory when device memory runs out - #10807
Conversation
…mory runs out The presentation scaler allocated its source image from device-local memory with vkAllocateMemory and threw on any failure. With the device-local heap full (Forspoken fills it on a 12 GB card next to the host's other GPU users), the next resize of the source failed with VK_ERROR_OUT_OF_DEVICE_MEMORY, the presentation failed and the title terminated. The source now picks its memory type through DeviceMemoryType, like textures, and takes its memory from the buffer pool's image path, which reclaims cached memory and then falls back to host memory when device-local memory is refused; it gives the memory back through ReleaseImageMemory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the fallback path and test. The host-memory fallback is exercised by injecting a device-local allocation refusal, and the test verifies both the refusal and resulting heap class. I did not find an additional correctness blocker in the changed ownership path: the source allocation records its size/type and returns through ReleaseImageMemory. The hosted workflow has conventions/intake/report green; platform product jobs are skipped. No separate local Vulkan rerun was available here.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
@MarcZX10 thank you for the review. The Vulkan runs are the ones under Tested: the new case fails 3 of 3 on AI-assisted: yes (Claude Code) |
|
One overlap follow-up after manually checking the bot’s full report: #7642 and #10193 are also open and both add the same |
|
@MarcZX10 thanks for checking the full report. Confirmed: #7642 and #10193 both add that AI-assisted: yes (Claude Code) |
MarcZX10
left a comment
There was a problem hiding this comment.
Thanks for confirming that #7642 and #10193 add the same GpuHeapEnabled() / OpenVulkanTestDevice() fixture block. Both PRs are still open, while this PR checklist remains Depends on: none. CONTRIBUTING.md requires the authors to agree which overlapping PR goes first and declare it with Depends on. Please coordinate with #7642 and #10193 and update the checklist (and have the other affected checklist(s) reflect the agreed order). I have not modified or rebased any branch. The hosted policy checks currently pass; product jobs are skipped.
|
@MarcZX10 I've asked both authors, on #7642 and #10193. I proposed this PR goes first: #7642 is a draft with changes requested, #10193 depends on #7642 and needs a rebase, and it's a merge order with no code dependency either way. The checklist now records that proposal and that they haven't answered yet. I'll update it, and rebase if they'd rather go first, once they reply. AI-assisted: yes (Claude Code) |
|
Thanks for merging, @oneandonlydean, and @MarcZX10 for the review. AI-assisted: yes (Claude Code) |
What
Forspoken (PPSA03984) on
mainstopped once at 118 s with:That run was on Linux, RTX 4070 Ti (12 GB), NVIDIA 615.71.09, with about 2.3 GB of the card held by other processes on the machine. The title fills the device-local heap (on that card it already reports
device-local heap 0 uses 9164 of its 9164 MiB budgetfor textures), so when the presentation source image was recreated, itsvkAllocateMemoryreturnedVK_ERROR_OUT_OF_DEVICE_MEMORY.PresentationScaler::EnsureSourceImageasked for device-local memory only and threw, and the presentation failure terminated the title. Any card whose device-local heap the title fills can hit this.How
PresentationScaler::EnsureSourceImagepicks the source's memory type withContext::DeviceMemoryType, as textures do: a device-local type while the heap's budget has room, else a host type. It takes the memory from the buffer pool's image path (AllocateImageMemory). On a refused device-local allocation that path reclaims cached pool memory first, then falls back to host memory throughRefusedDeviceMemoryFallback.release()gives the memory back withReleaseImageMemoryinstead ofvkFreeMemory. The scaler keeps the allocation's size and memory type for that.Overlap (
tools/pr_overlap.pyandgit merge-treeof each head with this one):tests/PresentationPass.cpp: perf(agc): GPU descriptor heap on VK_EXT_descriptor_heap (APS5_GPU_HEAP) #9508 adds itsGpuHeapEnabled()block right afterresidentPresentTests(device->GetContext())inmain, where this PR addssourceImageOutsideDeviceMemoryTests(device->GetContext()). feat(agc): look up table images at run time through a device image table on the GPU heap (APS5_GPU_PATH) #7642 and feat(agc): serve lane-keyed, pointer and runtime image-table columns #10193 carry that block through perf(agc): GPU descriptor heap on VK_EXT_descriptor_heap (APS5_GPU_HEAP) #9508. feat(videoout): composite the overlay bus over the main output #10507 adds its own test call there, and feat(agc): scale the presentation with FSR 1 under ANYPS5_UPSCALE=fsr #10043 adds helpers just beforemain, where this PR adds its allocation-refusing wrappers. Each resolves by keeping both sides, with this PR's call above the heap block.PresentationScaler.cpp: feat(agc): trace device memory allocations and cache budgets #10194 changes the same two lines (the source's allocate and free) toMemoryAccounting::Allocate/Free. With feat(agc): trace device memory allocations and cache budgets #10194,BufferPool::AllocateMemory,TakeImageMemory,PutImageMemoryandReleaseImageMemoryrecord the accounting themselves, so the merge keeps this PR'sAllocateImageMemory/ReleaseImageMemoryand drops feat(agc): trace device memory allocations and cache budgets #10194's two calls; proposed on feat(agc): trace device memory allocations and cache budgets #10194.Tested
Linux (Ubuntu 26.04.1, GCC 15.2, Release, RTX 4070 Ti, NVIDIA 615.71.09) unless noted.
agc_driver_presentation_passgains a case that gives the scaler a context whosevkAllocateMemoryrefuses every device-local memory type. It requires the source image to be created, at least one device-local refusal, and the image's memory to come from a host heap. With the test alone onmain(7d24e61d28) it fails 3 of 3 with the title's message,AGC graphics: vkAllocateMemory presentation source: Vulkan result -2. With this change it passes 3 of 3, and the pool logsvkAllocateMemory presentation source: memory type 1 is out of device memory for 16384 bytes ...; placed in host memory type 0. The case skips on a device with no host memory type for the image.presentation|buffer_pool|^agc_driver_graphics$|displaypass 3 of 3. On the Windows 11 VM (GT 1030, driver 582.78, WinLibs GCC 15.2.0 r7) the same 8 pass 3 of 3 on the merge base and on this head. The new case runs there too (memory type 7 refused, placed in host memory type 1).VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 ctest -R "$R" -V -j4: 8 of 8 onmainbd8e134 and on this head, no layer-missing line, noVUID-message on either.main2e22b67 plus a local diagnostic that only recordsVK_EXT_device_address_binding_reportevents: 2 of 15 runs stopped as quoted above (at 118 s and 115 s).presentation sourceline), so they show nothing regressed, not the fallback in the title.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code