Repository navigation
perf(agc): stop pooling released device buffers over the memory budget - #4472
solbjorg1245 wants to merge 4 commits into
Conversation
|
Is there a before/after for this change alone? The 15.3 GiB figure is one reading from a fork that carries the whole budget, and this hasn't run in a title on main; perf PRs need numbers for the change itself (#218). The |
|
Thanks, fair point. I could not get a clean before/after of this change alone on main, and I would rather say so than quote the 15.3 GiB figure as if it were one. What I tried:
Setup: Windows 11, RTX 5080 16 GiB (heap 15977 MiB, driver budget about 15.2 GiB), same binary and same save, interleaved on / off / off / on, 330 s settle after the load,
With the budget on the first What I can still do: my fork gets past both aborts above, so I can send those fixes as separate PRs first and then post the same two arms on main, or run them on a title that already boots on main. Without a title that loads a level there I cannot give a reading for the pool trim alone. On #4446: the bot is right that both PRs touch |
|
Thanks for saying so plainly. Send the two boot fixes as their own PRs first; this stays open, and the two arms on main once the title reaches a level load will do. |
|
Update: the two boot aborts I reported are already gone on current main, so there is nothing to send for them. My first try was on this PR's older base, sorry for the noise. I rebuilt on main (c2565f0) with this PR on top and ran the title again on the RTX 5080 16 GiB. The depth resummarize draw is accepted now (that is #4049, which merged after my first try), and the compute shader abort did not show up in two runs. The title plays the intro, reaches the main menu and starts loading the save. It then aborts during the load, in a pixel shader recompile: Up to the abort the budget never mattered: device-local usage stayed at about 4.7 GiB of the 14.9 GiB budget, nothing was released over the target, and there were no failures before the shader abort. I will rerun the two arms ( |
CnRJay
left a comment
CnRJay
left a comment
There was a problem hiding this comment.
One behaviour to check in the title run requested above: allocation churn near the budget. VK_EXT_memory_budget usage covers the whole process (textures, images, the pipeline cache, other pools), so the pool can be over target for reasons it can't fix. In that state every device-tier Put destroys its buffer, and the next Take of the same size calls vkAllocateMemory again. On Windows/WDDM that allocation can take milliseconds and is a known source of frame-time spikes. A level that sits just over the target would then alternate free and allocate every frame, which is worse than keeping a small pool.
Some ways to bound it:
- Hysteresis: start dropping at the target, keep dropping until usage is some margin below it, and only then start pooling again.
- Keep a small floor in the device tier (for example the sizes requested in the last N frames) even when over target.
- Count the
vkAllocateMemorycalls that follow a droppedPutwithin a frame in the[vram]line, so the trace shows whether churn happens.
The rest looks good. The integrated-GPU and missing-extension paths are left alone, and reading the budget at most every 100 ms keeps vkGetPhysicalDeviceMemoryProperties2 (which can be a kernel call) off the per-Put path.
|
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.
|
|
Thanks @parl0v for the floor and the rebase, and @oneandonlydean for putting it on the branch. I've updated the description for the floor and added the when-it-matters note and the crash section. @AlxcBTW, could you attach your device-loss logs when you get a chance? |
github-actions
Bot
added
needs rebase
|
I am updating #6553 after #5987 merged. Its remaining change checks the reported host-heap budget before a proactive host placement; main's actual-refusal fallback, pool trim and texture-cache reclaim routing are retained. There is a shared The guard's standalone and integrated tests do not establish combined behavior with this PR. Please let me know if the opposite landing order is preferable so we can compose and test it explicitly. AI-assisted: yes |
|
I found storage-cache churn below the reported heap budget on a 6 GB NVIDIA GPU. This is coordination evidence for a possible follow-up; it is not a test of #4472. GTA III, PPSA03527/01.003.000, stationary at the first "Give me Liberty" spawn beside the Kuruma, with HUD/radar visible. RTX 3060 Laptop 6 GB, NVIDIA 617.42, i5-11400H, 32 GB RAM, Windows 26300. Both runs use the same complete runtime,
The last default-pool budget report before the window ended was 4,471 MiB used against 5,161 MiB reported, with a 512 MiB storage-cache budget. That is a logged snapshot, not continuous heap telemetry. It does not meet this PR's over-budget predicate. An earlier default-pool run also gave 1.0-1.1 presents/s, but its starting cache differed, so the table uses the subsequent matched-cache control. These are profiled diagnostic intervals with a renderer readback every 30 presents, not clean performance captures. The write-back figures are logical surface traffic, not measured PCIe bandwidth. The bounded trace covered an earlier loading phase and overflowed event storage; it cannot support a gameplay speed claim. Neither setting is playable, and this does not justify shipping a fixed 128 MiB pool: #1147 documents a title that needs a larger pool. The result suggests investigating reclamation of cold idle device buffers when the storage cache is actively evicting below the heap budget, while preserving recent buffer demand. I have not changed this PR's target or implemented a competing retention policy. Please flag any existing follow-up that owns this case so the work can stay coordinated. Prepared with AI assistance. |
|
Thanks for the data. Nothing I have open covers that case: #4472 only stops pooling released buffers once the heap is over budget, so reclaiming cold idle buffers while the storage cache evicts below budget would be a separate follow-up, and I'm not working on one. If you take it on I'll rebase this around it. |
|
@solbjorg1245 This PR now conflicts with main in |
…et read Main's pool now keeps released image memory in the device tier and reads the heap budget itself (at most every 100 ms) to drop that image memory over the budget, frees idle device buffers under storage cache pressure, and trims the pool before an allocation retry. The VRAM budget had its own reader and only knew about the buffers its own Put destroyed. VramBudget no longer queries the driver: the pool's heap budget read feeds it, so a device buffer Put and an image Put within 100 ms share one vkGetPhysicalDeviceMemoryProperties2 call, and the read happens under the pool mutex like the image path's. Every device-tier free the pool makes between two reads is counted as released, so Excess() stays right until the next read: Put's evictions, the image path's drops and evictions, ReclaimIdleDevice and trim(). The bytes are counted under the pool mutex before the memory is destroyed, so a read that lands in between can only see the memory still in use, never count it twice. Over the target, Put gives back retained image memory before the buffers the demand floor keeps (main already drops it over the reported budget, and image takes are not demand), and the floor compares against the buffers alone. The refused-allocation host fallback in AllocateMemory now counts in HostPlacements() too, so the [vram] line reports both placement paths. APS5_NO_VRAM_BUDGET and APS5_VRAM_BUDGET_MIB are documented in ENVIRONMENT.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NWqixRjxAxzP2dBDkLsjZ6
|
This pull request is labeled |
|
Merged current main into the branch, with the reconciliation in its own commit (c58f968) so it reads separately from the merge. The budget no longer has its own reader: it takes the pool's existing 100 ms heap budget read, so buffer and image releases share one query. Every device-tier free (image memory in |
|
@solbjorg1245 VramBudget tracks the largest device-local heap, but releasedBytes counts every device-tier free without checking which heap owned it. With two device-local heaps, freeing 64 MiB from the smaller heap can take Excess() from 64 MiB to zero while the tracked heap is still 64 MiB over target. PutImageMemory, image-first eviction and trim can pass those unrelated frees through; ReclaimIdleDevice already guards the single-device-heap case. Please credit only actual frees from the tracked heap, or disable this policy when there is more than one device-local heap. Add a two-device-heap regression showing that freeing the other heap leaves the tracked excess unchanged, while freeing the tracked heap lowers it once. The current budget mock has only one device-local heap, so it cannot catch this. If you take the guard route, it only needs to count device-local heaps that have a memory type the device tier can use (DEVICE_LOCAL without HOST_VISIBLE). On the common setups without resizable BAR, the second device-local heap is the small BAR window and all of its memory types are host-visible, so the policy can stay on there. |
Verdictstale+rebase Findings
Freshness envelope
Cross-PR ledger
|
What
The buffer pool's device tier keeps released device-local buffers up to an eighth of the largest device-local heap (at least 512 MiB), however full that heap is. Near the budget that
VK_EXT_memory_budgetreports, this retained memory is what pushes the process over it. Past the budget an allocation can still succeed while the OS starts paging device memory orDeviceMemoryTypeplaces it in host memory, so the out-of-memory trim inBufferPool::AllocateMemorycomes late or not at all.VramBudget(new,Graphics/src/VramBudget.cpp) holds the policy for the largest device-local heap: the target is the reported budget itself (never below a quarter of the heap or 1 GiB, whichever is smaller). It does not query the driver itself. The pool's existing heap budget read (heapOverBudget, at most every 100 ms, the one the image memory path already uses) feeds it, so buffer and image releases share onevkGetPhysicalDeviceMemoryProperties2call.BufferPool::Put: while the reported usage, minus what the pool freed from the device tier since that read, is over the target, the pool first gives back retained image memory (the image path already frees it over the reported budget, and image takes are not demand). Then the device tier keeps released buffers only up to a demand floor: what it handed out (hits and misses) in the last second, in eight 125 ms buckets, capped at a sixteenth of the heap. Anything beyond the floor is destroyed, least recently used first. A title that doesn't churn has a small floor and gives back everything else; one whose working set sits just over the budget keeps what it is about to ask for again instead of re-allocating it every frame. Host tiers, the small tier and slabs are unchanged, and aPutthat lands in another tier is not counted as device memory released.Putevictions, image memory dropped or evicted inPutImageMemory,ReclaimIdleDeviceunder storage cache pressure, andtrim()before an allocation retry. The bytes are counted under the pool mutex before the memory is destroyed, soExcess()never counts a free twice.Context::integratedGpu, set from the device type: there the device-local heap is a carve-out the driver spills into system memory) and withAPS5_NO_VRAM_BUDGET=1.APS5_VRAM_BUDGET_MIBcaps the budget, which lets a large card behave like a smaller one. Both are indocs/dev/ENVIRONMENT.md.[vram]line is printed once, at the first read: the heap, its budget and usage. WithAPS5_PROFILE_DRAW, another[vram]line follows the[bufferpool]line every 10 s with the budget, usage, target, floor, the released device allocations not kept (count and MiB), the device allocations made while the pool was dropping (the churn) and the allocations placed in host memory, both byDeviceMemoryTypeand by the refused-allocation fallback inAllocateMemory(counted inHostPlacements()instead of two function-local statics).When it matters. The pool only drops buffers once the device-local heap is over the budget
VK_EXT_memory_budgetreports. At default settings Minecraft Dungeons II on an 8 GB card settles below it, and nothing changes. It matters for a heap that is already full: a bigger title, a forced or oversizedAPS5_TEXTURE_CACHE_MIB, or other processes using VRAM. There, without this PR, the pool's retained buffers push new device allocations into host memory.The crash it prevents (@AlxcBTW, RTX 5070 Laptop 8 GB, Minecraft Dungeons II world load + walk): with the device-local heap over its budget, the automatic budget evicted a 256 MiB 3D storage texture (1024x1024x256 R8). It was re-created in host memory, and a compute pass over it ran past the 2 s TDR limit (nvlddmkm 153 / LiveKernelEvent 141). Over the earlier rows that stack lost the device in 2 of 11; with this PR, 0 of 5. Logs: to be attached by @AlxcBTW.
Found with Demon's Souls on our fork: on a 16 GiB card the device-local heap reached 15.3 GiB at a level load while about 2 GiB of it was released buffers held for reuse. The demand floor replaces the first version's fixed headroom after @CnRJay's churn review; it is @parl0v's commit, with his lab rows in the thread (RTX 4060 8 GB, forced 3 GiB texture cache: walk 3.94 FPS with the headroom, 5.63 with the 1 s floor). Reclaiming cold cache entries would be a separate follow-up. The change itself has no title check.
Tested
On main
85a4d7f8c:agc_buffer_pool,agc_buffer_pool_small_device_shared,agc_buffer_pool_small_device_no_staging,agc_device_memory_fallback, the sampled and storage cache budget tests andagc_driver_pooled_buffer_rangepass;python tools/check_conventions.py --base origin/mainpasses.agc_buffer_poolbudget tests (mock device, no GPU needed):Putthat evicts from the small tier leavesExcess()unchanged;Puttrims the oldest retained buffers;ReclaimIdleDevicelowerExcess()by their size;Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code
https://claude.ai/code/session_01VFVE5WbaUJ2vyj8NAW44CS