Repository navigation
perf(shader): reuse a memoized result by patching its copied words - #10423
Merged
adriadam10 merged 1 commit intoOct 25, 2026
Merged
Conversation
12 tasks done
|
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.
|
imellag
approved these changes
Oct 24, 2026
imellag
left a comment
imellag
left a comment
Contributor
There was a problem hiding this comment.
I read the change against the code it relies on and found no case where a patched result can differ from a full one:
- The masked key keeps the same buffer words as
preparedBindingPlan's key (dwordCount,dwords[1] & 0xffff0000,dwords[3], present plus alignment), andGuestBuffersDescriptorcopies words 0 to 3 as they are, so the plan, the module and the binding shape cannot change between donor and patched result. ShaderDataDwordsForputs the user words at 0 to n-1, and the dispatch limit, the image metadata and the runtime state all start atDispatchThreadLimitDword()or later, souserDataRegisters.size() <= DispatchThreadLimitDword()covers the three overlaps the description lists. The memory offset words before it are not written byPopulate.- Push data and a
ShaderDatabinding never coexist (Populatefails on that), so patching only one of them is enough. partialThreads(request)is in both hashes,mergeTargetImageonly appends the target image, and the SPIR-V is aSharedSpirv, so copying the donor does not copy the module.
macOS data, Apple M4 Pro, macOS 27.0.1, MoltenVK 1.4.2, x86-64 Release under Rosetta 2, on a local stack based on main 8a85a5c848 with #8658's macOS port, with this PR's commit on top:
- The 47 tests run by
agc_shader_memory_testsandagc_driver_graphics_tests, withAPS5_VERIFY_MASKED_RESULT_MEMO=1: nomasked result memo mismatchline.agc_shader_table_snapshot_reuseandagc_shader_table_snapshot_reuse_unmaskedpass with the masked memo on and off. Two other tests fail on my stack, identically withAPS5_NO_MASKED_RESULT_MEMO=1, because of local patches unrelated to this PR. - Stray (PPSA02100) in gameplay,
APS5_PROFILE_DRAW=1, 10 s windows, the same build with and withoutAPS5_NO_MASKED_RESULT_MEMO=1: 72,000 to 89,000 misses become 24,000 to 27,000 misses plus 76,000 to 85,000 patched hits (61 % of lookups; exact hits stay at about 17.5 %). The materialization time falls from 364 to 426 ms to 125 to 184 ms, and the patching takes 228 to 276 ms (about 3 µs per patched hit, against about 5 µs per miss there), so on this scene the queue worker saves little, and the frame is bound by the GPU, so frame rate did not change measurably. I did not run the verifier in the game.
On the two open PRs that change the same memo, both of which conflict with this one in Recompiler.cpp, there are two interactions beyond the rebase:
- #9049 recycles the least recently used exact entry only when the memo holds its only reference (
victim->result.use_count() == 1). Here a full miss inserts the same result intomemoand, when its patch is valid, intomaskedMemo, so that victim usually still has a reference from the masked table and is not recycled. The two may add up less than expected unless the recycling also looks at the masked table. - #9989 bounds the bytes the result memos hold and trims their entries. A result that only
maskedMemostill references would stay alive outside that budget unless the masked table is counted and trimmed too.
AI-assisted: yes
This was referenced Oct 24, 2026
Collaborator
|
Re-checked against current main: the merge is clean and main's changes in the PR's files (plan loading in |
This was referenced Oct 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When the
Recompile(request, capture)result memo misses, the draw's snapshot often differs from a cached one only in words thatmaterializeResultcopies into the result verbatim:Populatewrites intoGuestBuffersbindings;ShaderDatabinding;FlattenedSrtbinding.On Sonic Origins Plus's Sonic 2 menu page, the memo hits about 26 % of lookups on
main, and every miss builds a whole result again. On that page the queue worker bounds the frame, andmaterializeDrawStageplus the release of the results it built take about half of its samples. With this PR, 72 % of the page's lookups become patched hits and only 1.5 % still miss.This PR adds a second, masked table per source:
snapshotHashwith each buffer's words 0-2 reduced to the class the binding plan already keys on (dwords[1] & 0xffff0000,dwords[3], and present plus alignment), and with the user data and the flattened SRT counted by their size only. Everything else in the key is unchanged.MemoPatch).poisonedSrtReadsis recomputed from the snapshot.Why the masked words are safe to patch:
preparedBindingPlankeys only on the buffer classes above,ResolveImageTablesSharedreadssnapshot.tables, andPopulatecopies the masked words verbatim (GuestBuffersDescriptor,ShaderDataDwordsFor, theFlattenedSrtbinding). Throws that depend on these words depend only on their sizes and classes, which stay in the key.APS5_NO_MASKED_RESULT_MEMO=1turns the masked table off.APS5_VERIFY_MASKED_RESULT_MEMO=1also materializes every patched hit in full and compares the bindings, push constants, specialization and module, printing the first twenty differences. Its count is in the[recompile] result memoline ofAPS5_PROFILE_DRAW, which now also shows the patched hits and the time per patch.agc_shader_table_snapshot_reuseexpects no table resolution for a patched result and checks that the result holds the moved buffer's descriptor.agc_shader_table_snapshot_reuse_unmaskedruns the same test with the masked table off, so the shared-resolution path stays covered.Related open PRs:
materializePreparedMemoizedtoo. Proposed order: perf(shader): reuse an evicted memo result's bindings for the next miss #9049 first (comment), then this PR rebases onto it.Tested
Windows 11 Pro 10.0.26300, NVIDIA GeForce RTX 2080 Ti, driver 617.14, i9-9900K, on
maine7f46f5.Sonic Origins Plus (PPSA05324), Sonic 2 page of the main menu,
main's defaults (noAPS5_*opt-ins).APS5_TRACE_SECONDS=200:15, after one warm-up boot each:Driver.Packet(ms/frame)Driver.Draw.shader_compile_and_link(ms/frame)mainmainround. The queue worker is about 97 % busy in both, so the page stays bound by it. The title's own FPS reading stays within 12.8-13.3 for both.APS5_VERIFY_MASKED_RESULT_MEMO=1withAPS5_PROFILE_DRAW=1on the same page: 0 mismatches. Per 10 s: 112,397 exact hits (26.2 %), 310,079 patched (72.3 %), 6,411 misses (1.5 %).ctest(2063 tests) onmainplus this PR: everything passes exceptagc_driver_sample_order,agc_driver_sample_order_grid_referenceandagc_driver_sample_order_recipe_depth_locations, which fail the same way on plainmainon this machine. The same suite underAPS5_VERIFY_MASKED_RESULT_MEMO=1gives the same result with 0 mismatches.ctest --test-dir build -R "^agc_shader_|^agc_driver_draw_cache" -VwithANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation: 48/48 onmain, 49/49 on this PR (the extra one isagc_shader_table_snapshot_reuse_unmasked). Neither log haswas not found but was requested by env var VK_INSTANCE_LAYERS, and neither has aVUID-line.The two summaries and their digests (the median round of each build; all six traces are attached):
main
seconds 200:15 from present 3460: 172 presents, frame p50 82.4 / p90 107.9 / p99 125.1 / max 127.7 ms, GPU busy 68.1 ms per present
NVIDIA GeForce RTX 2080 Ti, NVIDIA 617.14, Windows 10.0.26300, 3840x2160, commit e7f46f5, dropped: yes
{"commit":"e7f46f5c8937","dirty":false,"env":{"APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"200:15"},"gpu":"NVIDIA GeForce RTX 2080 Ti","driver":"NVIDIA 617.14","os":"Windows 10.0.26300","cpu":"Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz","cpu_threads":16,"memory_mib":32684,"title_id":"PPSA05324","title":"Sonic Origins","content_version":"02.002.000","resolution":"3840x2160","window":{"spec":"seconds 200:15","first_present":3460,"ms":15052.949},"presents":172,"frame_ms":{"p50":82.435,"p90":107.941,"p99":125.124,"max":127.668},"gpu_busy_ms_per_present":68.113,"worker_busy_pct":{"queue worker 0x0":97.328},"top_phases_ms_per_frame":{"Driver.Packet":85.107,"Driver.Packet.draw":66.837,"Driver.Draw":65.806,"Graphics.Draw":65.247,"Graphics.Draw.draw_and_resource_release":54.844,"Driver.Draw.shader_compile_and_link":41.123,"Shader.DrawStage":30.566,"Driver.Present":27.467},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":6.007,"events_lost":392686,"totals_full":false},"draw committer":{"events_full_after_s":13.602,"events_lost":28410,"totals_full":false}},"trace":"aps5-trace-e7f46f5c8937-6396.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): batch 68.0, Graphics.Draw 35.5, Graphics.Draw.draw_and_resource_release 29.8, Driver.Present 27.5, VideoOut.Flip 27.4, VideoOut.Flip.present 27.3, Driver.Present.render_fence_wait 26.9, Vulkan.Retire 26.9, Vulkan.Retire.wait 26.9, Driver.Packet 18.8
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
this PR
seconds 200:15 from present 3481: 185 presents, frame p50 77.3 / p90 95.0 / p99 112.3 / max 120.0 ms, GPU busy 67.5 ms per present
NVIDIA GeForce RTX 2080 Ti, NVIDIA 617.14, Windows 10.0.26300, 3840x2160, commit 54d7c78, dropped: yes
{"commit":"54d7c780a583","dirty":false,"env":{"APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"200:15"},"gpu":"NVIDIA GeForce RTX 2080 Ti","driver":"NVIDIA 617.14","os":"Windows 10.0.26300","cpu":"Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz","cpu_threads":16,"memory_mib":32684,"title_id":"PPSA05324","title":"Sonic Origins","content_version":"02.002.000","resolution":"3840x2160","window":{"spec":"seconds 200:15","first_present":3481,"ms":15037.835},"presents":185,"frame_ms":{"p50":77.32,"p90":94.965,"p99":112.311,"max":119.978},"gpu_busy_ms_per_present":67.469,"worker_busy_pct":{"queue worker 0x0":97.129},"top_phases_ms_per_frame":{"Driver.Packet":78.867,"Graphics.Draw":62.354,"Driver.Packet.draw":60.497,"Driver.Draw":59.489,"Graphics.Draw.draw_and_resource_release":52.254,"Driver.Draw.shader_compile_and_link":36.326,"Driver.Present":27.46,"VideoOut.Flip":27.373},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":6.274,"events_lost":374602,"totals_full":false},"draw committer":{"events_full_after_s":12.901,"events_lost":43909,"totals_full":false}},"trace":"aps5-trace-54d7c780a583-32184.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): batch 67.8, Graphics.Draw 31.6, Driver.Present 27.5, VideoOut.Flip 27.4, VideoOut.Flip.present 27.3, Driver.Present.render_fence_wait 26.9, Vulkan.Retire 26.9, Vulkan.Retire.wait 26.9, Graphics.Draw.draw_and_resource_release 26.7, Driver.Packet 17.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)Traces, one zip per round, each holding the
mainand the PR capture:masked-memo-traces-round1.zip
masked-memo-traces-round2.zip
masked-memo-traces-round3.zip