Repository navigation
perf(agc): bind a sparse descriptor heap's committed run in place from its import - #5249
Conversation
cb238fb to
c6e45b0
Compare
|
@MichonGoddijn231849 The overlap check lists #290. Every line except AI-assisted: yes (Claude Code). |
c6e45b0 to
c75fac9
Compare
|
Windows cross-test of the sparse-run change: author-preserved integration281506e81 plus older-layout CTest correction693182a5b65dde056084f6e6dad0b5199903ad96, https://github.com/Scramblrrrr/AnyPS5/tree/codex/first-light-sparse-run-import . Reviewed rebased headc75fac9a: sparse-run production differs fromc6e only by whitespace. Our stack already carries4515/4517 plus window-budget integrationc990fc176; it preserves non-sparse importAccess and the separate copied-read reuse checks. RTX5070Ti /Windows /GCC15.2 /Vulkan required: final8/8 controls PASS7.76s, including imported and opt-out copied sparse runs, mapped-run growth/reusable cut, import unmap/retirement, alias handovers, graphics/BDA. Targeted production-negative disables sparseRunImportsEnabled with fixtures unchanged: imported test FAILS (not bound in place), copied test PASSES. Restored/touched/rebuilt positives pass. Initial two CTest registrations failed DLL lookup in this older Windows layout, corrected before these results. No skips at the CTest level; this is not full CI or a fresh-current-main build. Our bounded title diagnostic matches the scope:12of16 sampled descriptors512MiB over10MiB covered by2 imported ranges. First title attempt on this port was guard-ended during startup with current host RAM pressure, before the scene or sparse-run samples, so no FPS/copy reduction claim yet. Guard/settings retained; investigating memory headroom before retry. Native evidence is independent of that incomplete title run. AI-assisted: yes (Codex); no title data attached. |
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: Head is |
|
On Windows the two new tests Built on AI-assisted (Claude Code). |
c75fac9 to
510f926
Compare
|
@Scramblrrrr @xXPBSXx Thanks for the Windows runs. This was a gap in the PR, not in the older layout: both new tests were missing the @blawesom Thanks for the RADV run. I'm keeping the branch on #4517's head f6c289e, so the #4517 commit here stays identical to AlxcBTW's. Your 49d2a5c matches c75fac9 patch for patch; 510f926 merges with current AI-assisted: yes (Claude Code) |
|
#290 is in now. It changed the import lookup in |
|
One more for the rebase: please drop |
510f926 to
9c8fd76
Compare
9c8fd76 to
a4d8c29
Compare
|
@oneandonlydean Rebased onto
Evicted joined runs, from the code (no test evicts a joined import yet):
AI-assisted: yes (Claude Code) |
…m its import A descriptor heap that a shader binds whole while only the pieces from its base are mapped (a sparse heap) is copied on the CPU for every build that binds it unless a mirror serves it. The sparse heap mirror takes a heap whose needed extent lies in one releasable registered range. A heap mapped piece by piece, each mapping its own allocation and the pieces back to back from the heap's base, is several registered ranges, so every build copied its mapped run again. Such a region is now bound in place from an import of that run: an import already covering it, the run's own import, or one joined over exactly the run (boykopovar#4517). This applies to any sparse heap no mirror took, including under APS5_NO_LEASE_MIRROR or APS5_NO_SPARSE_HEAP_MIRROR. A join is made only for a region that starts at a registered range, once the previous bind of that address saw the same last mapped byte: a heap that grows between every two of its binds keeps the copy, and one bound more often than it grows is joined again after each growth. Joins stop after 64 per joined run base, with a message: the heap is then bound in place while an existing import covers its run and copied otherwise. The records go with their range. The region's descriptor and BDA ranges end at the import's end, which covers the mapped run and the last view, as the copy does. A region whose mapped part does not start at its base, with a view past the run, written by its shader, or whose run is not imported is copied as before. A direct region whose import ends before it outside this path still fails in Descriptor, as before. A set over such a region stays reusable: DirectRegions and InPlaceReads name the region up to its import's end, so the import's serial proves it, and a reuse also requires the heap to be mapped exactly up to that end (CutRegions), checked whenever the imports or the registry changed, so a mapping past the cut makes the set miss. DirectRegions also refuses a set any of whose regions bound in place no longer holds the import found at its base (a later join in the same build retired it); this applies to every region bound in place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a4d8c29 to
d34564b
Compare
|
@adriadam10 Thanks. The list of failing lines came through empty in your comment (the same empty list went to six other PRs between 10:32 and 10:37 UTC). The description passes AI-assisted: yes (Claude Code) |
|
Checked the head merged onto current |
What
A descriptor heap that a shader binds whole while only the pieces from its base are mapped (a sparse heap) is copied on the CPU for every build that binds it, unless a mirror serves it. The sparse heap mirror (#3167) takes a heap whose needed extent lies in one releasable registered range. A heap the title maps piece by piece, each mapping its own allocation and the pieces back to back from the heap's base, is several registered ranges, so every build copied its mapped run again.
Such a region is now bound in place from an import of that run: an import that already covers it, the run's own import, or one joined over exactly the run (#4517).
APS5_NO_LEASE_MIRRORorAPS5_NO_SPARSE_HEAP_MIRROR.mainsince perf(agc-driver): join the imports of adjacent ranges of any size that a buffer is bound exactly over #4517. Joins stop after 64 per joined run base, with one message; from then on the heap is bound in place only while an existing import covers its run, and copied otherwise. The records go when their range does. The host-imports debt entry says so.Descriptor, as onmain.A set over such a region stays reusable in the resource cache:
DirectRegionsandInPlaceReadsname the region up to its import's end, so the import's serial proves it. Without that, a set over a region bound in place would stay unreusable, as one over a copied region is onmain, because no import covers the whole region.CutRegions). That check runs whenever the imports or the registry changed, which every mapping does, so a mapping past the cut makes the set miss instead of reading zeros through it.DirectRegionsrefuses a set any of whose regions bound in place no longer holds the import found at its base (a later join in the same build retired it). This applies to every region bound in place.There is no switch: the fallback is the existing copy path for every case it does not take.
Tested
Linux (CachyOS, kernel 7.2.9), GCC 16.2.1, Release. Windows was not built or run.
New test
sparseRunImportTestsin the recorder tests, asagc_sparse_run_importandagc_sparse_run_import_copied. Three 64 KiB allocations mapped back to back at the start of a 512 KiB range whose tail is unmapped:DirectRegions,InPlaceReadsandCutRegionsnaming the cut, and a GPU copy through the view returning the guest bytes;agc_sparse_run_import_copied: once the run has been seen, a bind with a view past the run is copied, nothing is read in place, and a GPU copy through the view returns the guest's mapped bytes.Red/green on lavapipe (Mesa llvmpipe): the in-place test fails on this branch with the binding removed ("a sparse run heap is not bound in place from its run's import"), with the cut reverted (
Descriptorrefuses the view), withDirectRegions/InPlaceReadsnaming the whole region ("… is not named up to its import's end for reuse"), without the reuse check of the mapped extent ("a set over a sparse run heap mapped past its cut was reused") and without the wait for an unchanged run ("a sparse run heap was joined the first time its run was seen"). It passes with the change. On a4d8c29, with the last view ignored (neededEndset to the committed end), both tests fail ("a sparse run heap with a view past its run is bound in place", "… past its stable run is bound in place"); the five earlier reds were run on the in-place test before the switch was dropped.Suite on lavapipe,
ctest -j 8 --timeout 300, on an earlier base,main0f2683c: 1035 of 1046 pass with the change and 1032 of 1044 without. The same 11 fail on both in that container (hardware_oracle,guest_syslog,agc_driver_depth_compare_copy,autorun_exit_status,guest_none_relocations,guest_module_argsand fivelinux_*executable tests);guest_audio_in_captureaborted once onmainunder the parallel run and passes alone on both. Onmaind1b3aa1, which has perf(agc-driver): join the imports of adjacent ranges of any size that a buffer is bound exactly over #4517 and feat(agc): host import windows, least-recent eviction under the budget #290, the two new tests and 494 of 496agc_tests pass on 9c8fd76, and 492 of 494 onmainalone; the same two fail on both in that container:agc_driver_depth_compare_copyandagc_driver_bc_block_copy(the guest block is not write-watched there). a4d8c29 (onmaina5f493f, the switch dropped) gives the same: both new tests pass and 494 of 496agc_with the same two failing.tools/check_conventions.py --base upstream/mainandtools/check_test_registration.pypass.Title: 007 First Light (PPSA11386), NVIDIA GeForce RTX 3080 (615.71.09). The title does not reach the scene on
mainalone, so the numbers come from one build of our 007 integration stack (upstream/main3e0e01f of 2026-10-12 plus 164 commits of our 007 integration work, including cross-tests of feat(agc): implement image and sampler tables indexed at runtime offsets #757, feat(agc): implement image tables loaded through 64-bit pointers #758, perf(agc): pipeline queue-0 draw commits behind the packet worker #1201, perf(agc-driver): join the imports of adjacent large ranges when a buffer is bound across them #4515, perf(agc-driver): join the imports of adjacent ranges of any size that a buffer is bound exactly over #4517, fix(agc): wait for recorded writes over a sampled texture's DCC keys before reading them #4554, fix(agc): preserve storage image dependencies during ahead dispatch capture #4724 and fix(agc): store small pending storage results at each flip #4841 (perf(agc-driver): join the imports of adjacent large ranges when a buffer is bound across them #4515, perf(agc-driver): join the imports of adjacent ranges of any size that a buffer is bound exactly over #4517 and fix(agc): wait for recorded writes over a sampled texture's DCC keys before reading them #4554 have merged since and fix(agc): preserve storage image dependencies during ahead dispatch capture #4724 was closed)), whose version of this change has this PR's behaviour behind the stack's own opt-in switchAPS5_SPARSE_RUN_IMPORTand prints counters of what it did. In the run with the change on, binds waited for an unchanged run in 10 of the 142 ten-second reports (1 to 5 binds each: four before the cinematic, five in its first 35 frames and one later), and no heap reached the join limit. With that switch off and on, from the same save, warm caches, 1500 s each; neither run crashed:The copied regions are the title's two descriptor heaps, each a 512 MiB view over 6–42 MiB of mapped pieces: about 30 GiB copied per 10 s without the change.
In each run's 60 s trace window (900–960 s: frames 248–263 of the cinematic with the change, 126–139 without), a resource build (
Resources.Complete, all threads) took 1.8 ms on average with the change against 4.9 ms without; builds per frame were about equal (619 against 570). The trace with the change dropped events: queue worker 0x0 reached its event limit at 50.9 s, so its window ends at the last present before that. The windows are at different frames, so the runs' summary lines are not a before/after pair (p50 frame time 4,598 ms with the change, 4,199 ms without).Later stretches of the cinematic vary a lot between runs; on earlier builds of our integration line without this change, the first 150 frames sat at 0.20–0.24.
Capture: 3840x2160. Both runs set further
APS5_TRACE_*switches,APS5_PROFILE_VRAM=1andAPS5_DUMP_SHADERS=1, and redirected the log, unlike the Performance report form; their environments differ only in the integration stack's switch for this change.The integration stack also carries a local change that keeps draws over copied read-only buffers reusable (sparse heaps included), so the column without this change is better than
main's copy path for these heaps.Summary lines, with
envreduced to the integration stack's switch for this change (the traces are attached asaps5-trace-on.json.gzandaps5-trace-off.json.gz):With the change:
{"commit":"d5422af08d7e","dirty":false,"env":{"APS5_SPARSE_RUN_IMPORT":"1"},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.71.09","os":"Linux 7.2.9-1-cachyos","resolution":"3840x2160","window":{"spec":"seconds 900:60","first_present":1629,"ms":62328.397},"presents":16,"frame_ms":{"p50":4598.014,"p90":5235.786,"p99":5268.656,"max":5268.656},"gpu_busy_ms_per_present":571.314,"worker_busy_pct":{"queue worker 0x0":99.936,"queue worker 0x20":99.941,"queue worker 0x38":99.986,"queue worker 0x28":98.878,"queue worker 0x30":99.98},"top_phases_ms_per_frame":{"Driver.Packet":19384.613,"Driver.Packet.memory_wait":15189.661,"Driver.Packet.draw":3108.319,"Driver.Draw":3107.283,"Graphics.Draw":2830.808,"Graphics.Draw.draw_and_resource_release":2770.853,"Committer.Commit":2644.087,"Driver.Draw.draw_enqueued":1842.702},"trace_dropped":true,"trace":"aps5-trace-on.json.gz"}Without the change:
{"commit":"d5422af08d7e","dirty":false,"env":{},"gpu":"NVIDIA GeForce RTX 3080","driver":"NVIDIA 615.71.09","os":"Linux 7.2.9-1-cachyos","resolution":"3840x2160","window":{"spec":"seconds 900:60","first_present":1498,"ms":60806.67},"presents":14,"frame_ms":{"p50":4199.032,"p90":4999.966,"p99":5253.996,"max":5253.996},"gpu_busy_ms_per_present":634.232,"worker_busy_pct":{"queue worker 0x0":99.929,"queue worker 0x20":99.797,"queue worker 0x38":99.981,"queue worker 0x28":99.982,"queue worker 0x30":99.989},"top_phases_ms_per_frame":{"Driver.Packet":21644.861,"Driver.Packet.memory_wait":16467.943,"Resources.Complete":2785.334,"Driver.Packet.dispatch":2565.201,"Driver.Dispatch":2563.671,"Driver.Packet.draw":2380.272,"Driver.Draw":2379.792,"Graphics.Draw":2348.481},"trace_dropped":false,"trace":"aps5-trace-off.json.gz"}InPlaceReadssits next to #4978'sSplitInPlaceReads. At #4978's current head a6970f1 the two conflict only on theif(WIN32)PATH list line inlibSceAgcDriver/CMakeLists.txt, where #4978 addsagc_recorder_read_listand this PR its two tests.SplitInPlaceReadsnames its own regions up toregion.end; whichever merges second should end them atinPlaceEnd(region), asInPlaceReadsdoes here.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)agc_, on d34564b (its rebase ontomain3e577ed; only theif(WIN32)test list merged) both new tests; NVIDIA GeForce RTX 3080, driver 615.71.09, Linux (CachyOS, kernel 7.2.9): at 510f926, before the rebase onto feat(agc): host import windows, least-recent eviction under the budget #290 and the switch removal, both new tests passed and noagc_test failed (487 run,agc_driver_stencil_ref_exportskipped itself), and the title runs on our integration stack with this change behind that stack's opt-in switch🤖 Generated with Claude Code
https://claude.ai/code/session_01Fnrk5Dazjb2tLooVSpdRQq