Repository navigation
perf(agc): reclaim idle device buffers under storage cache pressure - #8672
adriadam10 merged 4 commits into
Conversation
github-actions
Bot
added
perf
|
This pull request is labeled |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the reclaim policy and its call site. Take advances the pool clock, so entries reused during the one-second observation window are newer than reclaimBefore; active allocations are absent from the free tier, and slab allocations are explicitly excluded. The operation removes pool bookkeeping under the mutex and destroys Vulkan allocations after releasing it. The one-device-local-heap guard matches the implementation's lack of per-allocation heap tracking, and the budget is reread only after an actual release.
The PR description reports focused Vulkan validation with synchronization validation and paired traces for the same Windows/NVIDIA scene, while clearly limiting the performance claim to that scene. Hosted platform jobs are skipped, so this approval relies on the author's detailed run report rather than an independent build or GPU run 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.
|
a6c415f to
74b001a
Compare
|
Rebased to Both complete Windows/NVIDIA runtimes pass the same 28 focused tests with cache disabled and Khronos synchronization validation: no VUID errors or skipped cases. Conventions pass. The description now distinguishes these checks from the earlier title timing evidence. Prepared with AI assistance. |
github-actions
Bot
added
build
docs
agc
|
Second title, on AMD/Linux: neutral on Code Violet (PPSA26528). The reclaim path runs, but there is little idle pool to reclaim on this title. Setup
Why neutral. This title's storage-image cache churns, with 38–40% of lookups missing. Nearly all misses are budget evictions of surfaces cached before. The per-frame storage set (a 16320x16320 surface plus several 3840x2160 targets) does not fit a storage budget of 0.5–1.7 GiB; details are on #6256. A separate PR run with No regression: the control and PR ranges overlap on every column. AI-assisted: yes (Claude Code); results above were run on this machine. |
|
Correction to my Code Violet comment above: its setup said the runs used "identical shader-cache and save seeds". That was wrong. I have edited the comment.
The measured numbers are unchanged. The AI-assisted: yes (Claude Code); results above were run on this machine. |
|
Protocol note on my measurement above (comment 6337457417): it does not meet the performance-report protocol's "do not redirect the log" rule. The game's stdout and stderr were redirected to AI-assisted: yes (Claude Code) |
74b001a to
b996e31
Compare
|
Rebased linearly onto Both complete Windows/RTX 3060 Laptop 617.42 runtimes passed the same 26 focused tests with cache disabled and real Khronos synchronization validation. Main logged 26 synchronization errors in four cases despite passing CTest; this head logged none. That difference belongs to the included #8527 ordering fix. Its three additional ordering/failure-path tests also pass without skips or validation errors. Conventions pass, and inspected pre-push main The description records the exact tested commits. Historical GTA III timings retain their original commits; this rebase adds no new FPS claim. JJenkx's Code Violet observations remain diagnostic only, as corrected above. Prepared with AI assistance. |
|
@eliasbruno124-dev Draft #10194 adds opt-in allocation accounting for #10041. The overlap audit flags your storage-budget refresh/reclaim calculation and the adjacent test insertion in I propose keeping your existing merge order and rebasing accounting after yours lands. The resolution should preserve your cold-buffer reclaim and image exclusion, report the fresh budget after reclaim, and keep both sets of tests. Your PR does not need to wait for #10194. Does that order work for you? If accounting lands first, I can help integrate the reporting hook here. The current accounting branch builds independently and changes no cache policy; validation results are in #10194. AI-assisted: yes. |
|
@harry-z-kang agreed: keep #8527 -> #8672, then rebase #10194's accounting on the resulting code. #8672 does not gain a dependency on #10194. I inspected the accounting changes at Prepared with AI assistance. |
|
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 |
|
Checked the reclaim policy against the diff: only complete-idle-interval, independently allocated device buffers are released, oldest first and bounded by the shortfall; active, host, slab, retained-image and recently reused buffers are excluded; Vulkan frees happen outside the pool mutex; the budget is re-read only after an actual release. Tests cover bounded release, allocation failure during collection, backwards time, unsupported heap layouts, mixed image/buffer pools and a reused working set. Technical debt line and memory bound (retained pool 586 -> 298 MiB) are documented; #8527 is already in main. Not verified here: Linux/AMD/Intel timing; merged. |
|
A neutral observation on Echoes of the End (PPSA25636), as promised above. It is a single pair, not evidence for or against this PR. Setup
Over the last 300 s (in play)
0 crashes in either arm. Reading
|
ReclaimIdleDevice (boykopovar#8672) scans deviceTier for the oldest idle slots, and the host-placed device buffers this change retains live there too. An older host-placed slot could satisfy the requested byte count, so the reclaim freed host memory, reported it as released device memory and left the idle device-local slots in place. The scan now skips hostPlaced allocations. IdleReclaimSkipsHostPlaced retains an older host-placed and a younger device-local slot and checks that the reclaim frees and counts only the device-local one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
The storage-image cache can repeatedly evict surfaces while the device-buffer pool retains allocations the title has stopped using. GTA III reproduces this below the reported heap budget, where over-budget trimming does not apply.
On storage admission pressure, reclaim independently allocated device buffers unused during a complete one-second observation interval, oldest first and bounded by the admission shortfall. Reread the driver's budget after an actual release; the existing storage-budget formula, reserve, caps and forced-budget override remain in charge. Active buffers, host tiers, slabs, retained image allocations and recently reused buffers are protected. No fixed small pool cap is introduced.
Reclamation requires one device-local heap because retained buffers do not record their heap. The one-second retention choice and unmeasured effects on intermittent reuse are documented in technical debt.
Scope/order agreed with #4472 and #7833: cold-buffer release first; those branches rebase around it. This does not implement #6194's budget growth or #7833's in-flight image retention.
Tested
Current rebase validation
Head
b996e31d8c11956df72031e42d286e4682ebe1feis rebased linearly onto777bdbb4f80bcc03e870dfda0d292c0c6410fd6a, including the updated #8527 review correction (equivalent patches to itsa0ecec3d3head). The header conflict retains upstreamAgcDriver::Mutexand the idle-clock fields. The reclaim policy and retained-image exclusion are unchanged.On the Windows/NVIDIA system below, complete runtimes were built for that main and this head. The same 26 focused tests passed on both with
ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1, real Khronos synchronization validation andctest -V -j1 --timeout 30. Neither run skipped a case or missed the layer. Main nevertheless logged 26 synchronization errors in four tests:agc_import_settle_first_use,agc_import_settle_earlier_import,agc_storage_split_writebackandagc_driver_storage_write_back_overhang. This head logged none. These ordering corrections come from #8527, not the cold-buffer policy. Longest cases: 14.4429s on main and 12.9777s on this head.The three additional #8527 cases (
write_order,write_order_other_familyandwait_failure) also passed on this head without skips or validation errors. Conventions and whitespace checks pass. Pre-push main8eb8e3920was inspected and merges cleanly; it was not the tested runtime. Linux and AMD/Intel were not tested locally.The earlier mixed-pool regression fails without the retained-image exclusion and passes with it; image reuse, an image-only pool, repeated empty scans and exact cleanup remain covered. The title measurements below remain evidence for their explicitly named earlier commits. No new gameplay or performance measurement is claimed for this rebase.
Original title measurements
Windows 10.0.26300, i5-11400H, 32 GiB RAM, NVIDIA RTX 3060 Laptop GPU (6 GiB), driver 617.42. Original GTA III PPSA03527/01.003.000, 3840x2160, stationary first-mission spawn at Callahan Bridge with Claude, Kuruma, HUD and radar.
Pinned main
434e965d4620c48e315b25fb869a2d84b39cfb2dplus the unchanged #8527 dependency gives baseline9552b5e2ccbb297dccd3aba7d8df631bfa9094de; candidatea6c415f7ad63de979e16e39c898c0846ab34465cadds only this policy. Both use the same warmed cache/private-save seed and automatic navigation, default pools, complete runtimes and 61 verified loaded PRXs. Original metadata remained unchanged. No storage error occurred.APS5_TRACE_FRAMES=4440:10captures ten presents per side. Startup-only image exports ended at present 4410; navigation ended before that. No readbacks or input during capture, no additional TRACE/PROFILE options, no redirected game logs. Both traces are complete with no dropped events. The short window is corroborated by longer profiled diagnostics below; it is not a claim of steady gameplay or a general FPS gain.Median frame time fell 36.3%; p90 fell 43.2%. The scene remains slow (about 1.67 vs 2.67 presents/s across these short captures).
Driver.Packet.memory_waitfell from 517.818 to 166.197 ms/frame, inclusive; overlapping phases must not be added. GPU work remains substantial.Earlier paired 98.9/113.6-second diagnostic windows used profiling/readbacks: median logical storage-cache eviction writeback 10272.9 MiB/interval -> 0, retained device pool 586 ->298 MiB. Candidate released 348.875 MiB before and 111.5 MiB within the window; neither side acquired new device-pool allocations between the window's first/last counter rows. These are mechanism observations, not additional clean FPS measurements.
Both pinned runtimes pass the same 28 focused native cases using
ctest -V,ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1, and the realVK_LAYER_KHRONOS_validationwith synchronization validation. No missing-layer warning, VUID error or skipped case; longest cases 14.8167s baseline / 13.9844s candidate. Candidate regressions cover bounded release, repeated 384 MiB reuse, active/host/slab protection, allocation failure while collecting releases, backwards time, unsupported heaps and avoiding repeated empty scans. Conventions pass. Linux and AMD/Intel remain untested locally.Both complete
trace_digest.pyoutputs, including the rawaps5-summary.jsonlrecords, follow. The archive contains both raw traces, summary files and digests: gta3-cold-buffer-traces.zipBaseline: complete trace digest and summary
frames 4440:10 from present 4440: 10 presents, frame p50 587.4 / p90 705.2 / p99 705.2 / max 705.2 ms, GPU busy 298.7 ms per present
NVIDIA GeForce RTX 3060 Laptop GPU, NVIDIA 617.42, Windows 10.0.26300, 3840x2160, commit 9552b5e, dropped: no
{"commit":"9552b5e2ccbb","dirty":false,"env":{"APS5_DUMP_FRAMES":"147","APS5_DUMP_FRAMES_EVERY":"30","APS5_DUMP_SCALE":"4","APS5_LAZY_SHADER_PREPARE":"1","APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINE_JOURNAL_THREADS":"2","APS5_TRACE_FRAMES":"4440:10"},"gpu":"NVIDIA GeForce RTX 3060 Laptop GPU","driver":"NVIDIA 617.42","os":"Windows 10.0.26300","cpu":"11th Gen Intel(R) Core(TM) i5-11400H @ 2.70GHz","cpu_threads":12,"memory_mib":32509,"title_id":"PPSA03527","title":"Grand Theft Auto III \u00e2\u0080\u0093 The Definitive Edition","content_version":"01.003.000","resolution":"3840x2160","window":{"spec":"frames 4440:10","first_present":4440,"ms":5988.938},"presents":10,"frame_ms":{"p50":587.394,"p90":705.173,"p99":705.173,"max":705.173},"gpu_busy_ms_per_present":298.705,"worker_busy_pct":{"queue worker 0x0":99.262,"queue worker 0x20":89.756},"top_phases_ms_per_frame":{"Driver.Packet":1129.534,"Driver.Packet.memory_wait":517.818,"Graphics.Draw":321.549,"Graphics.Draw.draw_and_resource_release":302.126,"Driver.Packet.device_lock":249.122,"Driver.Packet.dispatch":216.385,"Resources.Complete":212.253,"Driver.Dispatch":204.801},"trace_dropped":false,"trace":"aps5-trace-9552b5e2ccbb-22520.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.Packet 1117.2, Driver.Packet.memory_wait 517.8, Graphics.Draw 319.0, batch 315.4, Graphics.Draw.draw_and_resource_release 297.2, Driver.Packet.device_lock 245.2, Driver.Packet.dispatch 216.3, Resources.Complete 212.1, Driver.Dispatch 204.7, Vulkan.Dispatch 158.2
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
Candidate: complete trace digest and summary
frames 4440:10 from present 4440: 10 presents, frame p50 374.1 / p90 400.8 / p99 400.8 / max 400.8 ms, GPU busy 293.6 ms per present
NVIDIA GeForce RTX 3060 Laptop GPU, NVIDIA 617.42, Windows 10.0.26300, 3840x2160, commit a6c415f, dropped: no
{"commit":"a6c415f7ad63","dirty":false,"env":{"APS5_DUMP_FRAMES":"147","APS5_DUMP_FRAMES_EVERY":"30","APS5_DUMP_SCALE":"4","APS5_LAZY_SHADER_PREPARE":"1","APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINE_JOURNAL_THREADS":"2","APS5_TRACE_FRAMES":"4440:10"},"gpu":"NVIDIA GeForce RTX 3060 Laptop GPU","driver":"NVIDIA 617.42","os":"Windows 10.0.26300","cpu":"11th Gen Intel(R) Core(TM) i5-11400H @ 2.70GHz","cpu_threads":12,"memory_mib":32509,"title_id":"PPSA03527","title":"Grand Theft Auto III \u00e2\u0080\u0093 The Definitive Edition","content_version":"01.003.000","resolution":"3840x2160","window":{"spec":"frames 4440:10","first_present":4440,"ms":3739.319},"presents":10,"frame_ms":{"p50":374.071,"p90":400.796,"p99":400.796,"max":400.796},"gpu_busy_ms_per_present":293.601,"worker_busy_pct":{"queue worker 0x0":98.964,"queue worker 0x20":94.925},"top_phases_ms_per_frame":{"Driver.Packet":723.913,"Driver.Packet.dispatch":263.692,"Recorder.Sync":201.478,"Driver.Packet.device_lock":193.149,"Driver.Packet.memory_wait":166.197,"Graphics.Draw":85.945,"Driver.Packet.draw":76.704,"Driver.Draw":75.787},"trace_dropped":false,"trace":"aps5-trace-a6c415f7ad63-32836.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.Packet 710.1, batch 324.8, Driver.Packet.dispatch 263.6, Recorder.Sync 201.5, Driver.Packet.device_lock 189.2, Driver.Packet.memory_wait 166.1
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
Fresh pre-publication main
50e0624d7a902d1083368fafee4a019a10296e1eis checked separately for conflicts and changed inputs; it is not the commit measured above.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)