Repository navigation
perf(shader): reuse an evicted memo result's bindings for the next miss - #9049
EdouardSence wants to merge 4 commits into
Conversation
A result memo miss now takes the least recently used entry of its set before materializing when the memo holds the only reference to it, and Populate copy-assigns the new bindings into that result's binding elements, so their vectors keep their storage instead of being freed and allocated again on every miss. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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.
|
|
Checked the files the overlap comment could not compare, against the diffs of those 25 pull requests. No other pull request changes the result memo ( Four touch the line this PR changes in
Each is a one-line textual conflict. Whichever lands second keeps both changes; the parameters are independent. No Tested and written by Claude (AI assistant) on a contributor's machine. |
|
Windows run of 7fcf715 merged onto Built |
|
@EdouardSence a Linux/NVIDIA data point on Stray (PPSA02100). It was measured on my stack, not on plain Setup
FPS (mean of four runs)
Tests and output
Your -8 % came from AI-assisted: yes (text with Claude Code; the numbers are from my machine). |
… binding's storage Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@teepean thanks for the Stray runs and the reading. Your guess about the move-assigned vectors was right, so I pushed edc9564: Sky Garden A/B, ASTRO BOT PPSA21567 01.019.000, Ryzen 7 7800X3D, RX 9070 XT (RADV Mesa 26.2.4), CachyOS. Headless gamescope 3840x2160, no input,
On this machine and scene, the previous head was not slower with jemalloc (its round 1 is the noisy one). The new head is about 8 % faster than
Tested and written by Claude (AI assistant) on a contributor's machine. |
|
Windows run of edc9564, the update since my earlier run, merged onto Built |
|
@EdouardSence thanks, here is edc9564 on our stack. Short version: the loss is gone; it is level with our base now. Setup
FPS (8 rounds, order alternating)
The PR side won 4 of 8 rounds. Run-to-run spread here is 0.3-0.5 FPS, so this is level within noise. Filling the reused binding's storage removed what cost us before. Checks
AI-assisted: yes (text with Claude Code; the numbers are from my machine). |
|
@teepean thanks for the 8 rounds and the verify run (0 wrong memo hits, 0 differing stage results). Level on your committer-bound stack and faster on the worker-bound Sky Garden here is what I hoped the change would do. About the exit at 184 s: none of my Sky Garden runs on either head exited early (8 runs today across the two heads, 180 s each). If it happens again, a core dump or the last Written by Claude (AI assistant) on a contributor's machine. |
|
Independent profiling lead from Outer Wilds (PPSA08102): the queue worker spends substantial CPU time allocating/destructing materialized shader results, so this PR targets a path worth comparing in that title. Profile packet, perf report and terminal review. This is not a speedup measurement or validation of #9049: it is a clean older The 60-second all-thread CPU profile has 6,333 samples and zero lost. The queue worker accounts for 22.82% of process CPU samples; within that thread, inclusive stacks include I've queued independent focused checks of current main |
|
@EdouardSence I ran this on another title with the same pattern: Sonic Origins Plus (PPSA05324), Sonic 2 page of the main menu. On Windows 11 Pro 10.0.26300, NVIDIA GeForce RTX 2080 Ti, driver 617.14, i9-9900K. I rebased your two commits onto Settings:
#10423 changes |
|
@EdouardSence #8564 landed and now conflicts with this PR in A heads-up for after that: #10423 (meh7an) also changes |
…result-bindings # Conflicts: # core/shader/recompiler/Recompiler.cpp
|
@oneandonlydean thanks, merged
@meh7an thanks for the Sonic Origins Plus run, a second title and vendor with the same direction. On the order with #10423: #9049 is mergeable again now, so if it lands first I'm glad to take your offer to rebase there. If #10423 is ready first, I'll rebase this one onto it instead; either way works for me. Written by Claude (AI assistant) on a contributor's machine. |
…ted memo result's bindings for the next miss): the next run is cold anyway after the d5f5cd8 merge changed the shader cache version
…result-bindings # Conflicts: # core/shader/recompiler/Optimization/src/DescriptorBindingBuilder.cpp
|
Merged current
I have not rerun the Sky Garden A/B on this merge; the conflict only changes where the AI-assisted: yes |
|
@EdouardSence the overlap check pairs this PR with #10423 (masked result memo) and #10757 (recycling evicted results), which change Proposal: this PR goes first. Once it merges, I rebase #10423 and #10757 onto it and give them one reuse path: an evicted result goes to #10757's pool, and your miss path takes its bindings from a pooled result instead of evicting the victim early. That keeps your reuse working with both tables. #10423 and #10757 then declare |
|
@meh7an That order works for me: this PR first, then #10423 and #10757 rebased on it with one reuse path, where an evicted result goes to #10757's pool and the miss path takes its bindings from a pooled result. The branch still merges cleanly into current AI-assisted: yes |
|
@EdouardSence This conflicts with main in a way we can't resolve for you. Main's 54d7c78 ("perf(shader): reuse a memoized result by patching its copied words") rewrote the memo lookup and gave |
|
@oneandonlydean I merged
The cost left around eviction is the patched copy and the evicted result's free, which #10757 addresses. @meh7an the order question is moot then; nothing from this PR needs to carry into #10757. AI-assisted: yes |
What
In ASTRO BOT's Sky Garden, the queue worker thread is at ~99 % and bounds the frame. The
Recompile(request, capture)result memo hits only ~6.5 % of the time there: about 35k misses per second, nearly every one evicting an entry. Every miss builds a newRecompileResult. Itsbindingsare copied from the binding plan with all of eachDescriptorBinding's vectors, and the evicted result is destroyed with all of its vectors on the same thread. In aneu-stacksample of the worker (120 samples), ~18 % of samples were in~RecompileResultundermaterializePreparedMemoized, and malloc/free showed up underMaterializeandPopulatetoo. WithLD_PRELOADof jemalloc the same build ran ~8 % faster, so allocation is a real share of the frame there.materializePreparedMemoizednow picks the set's least recently used entry before materializing. When the memo holds the only reference to it (use_count() == 1under the source mutex, which every other lookup also takes), the entry is taken out of the set and itsbindingsvector is passed tomaterializeResult.DescriptorBindingBuilder::Populatecopy-assigns each plan binding into the allocation's existing elements, then resizes to the plan's size, instead of building a new vector. The vectors in a reused element keep their storage. An empty allocation gives the same result as before.APS5_PROFILE_DRAW's[recompile] result memoline also counts these entries.Tested
-DBUILD_TESTING=ON: ctest 1786/1792 withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ANYPS5_NO_SHADER_CACHE=1 ANYPS5_REQUIRE_VULKAN=1on the RX 9070 XT. The 6 failures (agc_driver_guest_memory_lifetime,agc_driver_image_array_window_past_layer_limit,agc_driver_tessellation(segfault),net_dontwait_blocking_send,_sendto,_sendmsg) fail the same way onmain2abfb1b in this environment.ctest -Vand the same variables, onmainand on this branch:agc_shader_disk_cache,agc_shader_memory,agc_driver_graphics,agc_driver_testsand theagc_driver_draw*tests (17 tests) pass on both. Neither log has aVUID-line orwas not found but was requested by env var VK_INSTANCE_LAYERS.verifyBindingPlanSelection(agc_shader_disk_cache) now also populates the selected plan into an allocation that still holds the full plan's four bindings, and checks that the result equals a fresh populate.-lvl underwater_aerial_garden), headless gamescope 3840×2160, no input,APS5_TRACE_SECONDS=90:50,APS5_NO_SNAPSHOT_CHECK=1. Both builds also ran withAPS5_HOST_IMPORT_MIB=11815, becausemainwithout fix(agc): keep at most 4 GiB of the host heap from the host import budget #7844 thrashes textures in this level on a 16 GB card. Each build had a warm-up run, then the runs were interleaved.Driver.Draw.shader_compile_and_linkms/framemain2abfb1b, warm-upmain, run 1main, run 2This PR is about 8 % faster at p50 here, and p99 drops from 380-402 ms to ~261 ms. GPU time is unchanged. In the traces,
Shader.DrawStagegoes from 123.4 to 96.8 ms per frame. The worker stays at ~99 %, so the draw cost still bounds the frame.Summary lines (run 1 of each). Traces of both run 1s: main, this PR. Both traces dropped events after a few seconds on the worker (
trace_dropped); the frame times come from the presents.{"commit":"2abfb1beb4dd","dirty":false,"env":{"APS5_HOST_IMPORT_MIB":"11815","APS5_NO_SNAPSHOT_CHECK":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"90:50"},"gpu":"AMD Radeon RX 9070 XT (RADV GFX1201)","driver":"radv Mesa 26.2.4-arch3.1","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31228,"title_id":"PPSA21567","title":"ASTRO BOT","content_version":"01.019.000","resolution":"3840x2160","window":{"spec":"seconds 90:50","first_present":1601,"ms":50091.668},"presents":192,"frame_ms":{"p50":255.485,"p90":273.049,"p99":379.795,"max":562.272},"gpu_busy_ms_per_present":130.340,"worker_busy_pct":{"queue worker 0x0":99.321,"queue worker 0x40":99.504,"queue worker 0x48":99.349,"queue worker 0x46":0.840,"queue worker 0x3a":0.078},"top_phases_ms_per_frame":{"Driver.Packet":779.517,"Driver.Packet.memory_wait":492.248,"Driver.Packet.draw":198.854,"Driver.Draw":197.899,"Graphics.Draw":154.105,"Driver.Draw.shader_compile_and_link":137.224,"Graphics.Draw.draw_and_resource_release":133.193,"Shader.DrawStage":123.446},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.259,"events_lost":3623330,"totals_full":false},"draw committer":{"events_full_after_s":9.812,"events_lost":1080694,"totals_full":false}},"trace":"aps5-trace-2abfb1beb4dd-758987.json.gz"} {"commit":"7fcf715559eb","dirty":false,"env":{"APS5_HOST_IMPORT_MIB":"11815","APS5_NO_SNAPSHOT_CHECK":"1","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"90:50"},"gpu":"AMD Radeon RX 9070 XT (RADV GFX1201)","driver":"radv Mesa 26.2.4-arch3.1","os":"Linux 7.2.10-1-cachyos","cpu":"AMD Ryzen 7 7800X3D 8-Core Processor","cpu_threads":16,"memory_mib":31228,"title_id":"PPSA21567","title":"ASTRO BOT","content_version":"01.019.000","resolution":"3840x2160","window":{"spec":"seconds 90:50","first_present":898,"ms":50224.935},"presents":209,"frame_ms":{"p50":237.486,"p90":254.840,"p99":260.847,"max":262.016},"gpu_busy_ms_per_present":131.252,"worker_busy_pct":{"queue worker 0x0":99.238,"queue worker 0x40":99.617,"queue worker 0x48":99.519,"queue worker 0x46":0.908,"queue worker 0x3a":0.140},"top_phases_ms_per_frame":{"Driver.Packet":718.817,"Driver.Packet.memory_wait":448.867,"Driver.Packet.draw":179.435,"Driver.Draw":178.494,"Graphics.Draw":145.812,"Graphics.Draw.draw_and_resource_release":125.790,"Driver.Draw.shader_compile_and_link":110.315,"Shader.DrawStage":96.817},"trace_dropped":true,"trace_dropped_threads":{"queue worker 0x0":{"events_full_after_s":3.773,"events_lost":3198830,"totals_full":false},"draw committer":{"events_full_after_s":9.461,"events_lost":1128611,"totals_full":false}},"trace":"aps5-trace-7fcf715559eb-760875.json.gz"}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)Tested and written by Claude (AI assistant) on a contributor's machine.
AI-assisted: yes
🤖 Generated with Claude Code