Repository navigation
Conversation
…nt cap Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the ceiling calculation and its use in both cache budgeting and idle-buffer reclamation. The shared StorageTextureCeiling preserves the former StorageTextureBudget cap/floor behavior while removing the stale quarter-budget reclaim limit; the added sweep checks the invariant across heap budgets and usage levels. The contributor reports the focused Vulkan suite passing 6/6 with validation enabled on RADV, including a one-line mutation that makes the expected half-budget case fail. I did not rerun the GPU test locally.
|
Independent Windows/NVIDIA check of the reclaim-ceiling fix at head MeasuredEnvironment: Ryzen 5 5600X, 16 GB RAM, RTX 3070 8 GB, driver 32.0.16.1088, Windows 11 22635. Squirrel with a Gun
Focused test on this machine: Logs: Analysis (AI-assisted)The title did not reach the condition this PR changes in either arm. The storage cache stayed below the reported storage budget in the sampled windows, and the buffer pool reported 0 device evictions in both runs. The lower device retention in the head run (353 vs 438 MiB) is not attributable to the change from one pair. The images-phase and build differences are also within the large run-to-run spread this title shows; I am not treating them as an effect. So: no observable SWAG benefit, and no regression in this check. The fix is not exercised by this title under normal memory pressure. The Echoes of the End pair remains the better place for a throughput result, or a controlled lower-budget diagnostic would be needed if you want the reclaim branch itself exercised. AI-assisted report. Measured data and interpretation are separated above. |
|
Recorded your exact-head Windows/NVIDIA report in Tested with its limits: no fatal/OOM in the reported pair, but the title stayed below its storage budget and never exercised the changed reclaim branch. The observed pool/timing differences are not a performance result, and I have not treated unreported Windows validation-layer coverage as a clean validation run. The stale queued-pair wording is replaced with the specific outstanding measurement. The existing layered Linux gate and deterministic ceiling/negative-control checks remain separate. |
What
#8672 reclaims idle device-tier buffers from the buffer pool when the storage image cache needs more room than its budget gives. The amount is capped at a ceiling: the most the cache may hold. Its debt entry describes this as "the cache's shortfall below its existing ceiling". In
storageBudgetthe ceiling is computed asmax(512 MiB, min(TextureCacheBudget, budget / 4)), which is a copy of the storage cache's cap before #10044.#10044 raised that cap to
min(heap / 2, budget / 2)inStorageTextureBudget, but the copy instorageBudgetwas not updated. On main the two now disagree. When the cache's shortfall lies between a quarter and half of the budget, idle pooled buffers are no longer reclaimed for it, althoughStorageTextureBudgetwould let the cache grow into the room they free. Example: an 8 GB GPU with a 6 GiB budget. The cap is 3 GiB, but reclaim stops at 1.5 GiB.This PR moves the cap into one function,
StorageTextureCeiling, used by bothStorageTextureBudgetand the reclaim ceiling. It returnsmax(512 MiB, min(heap / 2, budget / 2)), orTextureCacheBudgetwithoutVK_EXT_memory_budget.StorageTextureBudgetreturns the same values as before:min(max(512 MiB, cap), max(512 MiB, free))equalsmax(512 MiB, min(cap, free)). The only behavior change is the reclaim ceiling.Tested
storageTextureBudgetTestsadds checks for the ceiling: a quarter of the heap withoutVK_EXT_memory_budget, half of a 6 GiB budget, half of the heap for a 40 GiB budget, and the 512 MiB floor. A sweep over budgets (1-24 GiB) and usage checks that the storage budget never exceeds the ceiling, and that an idle heap reaches it.Focused Vulkan gate. Machine: Linux 7.2.9, RX 6650 XT, RADV (Mesa 26.2.4). Command:
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_buffer_pool.*|agc_driver_sampled_cache_budget_.*)$'.aeeaefad98cc90fbcb7c+ one-line editagc_driver_graphicsfails with "a 10 GiB budget on the 16 GiB heap does not limit the storage image cache to half of the budget"cc90fbcb7cThe head was then rebased on
5d3ab7791dwithout conflict. None of the four files it touches differ betweenaeeaefad98and5d3ab7791d.Logs: https://github.com/JJenkx/AnyPS5/releases/tag/storage-reclaim-ceiling-evidence
This PR makes no performance claim. A title measurement that exercises the changed reclaim condition remains outstanding; the independent Windows report below did not reach it.
Related measurement, from a diagnostic and not this change. On the same GPU at its real reported budget (about 6 GiB), shrinking the pool's device and image tiers by override raised the storage cache budget in Echoes from 0.5-0.9 GiB to 1.6-2.2 GiB. That suggests idle pool retention competes with the storage cache there. The throughput effect was within run-to-run spread.
Independent Windows/NVIDIA coverage reported by @Rajkamalcode: exact head
0d6a6f96eeee90b4998deaadae50ef5e3cf01a53merged onto main5ed5427baas469b28f11, Ryzen 5 5600X / 16 GB RAM, RTX 3070 8 GB, driver 32.0.16.1088, Windows 11 22635. Their focused graphics executable passed, including the StorageTextureCeiling cases. Validation-layer use was not reported for that Windows run; the local layered Linux comparison above remains scoped separately.Their two 360-second Squirrel with a Gun (PPSA22102, 01.000.000) runs used the same pad schedule/cache/settings and had no fatal/OOM, with 39–41 presents per 10 seconds after load in both arms. The sampled storage cache stayed below its budget and both runs reported zero device-pool evictions, so the changed reclaim branch was not exercised. Pool retention and images-phase differences from that single pair do not establish a benefit. This is attributed independent coverage, not a locally reproduced run or performance evidence.
Checklist
main; no other open PR implements the same functions. fix(agc): grow the storage cache budget while it evicts under pressure #6194 also changes the storage budget, but it grows the budget while evicting and does not touch the reclaim ceiling.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