Repository navigation
perf(agc): check a mid-decode vertex table store against the queued write set and the retired items' writes - #10677
WallisSkill wants to merge 1 commit into
Conversation
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The retired-write ring and conservative overflow handling look internally consistent on the reviewed diff, and the published gk2 trace assets are present. I cannot approve this AGC change yet because the repository's AGC validation rule requires the same focused tests on main and this head with ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1, and VK_LAYER_KHRONOS_validation, then a comparison confirming no new VUIDs. The PR currently reports the layer is unavailable and only reports branch test counts. Please attach both runs from a layer-equipped machine/GPU (including confirmation the layer was actually loaded), or explain what supported validation route can satisfy the rule.
The contribution guide also requires tools/trace_digest.py output for performance claims. The release has the traces and per-capture JSON summary assets; please include the digest output for the paired captures (or the prescribed report output) so reviewers can reproduce the presented comparison. I downloaded the base-1 and pr2-1 trace assets and confirmed the digester reads them; its output reports 147 and 155 frames respectively, but no summary line was found when run against the release asset layout.
Finally, this diff conflicts with open #10678 in Draw.cpp, and both PRs currently say Depends on: none. Please coordinate the landing order with the other PR's author and record the dependency on the checklist before these overlapping changes are integrated.
|
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.
|
…rite set and the retired items' writes DecodeOrderedVertexStageInfo copied the write ranges of every queued item under the pipeline mutex before each vertex stage decode, and scanned every queued item again afterwards, so the queue worker paid for the pipeline depth on every draw. The watch now only notes the retired count; the check reads the deduplicated queuedWrites set and the writes of the items that retired during the decode, kept in a 512-entry ring by the committer. More than 512 retirements count as written. Graveyard Keeper 2, first dungeon room, APS5_PIPELINED_DRAWS 256: frame p50 67.3-68.8 ms on main, 63.3-63.8 ms with this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
864292e to
7b38e94
Compare
|
Thanks for the review. Addressed:
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head 7b38e94968eb760135eadcd3a0daf197d6b62a5f. The retired-write ring is indexed consistently with the monotonically increasing retired count; if more than 512 items retire during a decode, WrittenSince conservatively reports a write and forces the retry. The regression test covers queued and retired writes, and its described mutant fails. The author has now documented the same 38 focused AGC tests on main and this head with the validation layer confirmed loaded; both runs passed with identical VUID counts and no new VUIDs. The paired performance report now includes trace_digest.py summaries and the trace assets. Current hosted conventions, labels, eligibility and report checks pass; product CI jobs are skipped by policy.
What
DecodeOrderedVertexStageInfo(from #7691) watched queue 0 for a store that lands during a vertex stage decode by copying the write ranges of every queued item under the pipeline mutex before each decode, and by scanning every queued item again afterwards. With a deep pipeline that is work per queued item on every draw, on the queue worker, under the mutex the committer takes for every item it retires. In Graveyard Keeper 2 withAPS5_PIPELINED_DRAWS=256(sampled onmainwith #9508 applied) the queue worker spent about 14% inDecodeOrderedVertexStageInfo, 8% inDrawPipeline::WrittenSinceand 3% inDrawPipeline::WatchWrites.Now
WatchWritesonly notes the retired-item count.WrittenSincechecks the reads against the deduplicatedqueuedWritesset (the ranges of the items still queued, kept bycountWrites) and against the writes of the items that retired since the watch, which the committer keeps in a 512-entry ring indexed by the retired count. More than 512 retirements during one decode count as written, so the decode drains and runs again as before. Writes still held by the draw finisher count as queued, as before.agc_driver_vertex_table_read_ordercovers both cases; a mutant that skips the retired-writes check fails it ("the decode kept the vertex buffer of an attribute word that a queued store replaced during the decode").Tested
Windows 11 Pro 10.0.26200, AMD Ryzen 7 9800X3D, NVIDIA GeForce RTX 5070 (driver 616.92), MinGW Release build.
Performance: Graveyard Keeper 2 (PPSA35943), the first dungeon room after New game, 1920x1080 window, captured with
APS5_TRACE_SECONDS=120:10andAPS5_DRAW_PIPELINE=1 APS5_PIPELINED_DRAWS=256 APS5_PIPELINE_DISPATCH=1 APS5_MIMALLOC=1, no otherAPS5_TRACE_*orAPS5_PROFILE_*switch and no log redirection. Base ismain3fcb5e4; the change was built at 0c11c4bf8, which differs from this branch's head only in the commit message. Each build ran the scene once to warm the caches, then the captures below ran back to back on the same machine; the input came fromAPS5_PAD_INPUT_FILE(the libScePad command file), so the window was not focused. The trace buffer dropped events at its default size in every capture (dropped: yes); the frame times come from the presents.Summary lines (main, then this change):
{"commit":"3fcb5e4d7649","dirty":null,"env":{"APS5_DRAW_PIPELINE":"1","APS5_MIMALLOC":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINED_DRAWS":"256","APS5_PIPELINE_DISPATCH":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"120:10"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 9800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31908,"title_id":"PPSA35943","title":"Graveyard Keeper 2","content_version":"01.001.000","resolution":"1920x1080","window":{"spec":"seconds 120:10","first_present":2344,"ms":10047.517},"presents":148,"frame_ms":{"p50":67.270,"p90":72.242,"p99":77.354,"max":78.058},"gpu_busy_ms_per_present":8.861,"worker_busy_pct":{"queue worker 0x0":96.438},"top_phases_ms_per_frame":{"Driver.Packet":65.406,"Graphics.Draw":57.908,"Graphics.Draw.draw_and_resource_release":49.344,"Driver.Packet.draw":42.270,"Driver.Draw":41.904,"Driver.DrawFinish":35.172,"Driver.Draw.prepare":32.781,"Driver.DrawFinish.shader_compile_and_link":31.427},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":6.419,"events_lost":154720,"totals_full":false}},"trace":"aps5-trace-3fcb5e4d7649-48276.json.gz"} {"commit":"0c11c4bf8236","dirty":null,"env":{"APS5_DRAW_PIPELINE":"1","APS5_MIMALLOC":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINED_DRAWS":"256","APS5_PIPELINE_DISPATCH":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"120:10"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 9800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31908,"title_id":"PPSA35943","title":"Graveyard Keeper 2","content_version":"01.001.000","resolution":"1920x1080","window":{"spec":"seconds 120:10","first_present":2450,"ms":10025.271},"presents":156,"frame_ms":{"p50":63.792,"p90":69.182,"p99":74.001,"max":77.066},"gpu_busy_ms_per_present":8.979,"worker_busy_pct":{"queue worker 0x0":96.197},"top_phases_ms_per_frame":{"Driver.Packet":61.761,"Graphics.Draw":54.890,"Graphics.Draw.draw_and_resource_release":46.766,"Driver.Packet.draw":38.936,"Driver.Draw":38.569,"Driver.DrawFinish":34.866,"Driver.Draw.prepare":32.605,"Driver.DrawFinish.shader_compile_and_link":31.115},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":8.730,"events_lost":40701,"totals_full":false}},"trace":"aps5-trace-0c11c4bf8236-23400.json.gz"}Traces and the other summary lines: https://github.com/WallisSkill/AnyPS5/releases/tag/gk2-perf-traces-2026-10-24 (files
gk2-base-*andgk2-pr2-*).Tests: 38 AGC driver tests (draw pipeline, finisher, label order, vertex fetch, vertex table read order, draw cache, depth and DCC cases) in plain and
APS5_DRAW_PIPELINE=1 APS5_PIPELINED_DRAWS=32 APS5_PIPELINE_DISPATCH=1modes withANYPS5_NO_SHADER_CACHE=1: 76 of 76 pass. The branch is rebased ontomain137a8fa without conflicts; the captures above are from its first head 0c11c4bf8.Validation: the same 38 AGC driver tests ran on
main137a8fa and on this head (rebased onto it) withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation,ANYPS5_NO_SHADER_CACHE=1,ANYPS5_REQUIRE_VULKAN=1,APS5_DRAW_PIPELINE=1 APS5_PIPELINED_DRAWS=32 APS5_PIPELINE_DISPATCH=1andVK_LOADER_DEBUG=layer. The layer is theVkLayer_khronos_validation.dllthat Steam ships inbin/cef/cef.win64, registered throughVK_ADD_LAYER_PATHwith its own manifest; every test that creates a device logsInserted device layer "VK_LAYER_KHRONOS_validation"(31 of 38; the other 7 make no Vulkan device), and no log containswas not found but was requested by env var VK_INSTANCE_LAYERS. All 38 pass on both. The only validation message on either side isVUID-VkDeviceCreateInfo-pNext-pNext, 126 times onmainand 126 times on this head, the same count in each test (this layer build predates some device-creation structures the driver chains); no VUID is new.Digests of a paired capture (the release holds every trace and summary line):
trace_digest.py of base-1
seconds 120:10 from present 2344: 148 presents, frame p50 67.3 / p90 72.2 / p99 77.4 / max 78.1 ms, GPU busy 8.9 ms per present
NVIDIA GeForce RTX 5070, NVIDIA 616.92, Windows 10.0.26200, 1920x1080, commit 3fcb5e4, dropped: yes
{"commit":"3fcb5e4d7649","dirty":null,"env":{"APS5_DRAW_PIPELINE":"1","APS5_MIMALLOC":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINED_DRAWS":"256","APS5_PIPELINE_DISPATCH":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"120:10"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 9800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31908,"title_id":"PPSA35943","title":"Graveyard Keeper 2","content_version":"01.001.000","resolution":"1920x1080","window":{"spec":"seconds 120:10","first_present":2344,"ms":10047.517},"presents":148,"frame_ms":{"p50":67.27,"p90":72.242,"p99":77.354,"max":78.058},"gpu_busy_ms_per_present":8.861,"worker_busy_pct":{"queue worker 0x0":96.438},"top_phases_ms_per_frame":{"Driver.Packet":65.406,"Graphics.Draw":57.908,"Graphics.Draw.draw_and_resource_release":49.344,"Driver.Packet.draw":42.27,"Driver.Draw":41.904,"Driver.DrawFinish":35.172,"Driver.Draw.prepare":32.781,"Driver.DrawFinish.shader_compile_and_link":31.427},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":6.419,"events_lost":154720,"totals_full":false}},"trace":"aps5-trace-3fcb5e4d7649-48276.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Graphics.Draw 39.5, Graphics.Draw.draw_and_resource_release 31.0, Driver.Packet 27.3, Driver.Packet.draw 14.8, Driver.Draw 14.4, Driver.DrawFinish 10.8, Driver.Packet.dispatch 10.1, Driver.Dispatch 10.0, batch 8.9, Driver.DrawFinish.shader_compile_and_link 7.7
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
trace_digest.py of pr2-1
seconds 120:10 from present 2450: 156 presents, frame p50 63.8 / p90 69.2 / p99 74.0 / max 77.1 ms, GPU busy 9.0 ms per present
NVIDIA GeForce RTX 5070, NVIDIA 616.92, Windows 10.0.26200, 1920x1080, commit 0c11c4bf8236, dropped: yes
{"commit":"0c11c4bf8236","dirty":null,"env":{"APS5_DRAW_PIPELINE":"1","APS5_MIMALLOC":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_PIPELINED_DRAWS":"256","APS5_PIPELINE_DISPATCH":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"120:10"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","cpu":"AMD Ryzen 7 9800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31908,"title_id":"PPSA35943","title":"Graveyard Keeper 2","content_version":"01.001.000","resolution":"1920x1080","window":{"spec":"seconds 120:10","first_present":2450,"ms":10025.271},"presents":156,"frame_ms":{"p50":63.792,"p90":69.182,"p99":74.001,"max":77.066},"gpu_busy_ms_per_present":8.979,"worker_busy_pct":{"queue worker 0x0":96.197},"top_phases_ms_per_frame":{"Driver.Packet":61.761,"Graphics.Draw":54.89,"Graphics.Draw.draw_and_resource_release":46.766,"Driver.Packet.draw":38.936,"Driver.Draw":38.569,"Driver.DrawFinish":34.866,"Driver.Draw.prepare":32.605,"Driver.DrawFinish.shader_compile_and_link":31.115},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":8.73,"events_lost":40701,"totals_full":false}},"trace":"aps5-trace-0c11c4bf8236-23400.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Graphics.Draw 35.8, Driver.Packet 30.1, Graphics.Draw.draw_and_resource_release 28.4, Driver.Packet.dispatch 13.5, Driver.Dispatch 13.5, Driver.Packet.draw 13.4, Driver.Draw 13.0, Driver.DrawFinish 10.9, batch 9.0, Driver.Draw.prepare 8.5
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
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)git merge-treeof the two heads reports no conflict)🤖 Generated with Claude Code