Repository navigation
fix(agc): bound the draw cache by bytes - #9129
Conversation
|
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.
|
|
NVIDIA, Windows, where the new case runs instead of skipping. GeForce RTX 3080, driver 616.92, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release,
Not run in a title. AI-assisted: yes |
|
@gdols, thank you for running the new case on a GPU under the validation layer, with the revert check. It's now in Tested. |
|
@EagleBearerYS a second title for this PR: Ghost of Tsushima Director's Cut (PPSA02225), on your
This title still runs out of its cap in the opening, because other things grow there too: the NVIDIA driver, specialized modules and materialized results. The bound does what it says, though, and takes the draw cache out of that list. AI-assisted: yes (Claude Code) |
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the scope and current evidence: the PR adds a byte budget alongside the entry limit, expands per-variant accounting, and includes a focused over-budget regression. The report now includes a Windows validation-layer run with an over-budget negative control and an independent Ghost of Tsushima measurement; hosted policy checks pass, while product jobs are skipped. Local diff whitespace, conventions, and registration checks pass. I could not run Vulkan tests in this host.
I cannot complete an integration review against the published branch yet: tools/check_main_sync.py --base upstream/main --head <head> reports it is 2 commits ahead / 1257 behind current upstream/main (merge base fce8ecf16), with a clean merge. Driver.hpp and ENVIRONMENT.md also changed on main since that base (#7233 and #9026). Please merge current main, preserve those current changes and the declared overlaps, and rerun the focused cache-bound test plus conventions on the resulting head. This is based on the repository's current-main branch rule, not on the open-PR count.
|
@milescoviello thank you for the Ghost of Tsushima Director's Cut (PPSA02225) pair and the jemalloc profile. The 1,011 MiB under |
73ec16a to
a001a59
Compare
a001a59 to
45876fa
Compare
45876fa to
fa02afe
Compare
|
@adriadam10 I re-authored your squashed commit with my GitHub noreply address (fa02afe, was 45876fa), so the commit links to my account. Your squash and the tree are unchanged: |
|
@EagleBearerYS a measurement of the relocation-alias limit in your Limits section, from 007 First Light (PPSA11386 01.018.000). Linux, RX 9070 XT, RADV Mesa 26.2.5, Release; With this PR, I sampled live heap allocations by call stack with a small
|
|
@hugodearaujo thank you, that measures the case the Limits section only described. Since #9396 (merged 2026-10-24, after the |
|
@EagleBearerYS the rerun on a Estimated live MiB at 14 minutes:
|
|
@hugodearaujo thank you for the rerun; |
|
@EagleBearerYS Thanks. This now conflicts with |
# Conflicts: # core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawCacheStatistics.cpp
|
@oneandonlydean merged |
|
@EagleBearerYS a Windows run with a Vulkan device, for the five tests your Windows runner skips. Setup: a Windows 11 VM with a GT 1030 (driver 582.78), WinLibs GCC 15.2.0, Release.
AI-assisted: yes (Claude Code) |
|
@milescoviello thanks, that covers the five tests GitHub's Windows runner skips, and on both trees. I've added your run to Tested. |
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
|
Re-checked after |
What
The dispatch cache evicts by entry count or by bytes: past
APS5_DISPATCH_CACHE_BYTES(256 MiB by default) it drops its least recently used entries. The draw cache evicts only by entry count, at 4,096 entries, so it holds whatever those entries capture.When a draw's SRT walk captures a large table of guest addresses, each variant keeps a copy of it. In Resident Evil Village (PPSA01556)'s main menu, on a local tree of
mainplus our open PRs, the draw cache's own[draw-cache]profile line went from ~21 MiB to ~2,149 MiB of variants (4,096 entries, ~16,000 variants) at a ~4% hit rate.The change:
insertDrawEntryevicts from the LRU end whilesize > drawCacheEntries() || (size > 1 && drawCacheVariantBytes > DrawCacheBytes()), the dispatch cache's condition.size > 1keeps the entry just inserted, as the dispatch cache does.APS5_DRAW_CACHE_BYTESsets the budget: 256 MiB by default, 0 keeps the default, read once. Its row is inENVIRONMENT.md.[draw-cache]lines, e.g.126 entries (252 variants, ~254.3 MiB of a 256.0 MiB budget), 1473 evictions in total (1473 over the byte budget).Driver::DrawCacheSize()returns entries and bytes under the cache's lock, the twin ofDispatchCacheSize().What the count covers.
variantBytes, shared by both caches, counted the sizes of four vectors. It now counts the capacities of every vector a variant owns (runs, words, shared, data positions and slots, run bases, shader data patches, captured words and spans), plus aShaderMemory's pages for the stamp-validated dispatch variants that keep one. Draw variants keep none. Measured per draw variant:wordsgrows without areserve.No variant changes size after its insert, so the add and the remove compute the same number.
Two budgets, not one shared. The caches have their own locks and LRU lists. A shared budget would need a victim policy and a lock across both, and a phase heavy in one kind of work would evict the other's entries. The worst case is 512 MiB, and each budget can be tuned.
Limits:
shared_ptrs to entries, so an entry evicted while an alias named it stayed alive outside the count: @hugodearaujo measured about 2 GB held that way at 14 minutes in 007 First Light (PPSA11386) onmain11e8ad2 (see Tested). Since fix(agc): let draw relocation aliases name a cached key, not hold its entry #9396, onmainsince 2026-10-24, an alias names a cached key and a lookup drops it once its entry is evicted, so this no longer applies when this PR merges: his rerun onmainf95acfc keepstagVariantRunsat about 253 MiB with this PR (see Tested).DrawEntry, its decode and recipes stay outside the budget, as before.Speed, from a scratch bench on Linux with lavapipe (fork commit 413c412's
VillageDrawCost, with each draw's T# offset taken from a lane value so every variant captures ~1 MiB; 850 draws per frame over 6,800 keys, single runs):mainAPS5_NO_DRAW_SRT_ENTRIES=1)Overlap: checked with
git merge-treeagainst the 24 open PRs touchingDraw/DrawCache.cpp,Draw/Draw.cpporDraw/DrawCacheStatistics.cpp. This PR adds conflicts only with these:Driver.hppmember line; keep both sets of fields.evictDrawEntries, so the byte clause and its counters would move there.[draw-cache]append line. Whichever merges second keeps both insertions.The rest conflict with
mainalready or merge cleanly.Follow-up, not in this PR:
tagVariantRunscould reserve its total word count first, as the dispatch insert does: −25% ofwordscapacity on the stride-48 table.Tested
New case
SrtMovingPerDrawin the existingagc_driver_image_table_cache_bounds, besideSrtMovingPerDispatch. 64 keys, each drawn twice (a key is inserted on its second sighting), each capturing its own ~2 MiB of a 2 MiB image table, under a 16 MiB test budget. It checks after every draw that the cache stays within the budget, that the color comes from the table's T#, that evictions happen and are all byte evictions, and that an evicted key is looked up absent, inserted again and then hit with updated records.Linux (Ubuntu 24.04, GCC 13.3, lavapipe, CI's flags), on the same diff before its rebase onto
mainfce8ecf:main;^agcthree times: 860, 862 and 860 of 862, against 861, 861 and 862 onmain. The failures areagc_driver_pipelined_fill,agc_driver_storage_write_back_no_windowandagc_driver_sgpr_pattern_fill_keys, timing tests that fail onmaintoo and don't touch the draw cache;main, the same six network failures (no IPv6 on that machine);GitHub's runners (run, same diff before the rebase):
main's code the case doesn't build, because it uses the newDrawCacheSize().Windows with an NVIDIA GPU, where the case runs instead of skipping, by @gdols (comment): GeForce RTX 3080, driver 616.92, Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release,
main80565c5 (with test(agc): align the array window test's texels at run time #8310 to build) and this PR at f2c368b, withANYPS5_REQUIRE_VULKAN=1andANYPS5_NO_SHADER_CACHE=1.agc_driver_image_table_cache_boundsunderVK_LAYER_KHRONOS_validation(SDK 1.4.363.0): 3 of 3 with no validation messages.SrtMovingPerDrawreports the green line above.^agc_: 870 passed, 3 skipped and 3 failed, the same as onmain. They name the three asagc_driver_sample_order*, which they say test(agc): read the sample order tests' depth through a sampled copy #9056 fixes.MinGW
-fsyntax-onlyon the five changed sources: no errors.check_conventions.pypasses, with no comment lines added.Resident Evil Village (PPSA01556), static main menu, 11 min without input.
APS5_LAZY_SHADER_PREPARE=1,APS5_NO_GPL=1,APS5_HOST_IMPORT_MIB=6400,APS5_PROFILE_DRAW=1).main1ff5c8a with fix(agc): put a retired import's unchanged ranges back in the write watch #8926. Arm B adds this change. The PR itself is on currentmain.[draw-cache]profile line, from its entry count on:4096 entries (18017 variants, ~2596.3 MiB), 3655 evictions in total, 18 LRU moves282 entries (564 variants, ~255.0 MiB of a 256.0 MiB budget), 25447 evictions in total (25447 over the byte budget), 0 LRU moves[draw-cache]at the endMerged with
main0db47b2 (73ec16a). One conflict, in the[draw-cache]line inDrawCacheStatistics.cpp: the merged line keeps perf(agc): draw finisher pipeline (APS5_DRAW_PIPELINE) #6946's shape-update count and this PR's byte budget and byte-budget evictions (format and arguments checked, 30 and 30). On GitHub's runners (run),agc_driver_image_table_cache_boundsand the sevenagc_driver_draw_cache_*tests pass 10 runs in a row on Linux, none skipped; on Windows all 8 build and skip, as the runner has no Vulkan device. The libc and libkernel suites pass on both, run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.Ghost of Tsushima Director's Cut (PPSA02225), by @milescoviello (comment): Linux, RTX 4070 Ti, warm cache, a 20 GiB cap, on a stack with feat(agc): draw one-sample color targets beside a multisampled depth target with sample 0's coverage #8115, fix(agc): draw a PS_ITER_SAMPLE program once per pixel into a one-sample color target #8121, fix(agc): read the pipeline journal's modules at replay instead of holding the file #8213, perf(agc): take captureValidation's records in one pass over the lookup log #8608 and fix(agc): bound sampled array views to guest allocations #9318, with and without this PR at f2c368b.
main's count within 10 minutes of the opening. A jemalloc profile of the same phase, at 1,158 s, put 1,011 MiB live underDriver::tagVariantRuns(fromcacheDrawStages), somain's count undercounts these variants.007 First Light (PPSA11386) 01.018.000, on Linux with an RX 9070 XT (RADV, Mesa 26.2.5, Release), by @hugodearaujo on
main11e8ad2 with this PR's code (comment), 15 minutes on the First Launch Options screen per run:[draw-cache]holds the cache at its budget (688 entries (1447 variants, ~255.6 MiB of a 256.0 MiB budget)), with 47,412 byte-budget evictions in 12 minutes. Estimated live heap at 14 minutes: 11,351 MiB onmain, 7,272 MiB with this PR, and 5,294 MiB with this PR andAPS5_NO_DRAW_RELOCATION=1. hugodearaujo reads the difference between the last two as evicted entries that relocation aliases still named, the case in Limits; his rerun below, on amainwith fix(agc): let draw relocation aliases name a cached key, not hold its entry #9396, no longer shows it.007 First Light (PPSA11386) 01.018.000, rerun by @hugodearaujo on
mainf95acfc, which has fix(agc): let draw relocation aliases name a cached key, not hold its entry #9396, alone and with this PR's head fa02afe merged (clean), one run each, set up as before, 15 minutes from launch with X pressed every 30 s (comment). Estimated live heap at 14 minutes: 12,170 MiB onmainand 7,329 MiB with this PR; allocated intagVariantRuns: 3,004 MiB and 253 MiB. With this PR,tagVariantRunsholds 251-254 MiB from 3 minutes to the end, where on 11e8ad2 it climbed to 1,213 MiB, so with fix(agc): let draw relocation aliases name a cached key, not hold its entry #9396 relocation aliases no longer keep evicted entries alive in this title.Graphics::DrawundercommitDraw: 1,227 MiB onmain, 172 MiB with this PR. RSS at 14 minutes: 14.6 GB onmain, 9.2 GB with this PR. He observes that most of what remains with this PR is storage-texture upload buffers, about 4.9 GB in both runs.332b5f3, a merge of
mainf1f1737 (run), as oneandonlydean asked. The one conflict was inDraw/DrawCacheStatistics.cpp, wheremainaddedDriver::OverlayCounters()at the place this PR addsDriver::DrawCacheSize(); the merge keeps both functions, and this PR's own added and removed lines are the same.agc_driver_image_table_cache_bounds,agc_driver_pipelined_fill,agc_driver_sgpr_pattern_fill_keys,agc_driver_storage_write_back_no_window,agc_performance_overlayandagc_performance_overlay_statisticspass 10 runs in a row. Linux: 6 run, 6 pass, 0 skip, 0 fail. Windows: 6 run, 1 pass, 5 skip (the Vulkan tests, with no Vulkan device), 0 fail. Suites: libc and libkernel pass on Linux (161 and 264). On Windows libc passes (189) and libkernel passes 283 of 286:exec_arguments,guest_timed_join_import_libkernel_sysandguest_timed_join_import_libkernel_webfail at their setup in the verify's Windows build (FileNotFoundErrorstarting the title, or copying the libraries they load), and pass inmain's own Windows job at 1829def (job). The suites ran withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.Windows with a Vulkan device, by @milescoviello (comment): a Windows 11 VM with a GeForce GT 1030 (driver 582.78), WinLibs GCC 15.2.0, Release. 332b5f3 and its merge base,
mainf1f1737, each ran the six tests above 3 times, and all six pass in every run on both. The Vulkan ones run there instead of skipping, at 1 to 7 s each.Checklist
mainfce8ecf; 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)