Repository navigation
perf(agc): find cached draws across moved user-data pointers - #4943
Conversation
|
Windows test (MinGW GCC 15.2, Release, Windows 10, RTX 4080 SUPER). Built on main
Not tested: no game run. The batch was built once, not per PR; nothing failed, so no bisect was needed. AI-assisted (Claude Code). |
|
Tested on Minecraft Dungeons II (PPSA16064) gameplay: Windows 11, RTX 5070, cherry-picked onto our MCD2 build (
So the data behind the pointers likely differs per draw (per-draw constants), which this first step does not cover. "Reusing a stage when only buffer base addresses moved" sounds like what this title needs; happy to test it. AI-assisted (Claude Code). |
|
No measurable change on AMD in two light 2D catalog titles, the same result as @TideOPL's Minecraft Dungeons II run. Setup: Windows 11 (26300), Radeon 860M (RDNA 3.5, AMD proprietary driver 24.30.58.08), WinLibs GCC 15.2.0, Release, 1920x1080. Base is
In every build the queue worker is busy only 35-37 % of the window, and |
|
Besiege (PPSA20313), this branch (
The relocation does not take effect on this title. Over 10 s the draw cache sees about 75,000 lookups with 74,000 "no entry" and 927 hits, and your counters read |
|
Data for why the relocation does not fire on Besiege (PPSA20313, In steady state 70% of the misses are exactly that case, and in the first 40 sampled draws a single word changes in 39 of them (the 40th printed no changed word): the second stage's user word 8 (register 0x94, index 8 of the 0x8c group). It goes up by exactly +176 each time ( |
df467c2 to
67a3fdb
Compare
|
Thanks for the three data points. I've rebased the branch onto current @JAB-dev that 3-5% on Besiege was the alias lookup running for every draw with nothing to find (your counters show 2 donors in 123,013 lookups). Two changes for that case:
I don't have Besiege, so I can't confirm it closes the gap there. If you can run the new head against the same base, the Your second comment explains why nothing matches: one user word walking a ring of 176-byte per-draw constant slots, with everything else equal. Whether that can be reused depends on how the shader reaches the slot. If it binds the slot as a buffer, #5609 covers it: only the base address moves and the contents stay out of the match. If it reads the constants through the pointer during the capture, the contents differ every draw and neither PR can match; that case needs the constants refreshed in a cached stage rather than compared, which is closer to what #494 does for user data, and I haven't done it here. A run of #5609's head on Besiege would tell which it is. The rebase also had to deal with #5216 (a draw is cached the second time its key is seen). With moving pointers the exact key never repeats, so nothing was cached and the relocation had nothing to reuse. A repeat of the alias key now counts as a second sighting too. On the Flycast workload, with the follow-up applied: 59% of real time without the admission, 51% with #5216 as merged, 59% with this change. @TideOPL your shape analysis is the case the next step is for. A pointer pair that moves through a per-frame ring with different data behind it each draw can't hit here, since this PR only matches when the contents behind the pointers are equal. The follow-up keeps a cached stage when the only words that differ are buffer base addresses the shader binds as buffers, so the per-draw constants stay out of the match. It is up as #5609, stacked on this one. @LeandroLP23 thanks for the AMD numbers; no change is what I'd expect on titles that don't put pointers in the user data. AI-assisted (Claude Code). |
|
I ran this with #5609 on top on my machine (Linux, CachyOS, GCC, RTX 4090, NVIDIA 615.78.08), merged with The Linux build fails.
static_assert(sizeof(RecompileResult) == 368, ...);
static_assert(sizeof(BindingAllocationResult) == 200, ...);
// EncodeResult, after workgroupMemoryDwords
writer.Values(std::span<const std::uint32_t>(result.shaderDataUserWords));
writer.List(result.bufferBaseSlots, [](Writer& out, const std::array<std::uint32_t, 3>& slot) { out.Values(std::span<const std::uint32_t>(slot)); });
// DecodeResult, after workgroupMemoryDwords
reader.Values(result.shaderDataUserWords);
reader.List(result.bufferBaseSlots, 12, [](Reader& in, std::array<std::uint32_t, 3>& slot) {
std::vector<std::uint32_t> words;
in.Values(words);
if (words.size() != slot.size()) in.Fail();
else std::copy(words.begin(), words.end(), slot.begin());
});With that, the full ctest passes except The Hades II, hub, standing still (
LLM assistance used. |
A pixel shader loads its colour through a user-data pointer. Drawn from a third address with the same contents it hits the cached draw; with other contents it does not; the addresses seen before still hit exactly. AI-assisted (Claude Code). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… user words Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Unity (Outer Wilds) changes per-draw constant-buffer pointers in its user-data SGPRs: 57975 of 57981 entry-less draws matched an earlier draw's key but for those words, so the cache hit ~8% and every draw paid capture + decode + recompile. Port of boykopovar#4943 onto this fork's draw cache: - the SRT evaluator records which user words are only a raw read's base (pointers it dereferences) and which it evaluates as values, and every read with the word it went through; results name the user word behind each forwarded shader-data dword - per shader set (program addresses + resources) the driver learns its pointer words and, beyond boykopovar#4943, its free words (never read by the walk, at most forwarded into shader data); an exact-key miss looks up an alias key over the contents behind the pointers instead of their values - on an alias hit the cached stages are copied with their runs shifted by the pointer delta (validated by value as any hit) and the forwarded shader-data words patched; anything else falls back to the miss path - the base word of a dereference still counts as read for the capture memo; stages with vertex fetch decode reads or poisoned SRT reads stay out; a shader set hitting < 1/16 over 1024 alias lookups idles 16384 - [draw-relocate] line under APS5_PROFILE_DRAW; APS5_NO_DRAW_RELOCATION=1 turns it off Test agc_driver_draw_cache_relocation(_off): moved pointers to equal contents hit from the second draw and draw the right colour, other contents miss; a run-time user word that changes per draw hits and draws the patched value. Both fail with APS5_NO_DRAW_RELOCATION=1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
895d654 to
2ea25c5
Compare
|
@adriadam10 thanks for the review. Both branches are rebased onto Performance of the stack against current
Audio produced as a share of real time, unprofiled, about 9 points higher with the stack in every alternating pair. Draw-cache hits on D2R. You were right about the admission. An exact key was admitted at its first sight whenever its alias had been seen, whether or not that shader set's relocations ever hit. That admission exists so a shader set can get a first entry to relocate from: after an alias-admitted insert, the next draw with the same alias hits through relocation and is never inserted. So the cost falls on aliases that come up exactly twice. Each such pair inserts an entry that becomes a donor nobody uses. The rule now:
The new case in Disk-cache round trip. AI-assisted (Claude Code) |
…d again With APS5_VERIFY_DRAW_ENTRIES=1 every hit is captured again, so drawHit is false and no relocated hit counted for its shader set's trial, which judged every set unproductive and turned the relocation off under the very mode meant to check it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Linux results, since the description has no Linux build. Ubuntu 24.04, GCC 13.3, Release, RTX 4070 Ti, NVIDIA 595.99,
AI-assisted: yes |
|
Field data from PPSA08587 (Dragon Ball Xenoverse 2), in case it helps the relocation path: in its heavy gameplay scenes, this PR finds no donors, and the scene stays at 6-12 fps. Setup: Windows 11, i5-11400, RTX 3070 Ti (NVIDIA 610.88). Build: The miss is in user dataEach frame has ~1,300 draws at ~65 µs each on the queue worker; the worker is ~90% busy and the GPU ~35%. 10 s window:
Where the relocation path stopsTo find out, I added counters at each exit of the relocation path. They are local only and don't change behaviour. One line per 10 s in the heavy scene, with this PR as is:
Same scene, with probation and idle bypassedI then bypassed both, locally: a key whose alias The probation and idle gates were not the root cause:
So for this title, the gain would come from making stages with vertex fetch relocatable, when their fetch descriptors are reached through the moved pointer, and from looking into why so many aliases are unreadable. I can run a build from a branch or share the counter patch if that helps; the full logs are available too. The analysis was done with AI assistance (Claude Code). |
adriadam10
left a comment
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the update. Checked against our last review:
- Stack numbers on
main940eba9a2(audio 34.5-35.7% → 42.8-45.7%) answer the performance point; the gain needs #5609 on top, so the two should land back to back. - Alias admission now stops after two probation admissions unless relocated hits validate.
UnrelocatedAliasesStayOutfails when the probation condition is forced to always admit, and passes with the PR as is. agc_shader_disk_cachenow round-tripsshaderDataUserWords.
On this head merged onto main, the Agc/AgcDriver scope builds and agc_shader_disk_cache and agc_driver_draw_cache_relocation pass here on Linux; thanks @milescoviello for the independent Linux run. Not verified by us: the D2R hit counts on this head (@mrfelek's last run was on the earlier one), and Windows runtime behaviour beyond CI. Merging once the full Build is green.
relocateDrawEntry copied a cached variant field by field and left out meshIndexDescriptor, which the variant gained in #5791 before #4943 merged. lookupDraw compares a mesh stage's descriptor with the draw's index buffer words before validating anything else, and no draw has an all-zero one, so every relocated mesh-path stage was refused as a differing front stage. With no hits the shader set's relocation trial stopped admitting new aliases too. The copy now takes the donor's descriptor, so a relocated mesh stage matches the same index buffer the donor was built for and misses on any other. agc_driver_mesh_draw_relocation draws a mesh-path triangle whose vertex buffer descriptor is loaded through a user-data pointer, at eight addresses with the same contents: the last four now hit the cached draw (before: four differing front stages, no hit). AI-assisted: yes
A draw that misses on its own register key and is served by a relocated draw entry (relocateDrawEntry, boykopovar#4943) never got a draw recipe: - relocateDrawEntry builds a fresh DrawEntry with per-draw copies of the donor's variants. That entry is not stored in drawCache under the draw's key. - Driver::draw still passes the draw's own key to findDrawRecipe and, after recording, to attachDrawRecipe. Both look the key up in drawCache, find nothing and return, so a recipe built while recording such a draw is dropped and the next relocated draw of the same shape records in full again ("attaches 0" with APS5_TRACE_DRAW_CACHE). - The recipe records also compare stage identity against the per-draw copies, which are new objects on every draw, so even a stored record could never match. Changes: - DrawEntry and DispatchVariant gain a donor pointer. relocateDrawEntry sets it on the relocated entry and on every copied variant, always to the original cached entry/variant (a copy of a copy points at the root), so the chain never grows. - Driver::draw keys the recipe stages on the donor variant when a matched variant is a relocation copy, and hands the donor entry to findDrawRecipe/attachDrawRecipe (also through commitDraw on the pipelined path). With a donor, both read and update the donor entry's recipe list directly instead of looking the key up; without one they behave as before. - DrawWithRecipe rebuilds the push constant block from the current shaders and patches it through the recorded resources when the recipe has push stages, instead of replaying the bytes captured with the recipe. A relocated draw moves user words that end up in push constants, so the captured block would carry the first draw's values. Draws that are not relocated take the same path as before: their matched variants have no donor and recipe lookup still goes through drawCache. Measured on Hades (PPSA03355, discussion boykopovar#3234), where nearly every sprite draw is served through a relocated entry: lookups and attaches now succeed, but Graphics::Draw builds a recipe for only ~15 of ~100,000 draws per 10 s because the draws' ShaderResources are not Reusable(), so the frame rate does not change yet. The fix is what lets recipes apply to relocated draws once that is addressed.
Resolves #4943's draw relocation against this branch: tagVariantRuns, which now builds a draw variant's runs and words, also records the captured words it shares with a table's supplied blocks, per region as before. A variant whose runs relocation splits keeps sharing for the draw cache's word compare, and skips the vouched validation, which already requires one run per region.
A draw that misses on its own register key and is served by a relocated draw entry (relocateDrawEntry, boykopovar#4943) never got a draw recipe: - relocateDrawEntry builds a fresh DrawEntry with per-draw copies of the donor's variants. That entry is not stored in drawCache under the draw's key. - Driver::draw still passes the draw's own key to findDrawRecipe and, after recording, to attachDrawRecipe. Both look the key up in drawCache, find nothing and return, so a recipe built while recording such a draw is dropped and the next relocated draw of the same shape records in full again ("attaches 0" with APS5_TRACE_DRAW_CACHE). - The recipe records also compare stage identity against the per-draw copies, which are new objects on every draw, so even a stored record could never match. Changes: - DrawEntry and DispatchVariant gain a donor pointer. relocateDrawEntry sets it on the relocated entry and on every copied variant, always to the original cached entry/variant (a copy of a copy points at the root), so the chain never grows. - Driver::draw keys the recipe stages on the donor variant when a matched variant is a relocation copy, and hands the donor entry to findDrawRecipe/attachDrawRecipe (also through commitDraw on the pipelined path). With a donor, both read and update the donor entry's recipe list directly instead of looking the key up; without one they behave as before. - DrawWithRecipe rebuilds the push constant block from the current shaders and patches it through the recorded resources when the recipe has push stages, instead of replaying the bytes captured with the recipe. A relocated draw moves user words that end up in push constants, so the captured block would carry the first draw's values. Draws that are not relocated take the same path as before: their matched variants have no donor and recipe lookup still goes through drawCache. Measured on Hades (PPSA03355, discussion boykopovar#3234), where nearly every sprite draw is served through a relocated entry: lookups and attaches now succeed, but Graphics::Draw builds a recipe for only ~15 of ~100,000 draws per 10 s because the draws' ShaderResources are not Reusable(), so the frame rate does not change yet. The fix is what lets recipes apply to relocated draws once that is addressed.
A draw that misses on its own register key and is served by a relocated draw entry (relocateDrawEntry, boykopovar#4943) never got a draw recipe: - relocateDrawEntry builds a fresh DrawEntry with per-draw copies of the donor's variants. That entry is not stored in drawCache under the draw's key. - Driver::draw still passes the draw's own key to findDrawRecipe and, after recording, to attachDrawRecipe. Both look the key up in drawCache, find nothing and return, so a recipe built while recording such a draw is dropped and the next relocated draw of the same shape records in full again ("attaches 0" with APS5_TRACE_DRAW_CACHE). - The recipe records also compare stage identity against the per-draw copies, which are new objects on every draw, so even a stored record could never match. Changes: - DrawEntry and DispatchVariant gain a donor pointer. relocateDrawEntry sets it on the relocated entry and on every copied variant, always to the original cached entry/variant (a copy of a copy points at the root), so the chain never grows. - Driver::draw keys the recipe stages on the donor variant when a matched variant is a relocation copy, and hands the donor entry to findDrawRecipe/attachDrawRecipe (also through commitDraw on the pipelined path). With a donor, both read and update the donor entry's recipe list directly instead of looking the key up; without one they behave as before. - DrawWithRecipe rebuilds the push constant block from the current shaders and patches it through the recorded resources when the recipe has push stages, instead of replaying the bytes captured with the recipe. A relocated draw moves user words that end up in push constants, so the captured block would carry the first draw's values. Draws that are not relocated take the same path as before: their matched variants have no donor and recipe lookup still goes through drawCache. Measured on Hades (PPSA03355, discussion boykopovar#3234), where nearly every sprite draw is served through a relocated entry: lookups and attaches now succeed, but Graphics::Draw builds a recipe for only ~15 of ~100,000 draws per 10 s because the draws' ShaderResources are not Reusable(), so the frame rate does not change yet. The fix is what lets recipes apply to relocated draws once that is addressed.
11 participants
What
Addresses #4054: a guest driver that puts per-draw pointers in the user data (RADV does, for its descriptor sets and push constants) misses the draw cache on almost every draw, because the register key and the entry's memory runs both contain the pointer values.
mainnow caches a draw the second time its key is seen (perf(agc): cache a draw key the second time it is seen #5216). With moving pointers the exact key never repeats, so a repeat of the alias key counts as a second sighting too, and one draw of a shader set that has not been learned yet is admitted so the pointer words can be learned from it. Alias admission is limited: a shader set gets two such admissions on probation, and after that only one whose relocated entries actually hit (validated hits, not donors found) is admitted through its alias. Without this, aliases that come up exactly twice each inserted an entry nobody used (the D2R hit drop in the review).[draw-relocate]line counts the dropped ones.shaderDataUserWordsfield of a result, and its Linux-only size asserts expect the grown structs.main's incremental register fingerprint:RegisterStateKeygets an optionalshapeKeyout-parameter, and the user words are walked by one shared helper.APS5_NO_DRAW_RELOCATION=1turns it off.Related: #494 (draft) reuses a shape's decode and stages when only user data differs; this PR covers the case where the user data are pointers and the data behind them is unchanged or moved. They touch the same lookup and would need a rebase against each other.
This is the first part. Later steps I have locally and would send separately, each with its own numbers: relocating stages independently, reusing a stage when only buffer base addresses moved, and reusing draw snapshots.
Tested
Windows 11, NVIDIA GeForce RTX 5070 Ti, MinGW-w64 GCC 15.2.0, rebased on main
940eba9a2.ctest: 1269 of 1291 pass, 14 skipped.guest_call_trace_abort,windows_backtrace_reportandagc_driver_store_over_untrackedfail, and fail the same way on plain main here. Three newer tests,agc_driver_prepared_graphics_prebuild_0/_1andundeliverable_host_exception, fail under my test runner because it skips their environment changes and pass pattern; run with those, all three pass.ShaderDiskCache.cpponly compile on Linux. I have no Linux build; I compiled the structs under MinGW libstdc++ on x86_64, where all 31 asserted sizes match, including the ones this PR does not touch.agc_shader_disk_cache's result round trip now setsshaderDataUserWordsand compares it after decoding; it fails if the decoder drops them.agc_driver_draw_cache_relocation: a pixel shader loads its colour through a user-data pointer. The same contents drawn from eight addresses hit the cached draw from the fifth draw on (two draws teach the shader set, two more are the first and second sighting of the contents); other contents do not hit; an address seen before still hits. It fails withAPS5_NO_DRAW_RELOCATION=1. A second case draws 12 aliases that each come up exactly twice, between draws of a repeated one: at most 2 of them are cached and the repeated draw keeps all its hits (with the old admission rule all 12 were cached; the case fails on it).4430780bd(without perf(agc): cache a draw key the second time it is seen #5216) plus local changes outside this PR that the frontend needs to boot. Same build, relocation off against on, two unprofiled runs each: audio produced at 26.9% and 27.0% of real time against 38.5% and 39.1%.maina14ae8e23plus the same local changes) the same demo does not get faster with this PR alone: audio at 5.4% and 5.5% against 5.8% without it, unprofiled. About 40% of draws hit, but each draw on this base copies a 2 MiB guest buffer into a snapshot whether it hits or not, and that copy dominates. perf(agc): reuse cached draw stages when only buffer base addresses moved #5609 on top reaches 36%; see the numbers there.main940eba9a2, the same demo with this PR and perf(agc): reuse cached draw stages when only buffer base addresses moved #5609 on top against the same build without them (both with the same local changes the frontend needs to boot, immediate present, audio muted, three alternating unprofiled runs): audio produced at 34.5%, 35.7% and 35.6% of real time without, against 45.7%, 44.1% and 42.8% with.APS5_VERIFY_DRAW_ENTRIES=1, which captures every hit again and compares: 0 stages differed in every report (88,797 hits in the largest).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code