Repository navigation
perf(agc): let the storage image cache take up to half of the heap budget - #10044
Conversation
github-actions
Bot
added
the
unblocks title
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head 511ef5dd6a92cbb4ce9073497aac07605337d70b. The implementation changes only the storage-cache cap from the prior quarter-budget / quarter-heap-derived cap to min(heap / 2, budget / 2). The existing headroom formula, sampled-texture reservation, pressure response, fallback, and 512 MiB floor remain unchanged. The revised unit cases cover both caps, the pressure floor, monotonic shrinkage under added non-cache use, and the reserved headroom invariant. Current-main sync is clean (1 ahead / 0 behind), git diff --check passes, and hosted eligibility, intake, conventions, labels, and report checks pass. Product build/test jobs are skipped by repository policy.
I could not execute the Vulkan test locally: CMake configuration stops because SDL2 is uninitialized in this checkout and the host lacks XRandR development support. The author reports 15/15 focused Vulkan tests on the rebased patch, including a test-only negative control that fails on main. I downloaded the linked release summaries and traces and reran tools/performance_compare.py; it reproduces the reported 296 to 557 presents, p50 1018.7 to 529.3 ms, and GPU busy 466.6 to 277.7 ms per present. The comparison flags window.first_present as the only recorded configuration difference; the PR explains both measured windows begin in the same static scene. Both traces dropped detailed events, so phase attribution is partial, while the summary frame/GPU metrics remain available. I have not independently run the title or verified the physical scene.
One non-blocking maintainability note: the local conventions checker emits a debt-length warning for the updated texture-budget entry (1,522 characters); the hosted conventions check passes. Trimming it to the essential contract and evidence would clear the warning.
|
Thanks for the review. I condensed the texture cache budget debt entry to 903 characters in |
|
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.
|
|
Correction to the evidence in the description: the matched pair above does not isolate this change. I am returning the PR to draft until a repeated, alternating pair does. A later run of the same commit as the "with" arm ( The game's own budget lines (
So the outcome depends on the reported budget at run time as much as on the cap. When the budget is lower, the storage cache stays under its working set. It keeps evicting, the sampled cache grows with copies of re-created storage images, and the reserve shrinks the storage budget further. The mechanism in the description still holds. This PR raises the ceiling only when the budget has room, and the first pair happened to run with about 1 GiB more budget on the "with" side. Next: alternating arms (without, with, with, without, ...). Each arm will log the reported budget and storage occupancy, and the evidence will compare arms with similar budgets. I will update the description with that result, or close the PR if the gain does not hold. |
|
Alternating repeat, as planned in the correction above. Same seed and inputs as before, gameplay window
There were 0 amdgpu kernel events in every arm. Both arms with this change were faster than both arms without it. In all four arms, however, the reported budget was higher on the side with the change (by 485-640 MiB), and an earlier run of the same change at a median budget of about 6.1 GiB was slow (265 presents). So these runs still do not separate the cap from the budget. They are consistent with this: the change helps when the budget leaves room for about 2.8 GiB or more of storage images, and does not help below that. To isolate the cap, the next pair pins the reported heap budget that the caches read to the same value in both arms. It uses a private diagnostic override and runs at 6900 MiB and at 6300 MiB. The PR stays a draft until that result is in. |
|
Budget-pinned result. Both arms read the same reported heap budget: a private diagnostic override (
With equal budget input, this change is a modest, budget-dependent gain, not the 1.9x of the first pair:
Caveat on the 6900 MiB pair: other workers' compilations overlapped its "without" arm, which may have slowed it. I will repeat that pair on its own and report it. The PR stays a draft until then. I will rewrite the description's Tested section around the pinned pairs once the repeat is in, or close the PR if the 6900 MiB gain does not hold. |
Independent 8 GB NVIDIA/Windows check: pinned 6900 MiBI measured this on Squirrel with a Gun ( Diagnostic only: a private local Pinned 6900 MiB, quarter -> half (single pair, not ABBA)
Normalized per build:
The half cap creates 92.9% fewer storage images over the run, 25x fewer per build, and cuts creation CPU by roughly 33x per build. The storage budget moves from 1725 MiB to 3450 MiB, above the ~2.5 GB working set this title needs; the quarter cap stays below it. The half arm had a long 0-present interval and then ended at 18-21 presents/10 s; the quarter arm ended at 0. Builds continued through the 0-present intervals, so I do not use final presents as an FPS result. Free-running host is not a valid A/B hereFour earlier 360 s runs, ABBA (quarter, half, half, quarter), same tree without the diagnostic pin:
A separate fixed-2560 diagnostic ( ConclusionAt a stable 6900 MiB reported budget, the cap behaves as this PR describes: 1725 -> 3450 MiB storage budget puts SWAG's working set in cache and removes nearly all storage-image churn. That supports the author's budget-pinned methodology and the cap on a second 8 GB NVIDIA/Windows title. It does not establish a universal frame-rate gain, and it does not test #6194's adaptive growth. The free-running result is inconclusive because the reported budget itself moves by several GiB. I can repeat the reverse order at 6900 MiB when the GPU is free; the current pair is one direction and the host is now handed back. AI-assisted: yes. The tables above are measured; the conclusion is interpretation. |
|
Thanks @Rajkamalcode for the independent pinned-budget check on Squirrel with a Gun. I added it to the description as attributed, limited support, with your limits: old base, a single pair, no frame-rate claim, and the free-running A/B inconclusive. The description now rests on budget-pinned pairs on Echoes, with a repeat in reverse order. At 6900 MiB they show about +25 % presents and -13 to -17 % frame p50. At 6300 MiB they show no difference, because the reserve, not the cap, limits storage there. The earlier unpinned results are kept only as superseded. Marked ready. |
|
Provenance note on the 6900 MiB repeat pair in the description. Two 4-thread library builds by another worker ran on this machine from 16:53:42 to 17:01:16 UTC. That overlapped the loading phase of the repeat's "with" arm (run start 16:44:45 UTC). Its capture window was 17:03:55-17:08:55 UTC, after the builds ended. The "without" arm (window 17:28:34-17:33:34 UTC) did not overlap at all. Both arms' 1140 s screenshots show the same scene in play. The first 6900 MiB pair's "without" arm did overlap other compilations, as the description says, so the repeat is the cleaner pair. |
|
An interaction between #8672 and #10044, from reading both diffs. It is not measured. #8672's
On Echoes of the End (8 GB GPU), the formula gives about 1.6-2.9 GiB of storage depending on the budget, so the quarter ceiling (about 1.6-1.75 GiB) would usually keep #8672 from reclaiming when #10044 is present. Whichever PR lands second could derive the ceiling from the same cap expression as |
|
@JJenkx ShaderResources.cpp:1232–1238 still reserves only TextureCacheBudget for storage, while the new storage cap is half the heap/budget. The caches can then exceed the reported budget: an 8 GiB heap with 7.2 GiB budget and 2 GiB other use allows about 3.6 GiB storage plus 2.3 GiB sampled. Could the sampled budget reserve the storage cache’s actual size or its new cap, and could storageTextureBudgetTests assert the sum of both budgets stays within the reported budget? |
…dget Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean agreed; fixed in
Exact-head gate (
Logs: The Echoes and Squirrel with a Gun numbers in the description predate this commit. This commit only lowers the sampled budget when the storage cache holds more than |
a2f89ca to
9904c73
Compare
|
@JJenkx full CI on 9904c73 (run 40048597992) fails You updated the unit check in |
…cution test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@oneandonlydean thanks, fixed in Exact-head gate on Linux / RX 6650 XT / RADV, same command as before plus I agree the Windows |
Post-merge check on current main: SWAG now runs againCurrent main Hardware/title: Ryzen 5 5600X, 16 GB RAM, RTX 3070 8 GB, Windows 11 22635, driver 32.0.16.1088; SWAG Measured (no fatal/OOM in either run)
The storage cache held 3,132 MiB / 2,857 MiB at the tail; the sampled budget reached its 2,048 MiB floor. Storage hits were 1,004,617 / 1,151,183 with 47,077 / 53,311 own-object refreshes over 360 s. Neither run had a zero-present window after the initial load. For scale, the old-base free-running controls on this title created roughly 42k-48k storage images, and the old fixed-2560 MiB diagnostic created 3,150 images with a 10,152 ms images phase. AI-assisted interpretation: the merged half cap is visibly active on this 8 GB NVIDIA/Windows node, and the current-main combination keeps the storage working set near 3 GiB without the collapse to the 512 MiB floor seen on the old base. I am not claiming the images-phase or presentation change as #10044 alone: current main includes #8672 and many other merged changes. I also am not using presents as an FPS result, since the builds and scene progression differ between runs. |
What
The storage image cache's budget is what the reported heap budget leaves after three things: the heap's use outside the texture caches, the live sampled textures, and an eighth of the budget. That result is then capped at the smaller of
TextureCacheBudget(a quarter of the heap, at least 2 GiB) and a quarter of the budget. On an 8 GB GPU, the quarter caps bind first, at about 1.5-1.7 GiB, even when the budget formula leaves far more.Echoes of the End (PPSA25636) needs about 3.5-4 GiB of storage images in play. With the quarter caps it re-creates them all the time. In gameplay, with the default budgets, it evicted about 2000 storage images (about 22 GiB) every 10 s, and re-created and re-uploaded them on their next use. The technical debt entry for the budgets describes this case: "a title whose storage working set exceeds the shrunk budget re-creates storage images at each use".
This PR changes only the caps, to half of the heap's size and half of the reported budget. The reserve is unchanged: the other use, the live sampled textures and the eighth of the budget are still kept out. The 512 MiB floor and the fallback without
VK_EXT_memory_budgetare also unchanged. A heap under pressure therefore still shrinks the storage cache as before. The higher cap applies only when the budget has room.Tested
storageTextureBudgetTestsis revised on purpose:9904c737ca(adds the sampled-budget storage reservation from review). Exact-head gate:main1b5efcc8d916/16; this PR with the new reservation reverted 15/16, failing only "a storage image cache larger than TextureCacheBudget was not reserved in full"; this PR 16/16. 4 layer insertions and 0 VUIDs per arm. Details: perf(agc): let the storage image cache take up to half of the heap budget #10044 (comment). The joint sweep asserts that above the floors neither cache budget plus the other cache's held bytes exceeds the reported budget less its eighth.ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation VK_LOADER_DEBUG=layer ctest -V -j1 -R '^(agc_driver_graphics|agc_sparse_.*|agc_heap_mirror_.*|agc_host_import.*)$'. Machine: Linux 7.2.9, RX 6650 XT, RADV (Mesa 26.2.4).Results:
b7a451d62bb7a451d62b+tests/Graphics.cppfrom the changeagc_driver_graphicsfails with "a 10 GiB budget on the 16 GiB heap does not limit the storage image cache to half of the budget"92b3bf8f8b(the same source and tests as the heada2f89cad59, whose later commit only condenses the debt entry)Every arm prints only the two existing Windows guest-arena skips ("heap mirrors not tested", "heap mirrors of imported ranges not tested").
e3f17c7043) and with it (52893755e9), in one build directory. Both carry private diagnostic commits; none of them is part of this PR.Each arm starts from the same seed (the save in play, the shader caches and a private Mesa cache) and gets the same scripted input. Capture:
APS5_TRACE_SECONDS=1150:300, a static scene in play.The reported heap budget is pinned. The VK_EXT_memory_budget heap budget varies by about 1 GiB from run to run on this machine, with other VRAM users, and moves the storage budget as much as the cap does. So a private override,
APS5_DIAG_HEAP_BUDGET_MIB(not part of this PR), sets the value both budget functions read. The device's real memory use is not pinned.At 6900 MiB this change gives about 1 GiB more storage budget, +24-26 % presents, -13 to -17 % frame p50 and -17 to -19 % GPU busy, in both orders. Other workers' compilations overlapped the first pair's "without" arm; the repeat is clean.
At 6300 MiB the reserve, not the cap, limits the storage cache. The difference is within noise. This change helps only where the budget has room above a quarter.
0 amdgpu kernel events in every arm. Both traces of the repeat dropped some queue-worker events, so their phase totals are partial.
Summary JSON lines of the repeat pair (without, with):
Superseded: the first unpinned pair (296 -> 557 presents) and an alternating four-arm repeat. Both were confounded by the reported budget, which was 485-640 MiB higher in every "with" arm; see the comments.
Traces, digests and comparisons: https://github.com/JJenkx/AnyPS5/releases/tag/storage-half-evidence
Independent, attributed, limited: @Rajkamalcode measured Squirrel with a Gun (PPSA22102) on an RTX 3070 8 GB under Windows. The base is an old one that runs the title, not current
main. They used their own pinned 6900 MiB budget and one quarter -> half pair:They make no frame-rate claim, because of long zero-present intervals, and their free-running A/B was inconclusive because the reported budget moved by several GiB. Details: perf(agc): let the storage image cache take up to half of the heap budget #10044 (comment)
Diagnostic-only occupancy runs (private overrides): with a 4096 MiB storage budget, Echoes in play held about 4077 MiB of storage images, with 250 MiB of sampled textures and about 2.2 GiB outside both caches. Its storage working set is about 3.5-4 GiB.
Checklist
main; no other open PR implements the same functions. fix(agc): grow the storage cache budget while it evicts under pressure #6194 grows the storage budget while it evicts, but keeps at least 2 GiB of headroom, so on this 8 GB GPU near its budget it does not grow.NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file (no unimplemented path added).mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code