Repository navigation
perf(agc): find the address space regions under pending writes by search - #5465
Conversation
CnRJay
left a comment
There was a problem hiding this comment.
Looks correct to me, and the measurement is solid: five alternated pairs, summary lines and traces, with Resources.Complete as the phase that moved.
I checked the equivalence the description relies on. CompletePendingCpuWrites returns early when !recorder->PendingWriteOverlaps(address, bytes) (ShaderResources.cpp, after the writer and label pre-checks). So skipping base regions that no pending write overlaps drops only calls that did nothing. Re-taking the pending writes after a sync and continuing past that region handles writes that the completion retired or noted. The mutation tests pin both.
One small follow-up idea, not needed for this PR: each remaining call still repeats the pre-checks that don't depend on the region (DrawCopiedWriters() copy, PendingCompletionLabels(), the copiedWriters emptiness test). Hoisting them out of the UploadFinish walk, and skipping the walk entirely when there are no copied writers, draw writers or labels, would remove the PendingWriteRanges snapshot in the common case too.
1eedbd6 to
492d61b
Compare
492d61b to
100ef2e
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
100ef2e to
1ce75da
Compare
What
An address-based build that takes the cached address space completes pending CPU writes over the space's regions in
GuestBufferMemory::UploadFinish. It calledCompletePendingCpuWritesonce per base region: each call copied the draw writer list, read the label count and scanned every write of the open and in-flight batches. The space holds a few thousand regions, so on The Pathless's main menu (about 45 address-based builds per frame) that walk took 189 µs per build, 8.4 ms per frame.Now
UploadFinishtakes the pending writes once (Recorder::PendingWriteRanges: the writes of the open batch and of the batches in flight, the same setPendingWriteOverlapsreads), finds the base regions they overlap by binary search (OverlappedRanges; the base regions are sorted and disjoint), and callsCompletePendingCpuWritesfor those regions only, in ascending order as before. A region no pending write overlaps returned from that function before doing anything, so it is skipped without a call. When a call syncs, the pending writes are taken again and the walk continues after that region, since the completions it ran may have retired writes or noted new ones.CompletePendingCpuWritesis unchanged except that it returns whether it synced. The build's own regions keep the old loop.Tested
Windows 11, RTX 5070 Ti, WinLibs GCC 15.2.0 r7, Release.
New
agc_space_pending_writes(tests/Recorder.cpp):OverlappedRangesexactly whenPendingWriteOverlapsreports it, from several starting indices;Full
ctest(Windows, RTX 5070 Ti): 1097 of 1098 pass, 8 skipped.agc_driver_wave32_subgroupfails (thread 0 s_bcnt1 of a VCC compare is 0, expected 8) and fails the same way here with this change reverted.python tools/check_conventions.py --base upstream/mainpasses.CI on the fork, same steps as
build.yml(run 38451464704): Linux and Windows pass,agc_space_pending_writesincluded. The first Linux attempt lostguest_ampr_threads(mprotect overlaps host-only guest thread stack pages, nothing this change touches); the rerun passed.The Pathless (PPSA01825, 02.000.000), main menu,
main61b8742 + fix(libScePlayGo): answer chunk ids from the package layout #4491 (needed to reach the menu then; merged since),APS5_TRACE_SECONDSwindow of 12 s, warm caches, 5 valid runs per side alternated base/change (runs lost to the Wwise crash of Windows: resuming a write fault overwrites the guest red zone when AVX state is live #361 repeated):Resources.Completeon queue worker 0x0mainThe change was faster in each of the five base/change pairs (by 5.3 to 15.4 ms). One
APS5_PROFILE_DRAWrun per side: the draws' upload phase went from 827 ms to 44 ms per 10 s (8.4 to 0.4 ms per frame, 189 to 9.5 µs per address-based build), andCompletePendingCpuWritesfrom 4.7 % to 0.1 % of the worker's samples; the zones the change does not touch stay within run-to-run noise. The rest of the frame-time difference (about 3 ms above the upload phase) I count as noise: the base runs drifted from 104 to 116 ms during the batch.Summary lines of the ten runs,
mainfirst (commitis the local merge ofmain61b8742 + fix(libScePlayGo): answer chunk ids from the package layout #4491, 60c13b89eb20, and the same with this PR, 1e9e0534448d); the ten traces are in the attached zip.aps5-summary.jsonl
LEGO Horizon Adventures (PPSA14632, 01.004.000), intro,
main61b8742, 3 valid runs per side: 127.7 ± 1.6 against 128.9 ± 2.2 ms per frame, no change (2 address-based builds per frame).Hades (PPSA03355), ANIMAL WELL (PPSA22520), Balatro (PPSA21402) and Dead Cells (PPSA15552),
main61b8742 against this PR, 90 s each with 8 dumped frames: the same points reached (the first three keep running at their menus, Dead Cells stops at the sameNGS2: rack id 0x4001of Dead Cells (PPSA15552): NGS2 rack id 0x4001 is not implemented #3553), the same set of[gpu]log lines, ANIMAL WELL's frames identical; Hades's and Balatro's frames differ only where their animations (Hades's logo fade-in, Balatro's animated menu) were when dumped, which I checked by eye.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)space-pending-writes-traces.zip