Repository navigation
perf(agc): reuse address-based draw builds whose inputs are unchanged - #6636
Closed
EagleBearerYS wants to merge 23 commits into
Closed
EagleBearerYS wants to merge 23 commits into
EagleBearerYS wants to merge 23 commits into
Conversation
…gistered An address-based build takes the cached address space when it is the device's, of the registry generation read before any lease, and no import was retired since; it then stamps the space's imports as used and brings the space's sparse, writable and heap mirrors up to date. Those steps now live in helpers of their own (spaceMiss, stampSpaceImports, refreshSparseMirrors, appendWritableMirrors), so a later use of an existing build can make the same proof and the same refresh as a build that takes the space. No behaviour change.
A draw whose stages reach guest memory through the BDA table built its resources on every draw: the lease of the registered memory, the upload, the page table and its fault buffer, the descriptor set and the image lookups. boykopovar#2657 skipped the resource cache for such draws because the build was never kept: its completion checked the faults, marked the pages the shader stored to, wrote the written ranges back and released the lease, once. A draw build over the cached address space is now kept when everything it depends on can be proved again cheaply: the space it was built over must be the one a build would take now (the device's, of the current registry generation, with no import retired since), its own regions must be imports or read-only image mirrors with unchanged serials, or snapshot copies whose bytes the draw's snapshots repeat in the same order; the images are proved as for any cached build. The read-only buffers and data words the content key leaves out must equal the build's: a hit with other ones rebinds them per draw, which an address-based build cannot. A reused draw takes the space again and refreshes its mirrors as a build does, and the imports are checked once more after the image proofs. Each draw keeps its own completion: its batch holds the space until then, and its completion checks the faults, marks the written pages and the written ranges served in place. Between draws the build pins no guest allocation, so a guest unmap never waits for the resource cache. The fault buffer, which holds the fault and written-page records, belongs to the build, so one draw at a time uses a build: a draw that repeats it before the previous draw's completion ran builds anew. A build whose draw faulted is not reused. Dispatches and draw recipes do not reuse address-based builds. APS5_NO_ADDRESS_DRAW_REUSE=1 builds every one as before.
The reuse of address-based draw builds rests on choices that are not measured on titles: one draw at a time per build (the fault buffer holds one draw's records), the snapshot list repeated in the same order, and no reuse for dispatches or draw recipes. Recorded as technical debt so they can be revisited with title measurements.
MarcZX10
reviewed
Oct 16, 2026
|
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.
|
11 tasks done
…ation A guest buffer descriptor can describe more than its build binds. Past the device's range limit the build cuts the size at the registered end, or binds an empty buffer when nothing is registered there, and it trims a stride overhang that reaches past the allocation. A reuse compared each descriptor's raw size with what the build had bound, so such a buffer always looked moved: an address-based draw built its resources again on every draw, and a cached build of another draw rebound a trimmed buffer at its raw size, past its allocation. The moved-buffer check and the alias checks a reuse repeats now both take the size from the build's own computation. The alias checks need it as well: with only the moved-buffer check changed, the reused draw failed with "shader buffer aliases the render target", which fails the driver. agc_driver_address_draw_reuse draws twice with a vertex buffer descriptor of 0xffffffff records whose render target lies above its 64 KiB allocation. The second draw must reuse the first one's build; without the fix it built again.
Contributor
|
In-game run of head ec17620: Epic Mickey: Rebrushed (PPSA15657, UE 4.27), short runs to the first level with No new failures. No driver failure, Speed: no measurable difference in this title. It has about 240 address-based draws per 10 s in the level. Means of the last four
Only one run with reuse off reached the level, so this is one comparison point. Most runs with reuse on sit at 0.8 to 1.1 ms per address-based draw, the same as that run; two runs averaged 4.4 and 10.2 ms. The PR doesn't log how many builds it reuses, so I can't tell how often reuse happens here. A Crash rate at level load. This title has an intermittent crash on
So reuse doesn't cause it; it makes it rarer. The PR isn't the fix, but this suggests the per-draw build, lease and completion of address-based draws on AI-assisted: yes (Claude Code) |
Contributor
Author
|
Thanks for the runs and the tables. You're right that nothing counts reuses yet. I'm adding a count of reuses, and of refused reuses by reason, to the draw profile output, which should also show whether the two slow runs (4.4 and 10.2 ms per address-based draw) went into refused proofs. Thanks as well for the crash counts with reuse on and off. I'll post the counts from Resident Evil Village (PPSA01556) here once the commit is in. |
Contributor
Author
|
On the level-load crash: a 64 KiB block zeroed shortly after the title read it matches a known gap listed in |
Both walks of the reuse proof skip a rejected image-table element, so it is never proved again. Two rejections are transient (a surface that is not mapped, a texture lookup that throws), and pages committed in a registered reserved range leave the registry generation alone: an address-based build could be reused with the element still rejected after its surface became valid. Such a build is now not reusable. The address-based proof could also throw out of Revalidate and fail the draw, where a fresh build would reject the element or report the error itself. Anything the proof throws now refuses the reuse.
Merged
11 tasks done
A batch whose end or submit fails is discarded without running its completions. The use count BeginUse took was only dropped by CompleteUse, so the build stayed in use and was never reused again. The count now goes with the token BeginUse returns, which the completion owns: it drops when the completion runs or is discarded. The mock's queue submit can now fail for the test.
A fresh capture lists a draw's snapshot regions sorted by address (ShaderMemory::Regions). A decode-cache hit lists the stages' code and their matched regions in stage order instead. SameSnapshots compares the two lists entry by entry, so the first hit after a decode miss failed the address-based reuse and built the draw again whenever the orders differed, as they do when the pixel shader's code lies below the vertex shader's. The draw now sorts its snapshots by address before it hands them on.
EagleBearerYS
added a commit
to EagleBearerYS/AnyPS5
that referenced
this pull request
Oct 16, 2026
EagleBearerYS
added a commit
to EagleBearerYS/AnyPS5
that referenced
this pull request
Oct 16, 2026
…mpletion-store fix and boykopovar#6636 reuse gap
11 tasks done
…rofile A tester of address-based draw reuse could not tell how often a draw reused its build: the driver counted no reuse, and two runs averaged 4.4 and 10.2 ms per address-based draw where others took about 1 ms. A refused reuse pays for its proof and then for the build, so the refusals and their reasons are what tell such runs apart. The resource cache now counts, for address-based draw builds, the proofs Revalidate runs, the reuses, and the refusals by reason: in use (the build's previous draw has not completed; refused before any proof), not reusable (its draw faulted), snapshots differ, the space did not resume (by its state, a replaced space, the device, the registry generation, the import epoch, or a sparse mirror that could not be refreshed), the image or buffer proof failed, the imports changed, the proof threw, and read-only buffers that moved after the proof passed. The time of refused proofs is summed under APS5_PROFILE_DRAW. A reused draw's hold of its space is counted until its use ends, and host import refusals, over the budget or of an import that would replace a held one, are counted with how many came while such a hold was live. The reused flag travels in the use token BeginUse returns, so the hold is released with the use count when the token goes, also when a batch is discarded without running its completions. A proof that throws is counted on its own, as "threw": the throw can come from resuming the space, from the proof or from the imports check, and counting it under any of those would hide an error among ordinary refusals. APS5_PROFILE_DRAW prints them every 10 s right after the [rescache] draws line, beside the pin-contention waits of the same window (the [address-sync] leases line prints those cumulatively on its own timer): "[rescache] address-based draws: P proofs, R reused; refused: in use N, ..., threw N, moved buffers N; ...; host import refusals: budget B (H while a reused draw held its space), held import ...; pin-contention waits W (T ms)". agc_driver_address_draw_reuse reads the counters: repeated unchanged draws count as reuses, a draw repeated before the previous one completed counts one refusal in use, a draw after the guest rewrote its pixel program counts one refusal for a changed snapshot, a draw after a guest write into memory it reads in place counts a reuse, the draw after a fault counts one refusal of a build that is no longer reusable, and a proof that throws counts one refusal that threw. The BDA resource tests check that a reused draw's hold is counted while its batch is pending and released when the batch fails to submit.
Contributor
|
@EagleBearerYS thanks,
The three failures without it:
Earlier runs of the same tree without the variable failed 2 of 5. With the flag, all 16 runs reached the level and played to the end of the run. So it points at the late store in AI-assisted: yes (Claude Code) |
Contributor
Author
|
@johnmaia thank you for the alternated runs. 0 of 16 with |
Contributor
Author
|
@johnmaia Thanks for the A/B. It pinned this down. A storage image's write-back is recorded on the GPU and can land after a later APR read into the same memory, which zeroes the file data. Host writes into guest memory also didn't wait for GPU writes pending there. A fix is on |
11 tasks done
13 tasks done
Contributor
|
Data point: RTX 3060 12 GB (NVIDIA 617.42), i5-12400F, Windows 10 19045, MinGW GCC Release. Minecraft Dungeons II (PPSA16064 01.100.100), title screen, no input, 200 s rows,
Neutral on this scene (within the row spread); no regression. 0 fatal. AI-assisted: yes (Claude Code) |
12 tasks done
…ss reuse proof
The reuse proof of an address-based draw build compares the draw's
snapshots with guest memory for live regions and with the build's
copies for the others. A write that earlier work in the batch recorded
over a snapshot's range (a fill, a copy, a dispatch storing through a
descriptor) may still be pending when the proof runs: memory then still
holds the bytes the capture saw, the proof passes, and the draw runs
after the write on a program the decoder specialized on the old bytes.
Revalidate now syncs the recorder through every snapshot range that a
recorded write overlaps before it compares the snapshots, so the
comparison sees the memory the earlier work leaves, and a capture made
before that write is refused as a changed snapshot. A range with no
pending write costs one overlap test. Stores through the BDA table are
not recorded as pending writes; a later use reads them in place.
agc_driver_address_draw_reuse gains the case: a draw whose snapshots
include the guest pointer its pixel shader reads, reused once, then a
FillBuffer recorded over that pointer and the same draw with the old
capture in the same batch. The draw is refused for its snapshots,
builds again and reads the new pointer; without the sync it was reused
("1 reuses, 0 builds").
Contributor
Author
|
@adriadam10, thanks for the review. All three points: Conflicts with The macOS failure. Your and @jotitadev's reading is right. At 28433ba, fc9c541 keeps it covered on CI:
With #7650's line removed, the new registration fails at the address-based case ("an address-based draw build whose sampled image's memory became a multisampled target was reused") and the existing one at the non-address case ("a draw build without the address table whose sampled image's memory became a multisampled target was reused"). With it, each passes 20 runs in a row on Linux, and on GitHub's runners (run) both pass 5 runs in a row with the tests tied to the changed files. The @jotitadev, could you run The perf pair. I'll post a Resident Evil Village (PPSA01556) pair from a scene with address-based draws: |
EagleBearerYS
added a commit
to EagleBearerYS/AnyPS5
that referenced
this pull request
Oct 21, 2026
This was referenced Oct 21, 2026
main brings boykopovar#8224, which cuts a read-only strided buffer that runs less than one stride into the render target at the target, in addGuestBuffer and in CheckBufferAliases. CheckBufferAliases keeps this branch's GuestBufferBytes size and applies main's cut to it, as addGuestBuffer does after guestBufferBytes.
…uild does Since boykopovar#8224, a read-only strided buffer that runs less than one stride into the render target is bound only up to the target, and the cached build keeps that size. MovedReadOnlyBuffers compared it with the size without that cut, so a hit with the same words took the buffer as moved and rebound all of it, over the render target, and an address-based build holding such a buffer was never reused. GuestBufferBytes takes the target and applies the cut. The build, CheckBufferAliases and MovedReadOnlyBuffers pass the draw's target for read-only buffers, so all three size a buffer the same way. agc_driver_graphics checks a cached build whose read-only stride-60, 4-record buffer runs 8 bytes into the target: the same words list nothing moved, a buffer moved to run 4 bytes in is listed at 236 bytes, and one moved to run a whole stride in keeps its 240.
The entry described each proof the reuse makes. It now names where they are (ShaderResources::Revalidate) and keeps what stays open: one draw at a time per build, no reuse for dispatches and draw recipes, and no measurement in a title. It goes from 1,537 to 871 characters.
12 tasks done
github-actions
Bot
added
needs rebase
12 tasks done
Contributor
Author
|
@adriadam10, the Resident Evil Village (PPSA01556) runs show this PR doesn't pay off there, so I'm closing it rather than make you wait for the pair I promised. What the runs showed. Both ran at the title's menu, with this PR at fc9c541. They aren't on
Why. The proof compares the addresses a build read through its address space. Most of those reads are per-draw constants whose address changes from one draw to the next: mesh-stage constants of 0x50 and 0x70 bytes, and 4- and 8-byte pixel-stage reads. Ten draw keys read 9 or more distinct addresses each, and they account for 55% of the address reads the second run logged. Keeping a few builds per key would match about a quarter of those reads at best. A gain here would need a proof over what the build derives from those reads, not over their addresses. That's a bigger change than this PR. Thanks to you for the review, to @jotitadev for the macOS run, to @MarcZX10 for the review comments, to @simonbronk for the Onimusha: Way of the Sword (PPSA27836) runs, and to @johnmaia for the Epic Mickey: Rebrushed (PPSA15657) runs. |
teepean
added a commit
to teepean/AnyPS5
that referenced
this pull request
Oct 21, 2026
… (local test) Address-based builds take boykopovar#6636's Revalidate path with the full proof; the proof memo stays for the other builds (memoRevalidate). The repeated template of the previous draw goes through the same in-use check and snapshot proof as a cache hit, and the [address] line keeps its keeps and stamps laps around boykopovar#6636's stampSpaceImports. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
teepean
added a commit
to teepean/AnyPS5
that referenced
this pull request
Oct 21, 2026
Celegans12
added a commit
to Celegans12/AnyPS5-Ghost-of-Yotei
that referenced
this pull request
Oct 23, 2026
… (APS5_NO_BDA_TEMPLATES) Stage 1 of research/structural/committer-key.md. An address-table draw build whose stages never store through BDA (SPIR-V check), with no written buffer, written image or GPU copy, is cached as a template: - later draws use it while earlier uses are still in flight (it is never written after its build: each use takes its own descriptor set for changed data words or moved read-only buffers, as non-address templates do); - a hit proves the address space (resume), the textures and imports as boykopovar#6636's reuse does, and its captures with SnapshotsServed: every capture lies in live registered memory and equals it, or equals one of the build's own snapshot copies, and every snapshot copy is captured again. Live memory is read in place through the page table, so it may change between uses. Builds that store through BDA are cached only as before (boykopovar#6636, off in the README env). APS5_NO_BDA_TEMPLATES=1 (a RuntimeSwitch, APS5_AB-flippable) restores today's behaviour. [bda-template] counts hits while a use is in flight and per-use sets. No SourceVersion change. Tests (agc_driver_graphics, BDA resource tests): two uses in flight; a store to live memory between uses is taken, a stale capture, a changed snapshot copy, a missing or outside capture refuse; other data words get their own set and never reach the template; a storing build keeps one use at a time; a rejected image-table element refuses a read-only template too; the switch restores today's refusals. AI-assisted: yes
alex73630
pushed a commit
to alex73630/AnyPS5
that referenced
this pull request
Oct 24, 2026
Integrate PR boykopovar#6636 at ca05f74. Revalidate snapshots, address spaces, imports and image resources before reuse, and retain each draw's resources through its completion. Keep the existing DeviceReads output overload and exclude packed blend targets from reuse. Apply guest allocation bounds to alias checks for both address-based and ordinary template hits. Refs boykopovar#6636
alex73630
pushed a commit
to alex73630/AnyPS5
that referenced
this pull request
Oct 24, 2026
Integrate PR boykopovar#6636 at ca05f74. Revalidate snapshots, address spaces, imports and image resources before reuse, and retain each draw's resources through its completion. Keep the existing DeviceReads output overload and exclude packed blend targets from reuse. Apply guest allocation bounds to alias checks for both address-based and ordinary template hits. Refs boykopovar#6636
alex73630
pushed a commit
to alex73630/AnyPS5
that referenced
this pull request
Oct 24, 2026
Integrate PR boykopovar#6636 at ca05f74. Revalidate snapshots, address spaces, imports and image resources before reuse, and retain each draw's resources through its completion. Keep the existing DeviceReads output overload and exclude packed blend targets from reuse. Apply guest allocation bounds to alias checks for both address-based and ordinary template hits. Refs boykopovar#6636
11 tasks done
EagleBearerYS
added a commit
to EagleBearerYS/AnyPS5
that referenced
this pull request
Oct 31, 2026
A tester of address-based draw reuse cannot tell how often a draw was served by its cached build, or why it was not: a refused reuse pays for its proof and then for the build. Every address-based draw that could take part in the resource cache (recordable, its stages compiled, the reuse on) now counts exactly one outcome: - reused: a hit served by the cached build; - built: a new build that the cache keeps for later draws; - not read-only: a stage stores through addresses (NeverReusable); - not space-only: the build cannot be kept, as some of its bindings lie outside the cached address space (or noteReusable keeps it out for another reason: copied writes, multisample sources, the loop guard); - snapshot outside the space (copied): a snapshot of the draw lies outside the space, or in a region that is not read live; - partial overlap: a snapshot straddles the end of a region; - snapshot changed: a snapshot no longer equals guest memory, or lies under a unit shadow; - moved GuestBuffers element and patched data word: the moved-buffer proof refused the hit, for a data word under a misalignment patch or for anything else; - space or epoch change: the space could not be retaken (replaced, or its device, registry generation or import epoch moved); - in use: the cached build still holds its space for another draw, which the hit no longer shares (nor removes from the cache); - revalidation: Revalidate refused the hit. SnapshotsServedInPlace returns which of its checks refused, and MovedReadOnlyBuffers says when a patched data word did. APS5_PROFILE_DRAW prints them every 10 s after the [rescache] draws line, as boykopovar#6636's 89ab0ea did for its reuse: "[rescache] address-based draws (10 s): reused N, built N; refused: not read-only N, ...". The counters are cumulative (AddressDrawReuseCounters) and the line shows a window. agc_driver_address_draw_space_reuse's counters case draws through each refusal it can reach and requires each to count under its own reason (two reuses, a space change after an allocation is mapped, a snapshot outside the space and the uncached builds after it, a snapshot across the end of an allocation, a snapshot that differs from memory, a storing pixel shader), and the outcomes of all its draws to add up to the number of draws.
EagleBearerYS
added a commit
to EagleBearerYS/AnyPS5
that referenced
this pull request
Oct 31, 2026
…reuse proof The reuse proof of a space-reuse draw build compares the draw's snapshots with guest memory inside the cached space. A write that earlier work in the batch recorded over a snapshot's range (a fill, a copy, a dispatch storing through a descriptor) may still be pending when the proof runs: memory then still holds the bytes the capture saw, the proof passes, and the draw runs after the write on a build proven against the old bytes. As boykopovar#6636's 6e5b5bd did for its proof, the hit now syncs the recorder through every snapshot range a recorded write overlaps before SnapshotsServedInPlace compares them, so the comparison sees the memory the earlier work leaves and a capture made before the write is refused as a changed snapshot. A range with no pending write costs one overlap test. agc_driver_address_draw_space_reuse gains the case and the moved-ring cases: - pending-snapshot-write: a draw whose snapshots include the pointer its pixel shader loads, reused once, then a FillBuffer recorded over that pointer and the same draw with the old capture in the batch. It is refused for a changed snapshot and builds again; on the previous commit it was reused ("reused 1 and 0 builds"). - ring-same-constants: each draw writes the same constants into the next slot of an eight-slot ring in an imported allocation, passes the slot's address in user data and its bytes as a snapshot; all fifteen draws after the first are reused. - ring-own-constants: as above with other constants per draw; each draw renders its own and all are reused. - ring-old-slot-write: a CPU write to the previous slot after each draw is recorded leaves the reused draw's result alone. - ring-refusals: a moved snapshot outside the space is refused under its reason; a moved read-only vertex buffer inside the space is bound anew and reused (boykopovar#9508's moved-buffer path), and one that a pending fill overlaps is refused as a moved GuestBuffers element. The ring cases pass from the default-path commit on (with APS5_NO_ADDRESS_DRAW_REUSE=1 they fail, 15 builds for 15 draws). No case reaches the patched-data-word refusal: the stage this test builds has no data word under a misalignment patch. docs/dev/debt/libSceAgcDriver.md records the reuse and what is not measured.
12 tasks done
oneandonlydean
pushed a commit
that referenced
this pull request
Nov 4, 2026
A tester of address-based draw reuse cannot tell how often a draw was served by its cached build, or why it was not: a refused reuse pays for its proof and then for the build. Every address-based draw that could take part in the resource cache (recordable, its stages compiled, the reuse on) now counts exactly one outcome: - reused: a hit served by the cached build; - built: a new build that the cache keeps for later draws; - not read-only: a stage stores through addresses (NeverReusable); - not space-only: the build cannot be kept, as some of its bindings lie outside the cached address space (or noteReusable keeps it out for another reason: copied writes, multisample sources, the loop guard); - snapshot outside the space (copied): a snapshot of the draw lies outside the space, or in a region that is not read live; - partial overlap: a snapshot straddles the end of a region; - snapshot changed: a snapshot no longer equals guest memory, or lies under a unit shadow; - moved GuestBuffers element and patched data word: the moved-buffer proof refused the hit, for a data word under a misalignment patch or for anything else; - space or epoch change: the space could not be retaken (replaced, or its device, registry generation or import epoch moved); - in use: the cached build still holds its space for another draw, which the hit no longer shares (nor removes from the cache); - revalidation: Revalidate refused the hit. SnapshotsServedInPlace returns which of its checks refused, and MovedReadOnlyBuffers says when a patched data word did. APS5_PROFILE_DRAW prints them every 10 s after the [rescache] draws line, as #6636's 89ab0ea did for its reuse: "[rescache] address-based draws (10 s): reused N, built N; refused: not read-only N, ...". The counters are cumulative (AddressDrawReuseCounters) and the line shows a window. agc_driver_address_draw_space_reuse's counters case draws through each refusal it can reach and requires each to count under its own reason (two reuses, a space change after an allocation is mapped, a snapshot outside the space and the uncached builds after it, a snapshot across the end of an allocation, a snapshot that differs from memory, a storing pixel shader), and the outcomes of all its draws to add up to the number of draws.
oneandonlydean
pushed a commit
that referenced
this pull request
Nov 4, 2026
…reuse proof The reuse proof of a space-reuse draw build compares the draw's snapshots with guest memory inside the cached space. A write that earlier work in the batch recorded over a snapshot's range (a fill, a copy, a dispatch storing through a descriptor) may still be pending when the proof runs: memory then still holds the bytes the capture saw, the proof passes, and the draw runs after the write on a build proven against the old bytes. As #6636's 6e5b5bd did for its proof, the hit now syncs the recorder through every snapshot range a recorded write overlaps before SnapshotsServedInPlace compares them, so the comparison sees the memory the earlier work leaves and a capture made before the write is refused as a changed snapshot. A range with no pending write costs one overlap test. agc_driver_address_draw_space_reuse gains the case and the moved-ring cases: - pending-snapshot-write: a draw whose snapshots include the pointer its pixel shader loads, reused once, then a FillBuffer recorded over that pointer and the same draw with the old capture in the batch. It is refused for a changed snapshot and builds again; on the previous commit it was reused ("reused 1 and 0 builds"). - ring-same-constants: each draw writes the same constants into the next slot of an eight-slot ring in an imported allocation, passes the slot's address in user data and its bytes as a snapshot; all fifteen draws after the first are reused. - ring-own-constants: as above with other constants per draw; each draw renders its own and all are reused. - ring-old-slot-write: a CPU write to the previous slot after each draw is recorded leaves the reused draw's result alone. - ring-refusals: a moved snapshot outside the space is refused under its reason; a moved read-only vertex buffer inside the space is bound anew and reused (#9508's moved-buffer path), and one that a pending fill overlaps is refused as a moved GuestBuffers element. The ring cases pass from the default-path commit on (with APS5_NO_ADDRESS_DRAW_REUSE=1 they fail, 15 builds for 15 draws). No case reaches the patched-data-word refusal: the stage this test builds has no data word under a misalignment patch. docs/dev/debt/libSceAgcDriver.md records the reuse and what is not measured.
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
A draw whose shaders reach guest memory through the BDA table (an address-based draw) builds its resources again on every draw: the lease of the registered memory, the upload, the page table and its fault buffer, the descriptor set and the image lookups. #2657 left such draws out of the resource cache because their build could not be kept: its completion checked the faults, marked the pages the shader stored to, wrote the written ranges back and released the lease, once per build.
In Resident Evil Village (PPSA01556) at the menu, most of queue 0's draws are address-based builds (
APS5_PROFILE_DRAW=1,[draws]lines per 10 s):main6ea62f9, with the fixes it needs to reach the menu: 218 of 224 draws, at 29.8 ms each (BDA 29829 (218));mainplus open PRs: 867 of 1,392 draws, at 3.8 ms each (BDA 3821 (867)).The fix. A draw build over the cached address space (#5542) is now kept and reused by a later draw with the same content key, when everything it rests on can be proved again cheaply:
A reused draw takes the space again and refreshes its mirrors as a build does, and its imports are checked once more after the image proofs. Each draw keeps its own completion: its batch holds the space until then, and the completion checks the faults and marks the written pages and the written ranges served in place. Between draws the build pins no guest allocation, so a guest unmap never waits for the resource cache.
Buffers whose descriptor reaches past their allocation (bae2cd4). The two reuse checks of read-only buffers,
MovedReadOnlyBuffersandCheckBufferAliases, took each guest buffer descriptor's raw size. A build binds less: past the device's range limit it cuts the size at the registered end, and it trims a stride overhang past the allocation. So a draw with such a descriptor was never reused, and fixing only the moved-buffer check would have made the reused draw fail the alias check. Both checks now use the size the build binds (ShaderResources::GuestBufferBytes, split out ofaddGuestBufferwithout changing what it binds). #7044 (AlxcBTW, merged) addedguestBufferBytesfor the same size, for its moved compute dispatches. Since the merge ofmain55f82fe that size has one body: main's, inGuestBufferBytes, whichguestBufferBytesforwards to, so builds, both reuse checks and #7044's dispatch check size a buffer the same way. Templates of non-address draws use the same checks, so they get the same fix. This was found by an after-the-fact completeness review, not by a title.Review follow-ups. The same review found three more gaps, each fixed with its test:
BeginUsenow returns a token that releases the use when it is destroyed: at the end of the completion as before, or when the batch is discarded.SameSnapshotsand built again. Both paths now sort them by address, then size.Recorded GPU writes over a snapshot (6e5b5bd). The reuse proof compared a draw's snapshots with memory right after its in-use check. A write recorded earlier in the batch over a snapshot's range (a fill, a copy, a dispatch storing through a descriptor) that hadn't run yet left memory holding the bytes the capture saw, so the proof passed and the reused draw ran after the write on a program specialized on the old bytes. A draw that builds anew syncs through such writes in the decoder's own validation. The proof now first completes the recorded writes over each snapshot's range (
CompleteSnapshotWrites), so such a capture is refused and the draw builds again. Stores through the BDA table aren't recorded as pending writes; a later use reads them in place. This was found by a later review, not by a title.Profile counters. With
APS5_PROFILE_DRAW=1, a[rescache] address-based draws:line follows the[rescache] draws:line every 10 s, in windows that had an address-based proof or an in-use refusal. It gives the proofs, the reuses, the refusals by reason (in use, not reusable, snapshots, the space and why, images or buffers, imports, a proof that threw, moved buffers), the time spent in refused proofs, the host import refusals (budget and held import, and how many came while a reused draw held its space), and the pin-contention waits. The counts are relaxed atomics, and without the profile nothing prints. The use token also carries whether its use was a reuse, so a discarded batch releases that count too. From the test:What it leaves out (also in the debt file):
APS5_NO_ADDRESS_DRAW_REUSE=1builds every one as before.With the draw pipeline (#1202, on by default). A pipelined direct draw is recorded by the committer in
commitDraw, and any other draw inline afterlockForDraw; both hold the GPU lock throughGraphics::Draw, where the cache lookup, the proofs andBeginUserun. Completions run under the hold that reaps or syncs their batch. So the build's use count and its fault verdict change only under that lock, and pipelining opens no reuse race.The commits:
refactor(agc): split the cached address space checks out ofAcquireRegistered(spaceMiss,stampSpaceImports,refreshSparseMirrors,appendWritableMirrors), so a reuse makes the same proof and refresh as a build. No behaviour change.perf(agc): the reuse, with its tests.docs(agc): the debt entry.fix(agc): buffers whose descriptor reaches past their allocation (above).fix(agc): refuse address-based reuse after rejections or a throw.main5d9dcc4, which brings fix(agc): discard recorder batches after submission failure #5837's discarding of failed batches.fix(agc): release a draw build's use when its batch is discarded.fix(agc): order a draw's snapshots the same on decode misses and hits.perf(agc): count address-based draw reuses and refusals in the draw profile.fix(agc): complete recorded GPU writes over snapshots before an address reuse proof (above).main55f82fe, which brings perf(agc): serve compute dispatches whose read-only buffers moved from the cached template #7044; itsguestBufferBytesand this PR'sGuestBufferBytesnow share one body (above).main2626e5e, which brings fix(agc): harden host import construction and mutation waits #7339's host import construction guard.WaitForLeasesAndImportstakesmain's signature, withoutnoexcept, and this PR'sliveSnapshotComparedstays beside it.main0d078f7, which brings fix(libSceAgcDriver): normalize reserved bit 30 in guest buffer descriptors #7089's masking of reserved bit 30 in guest buffer descriptors.GuestBufferByteskeeps this PR's signature and takesmain's mask.docs(environment): theAPS5_NO_ADDRESS_DRAW_REUSErow indocs/dev/ENVIRONMENT.md, which lists every switch since feat(tools): add the APS5_* and ANYPS5_* environment reference and check it #6078.main6efb726, which brings fix(agc): drop resource-cache entries that hold released sampled textures #6492's debt entry next to this PR's; both are kept.main6696def, which brings fix(agc): enforce MSAA guards for ordinary image cache hits #7650's multisample target guards for cached images and fix(agc): plan a heap again when its mirror's range has left the write watch #6646's mirror check.importAllocationtakesmain's step-down andpinnedBytes, and counts a refusal only where the import is refused. A cached space is reused only when each of its heap and sparse mirrors is still watched or can't be imported now, both when it is acquired and inResumeSpace, and its imports are stamped once per space, asmaindoes.test(agc): reused draw builds over a sampled image whose memory became a multisampled target, with watched imports and without the address table (below).Cost. A reuse makes the proofs above in place of a build. Synthetic draw on lavapipe (200 draws, median of 3): with each draw waited for, the per-draw time drops 57%, 50% and 48% at 512, 6,600 and 10,800 registered ranges (74.3 to 32.2, 197.2 to 99.2, 283.5 to 148.3 us). Back to back, a draw's build is still in flight when the next draw repeats it, so nothing is reused, and the time is within 3% of
main's. Resident Evil Village's pair, with and withoutAPS5_NO_ADDRESS_DRAW_REUSE=1, follows in a comment.Overlap. Conflicts with open PRs are in shared lines, resolved by keeping both sides:
ShaderResources.cpp. Merged together, that condition must also keep an address-based build from being reused.Draw.cpp, where its prepared content key and this PR's in-use check combine.BdaResourcestable it was built with, and fix(agc): exclude native MSAA spans from BDA tables #6628 makes that table depend on its multisample guard generation, which none of this PR's space checks read. Whichever merges second hasBdaResourceskeep the guard generation its table was built or found under and exposeTableCurrent(), and has the address-based reuse proof refuse a hit when it returns false, with a test that adds and then clears a guard around a cached address-based draw.main6696def: a cached space is reused only when each of its heap and sparse mirrors is still watched or can't be imported now, both whenAcquireRegisteredfinds it current and inResumeSpace. A mirror whose import is refused doesn't count against the space: its existing refreshes bring that mirror up to date in full.UploadFinish, which a reused build never runs: it takes its space back throughResumeSpaceand proves itself inRevalidate. Whichever merges second publishes the space's unit shadows afterproveBuildinRevalidate(aPublishSpaceShadows()call), with a test that reuses a draw over a shadowed heap.Tested
agc_driver_address_draw_reuse(tests/execution/AddressDrawReuse.cpp, lavapipe):agc_driver_graphics(tests/BdaResources.cpp) additions:SpaceRegions,DirectWrites,SameSnapshots, suspending and resuming the space.agc_driver_address_draw_reuse'sOversizedVertexBuffercase (bae2cd4) has a vertex buffer descriptor of 64 GiB over a 64 KiB allocation. The draw is drawn, waited for and drawn again, and both draws must store through the BDA table, with the second reusing the first one's build. On GitHub's runners (run), withShaderResources.cppandDraw.cppreverted to ec17620's, it fails in all 3 runs on Linux ("a draw whose vertex buffer descriptor reaches past its allocation built its resources again"). With them, it andagc_driver_graphicspass 10 runs in a row. On Windows the draw test skips without a Vulkan device andagc_driver_graphicspasses.main44df216 (only the new test applied):agc_driver_address_draw_reusefails on Linux (run 38795295627); Windows skips it without a Vulkan device.agc_driver_address_draw_reuse,agc_driver_address_space_teardown,agc_sparse_buffer_holes,agc_compute_descriptor_layout_flags,agc_descriptor_pool_reset_failureand the three alpha-to-coverage and centroid state tests): all pass on Linux and Windows (run); Windows skips the lavapipe one. The libc and libkernel suites pass apart from libc'sprecise_waiton Linux (a timing test; it passed in the red run) and libkernel'sguest_kqueueon Windows, which fails onmainon that runner too.^agcsweeps at the head:main's code:agc_driver_mesh_index_cacheaborts inside llvmpipe's LLVM ("Cannot emit physreg copy instruction"), also withmain's code (run). fix(agc): zero the pending key store block, reapply #5791 with an llvmpipe skip #6560 now skips it on llvmpipe.agc_driver_preparation_scope_settlefails only under the sweep's parallel load and passes alone.agc_driver_pm4_replay_tool(0xc0000139), also fails onmainon that runner.maindoesn't build in full with MinGW since fix(agc): keep the gate run going past indirect dispatches whose arguments stay on the GPU #5747 (alignas(65536)in a test), so this sweep ran with test(agc): align the indirect arguments block at run time #6452's fix for that test applied, for this check only.main44df216; it now mergesmain5d9dcc4.agc_driver_graphics'saddressRejectedTableElementTests: an address-based draw whose one image-table element points at reserved pages, which the build rejects as unmapped; the pages are then committed with no registry change. Red: "an address-based draw build whose image table element was rejected as unmapped was reused after the element's surface was committed".agc_driver_address_draw_reuse'sThrowingProof: after the build, the sampled image's memory becomes a 4x multisampled color target, so the reuse proof's texture lookup throws. Red, with rejections blocked but no catch: "the reuse proof of an address-based draw build whose sampled image's memory became a multisampled target threw instead of refusing the reuse".agc_driver_graphics'saddressFailedSubmissionTests: a build whose batch fails to submit (the mock returnsVK_ERROR_OUT_OF_HOST_MEMORY) must not stay in use. Red: "an address-based draw build stayed in use after the batch that took it failed to submit".agc_driver_address_draw_reuse'sDecodeCacheHit: a decode miss, a second miss and a decode-cache hit of the same submitted draw build once. Red: "a submitted draw that hit the decode cache built its resources again after a decode miss of the same draw".^agcpasses 607 of 607 (43 skipped), and each commit builds and passes its own tests.guest_kqueue, which fails onmainon that runner too.agc_driver_address_draw_reuse'sReuseCounters: four waited draws count 4 proofs and 4 reuses; two draws back to back count one reuse and one "in use" refusal; a rewrite of the pixel program's bytes counts one "snapshots" refusal.GuestWrite,FaultandThrowingProofcheck their counts too, and the reused case ofaddressFailedSubmissionTestschecks that a reused use's hold is released when its batch fails to submit.^agcpasses 607 of 607 (43 skipped).agc_driver_address_draw_reuseandagc_driver_graphicswith the tests run alongside pass 10 runs in a row on Linux and Windows (the new tests need the counters' API, so they don't build on the previous head). The suites' only failures were libc'sprecise_waiton Linux, a timing test, and libkernel'sguest_kqueueon Windows, which fails onmainthere too.agc_driver_address_draw_reuse'sPendingSnapshotWriterecords aFillBufferthat rewrites the pointer the pixel shader loads and, in the same GPU-lock scope, offers the same draw with its old capture. It must read the new value, count one more snapshots refusal and no reuse, and build once more. Red without the sync: "a draw whose captured snapshot a pending GPU write changed was not refused for a changed snapshot and rebuilt (1 reuses, 0 builds)". With it the test passes 20 runs in a row, and^agcpasses 607 of 607 (43 skipped; a first sweep's one failure wasagc_driver_preparation_scope_settleunder the sweep's parallel load, which passes 5 of 5 alone). On GitHub's runners (run) the new case fails 2 of 2 runs on Linux at the previous head, and with the sync it and the tests tied to the file pass 10 runs in a row. On Windows (run, with test(agc): align the indirect arguments block at run time #6452's MinGW fix for an unrelated test applied for this check only),agc_driver_graphics,agc_driver_address_space_teardownandagc_sparse_buffer_holespass 10 runs and the draw test skips without a Vulkan device. The^agcsweeps' only failures areagc_driver_preparation_scope_settleon Linux (and, in the first run,agc_frame_dump_io's pipe flake, whichmainhas since fixed in 4b8ccd2) andagc_driver_pm4_replay_tool(0xc0000139) on Windows, as before.ThrowingProoffailed with imports watched. The head that run tested, 28433ba, predates fix(agc): enforce MSAA guards for ordinary image cache hits #7650, which refuses that case inproveBuild(RequireNoMultisampleColorTarget) before either proof runs; the merge of 6696def brings it. Linux (lavapipe), CI's configuration:agc_driver_address_draw_reuse_watched_imports: the same test withAPS5_WRITE_WATCH_IMPORTS=watch, registered on Linux and macOS builds.ThrowingProofalso checks a cached build without the address table.agc_driver_address_draw_reuseat the non-address case ("a draw build without the address table whose sampled image's memory became a multisampled target was reused"). With it, each passes 20 runs in a row.^agcpasses 792 of 793 (57 skipped). The one failure,agc_driver_sgpr_pattern_fill_keys, fails under the sweep's parallel load onmaintoo.precise_waiton Linux, a timing test.main6696def (084de35), on GitHub's runners (run): the reuse tests and the tests tied to the merged files pass 5 runs in a row on Linux and Windows (Windows skips the lavapipe ones), and the libc and libkernel suites pass.main0d078f7 and 6efb726 (ff5fe23, then the row 4bacab4, then ca05f74, the head), on GitHub's runners: at 4bacab4 the^agctests pass 3 runs in a row on Linux (710, 0 failed), and the libc and libkernel suites pass (run); at ca05f74 they pass again on Linux (710, 0 failed), as do the suites (run). On Windows the only failures at both areagc_driver_pm4_replay_toolandagc_shader_replay_tool, as in the runs below, and the suites pass.main2626e5e (3db18a2), on GitHub's runners (run): the^agctests (705, 0 failed), with the reuse tests and fix(agc): harden host import construction and mutation waits #7339'sagc_driver_host_import_lifetimeandagc_driver_host_import_mutation_race, pass 3 runs in a row on Linux, and the libc and libkernel suites pass. On Windows the only failures areagc_driver_pm4_replay_toolandagc_shader_replay_tool, as in the run below.main55f82fe (b5d0a2a), on GitHub's runners (run): the reuse tests and the 14 tests tied to the changed files pass 3 runs in a row on Linux, as do the^agctests (682, 0 failed), and the libc and libkernel suites pass. On Windows the only failures areagc_driver_pm4_replay_toolandagc_shader_replay_tool, which fail the same way on that runner for an unrelated branch (run).[draws]lines above.APS5_TRACE_SECONDS=60:30). The base ismain84b0d2f with fix(agc): import native multisample depth whose HTILE is not ITERATE_256 #6557, fix(agc): retire an unwritten native multisample depth surface on guest access #6727 and fix(agc): replace with a stencil operation value the pixel shader exports #6762, 6 runs (3 before and 3 after this PR's); this PR is merged on top of it, 3 runs (comment):Frame p50: 77.9 to 84.5 ms on the base (mean 82.0); 78.7, 85.6 and 87.6 ms with this PR (mean 84.0). Base p90: 84.3 to 93.6 ms; p90 with this PR wasn't given.
No gain in this scene. simonbronk reads it as no measurable change and notes that the menu may not reach the path this PR speeds up.
simonbronk's summary lines for his first pair, the first base run (
base-r1) and the first run with this PR (pr6636-r1), from his comment. The runs and traces are his:{"commit":"5c521efea4cb","dirty":false,"env":{"APS5_BINDLESS_MATERIAL_SCAN":"65536","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"60:30"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 617.42","os":"Windows 10.0.26200","resolution":"3840x2160","window":{"spec":"seconds 60:30","first_present":414,"ms":30052.503},"presents":256,"frame_ms":{"p50":84.478,"p90":93.612,"p99":972.971,"max":1952.003},"gpu_busy_ms_per_present":27.587,"worker_busy_pct":{"queue worker 0x38":1.653,"queue worker 0x20":35.070,"queue worker 0x0":91.809,"queue worker 0x28":64.589},"top_phases_ms_per_frame":{"Driver.Packet":224.620,"Driver.Packet.memory_wait":107.008,"Driver.Packet.draw":63.092,"Driver.Draw":63.071,"Driver.Packet.dispatch":52.429,"Driver.Dispatch":50.815,"Driver.Draw.draw_and_resource_release":48.789,"Graphics.Draw":48.759},"trace_dropped":false,"trace":"aps5-trace-5c521efea4cb-1894532.json.gz"} {"commit":"9f86b47f65bc","dirty":false,"env":{"APS5_BINDLESS_MATERIAL_SCAN":"65536","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"60:30"},"gpu":"NVIDIA GeForce RTX 5070","driver":"NVIDIA 617.42","os":"Windows 10.0.26200","resolution":"3840x2160","window":{"spec":"seconds 60:30","first_present":396,"ms":30238.620},"presents":273,"frame_ms":{"p50":78.703,"p90":87.730,"p99":951.593,"max":1017.455},"gpu_busy_ms_per_present":25.760,"worker_busy_pct":{"queue worker 0x38":1.992,"queue worker 0x20":38.049,"queue worker 0x0":97.744,"queue worker 0x28":72.995},"top_phases_ms_per_frame":{"Driver.Packet":231.188,"Driver.Packet.memory_wait":112.370,"Driver.Packet.draw":60.955,"Driver.Draw":60.933,"Driver.Packet.dispatch":55.705,"Driver.Dispatch":54.322,"Driver.Draw.draw_and_resource_release":46.104,"Graphics.Draw":46.074},"trace_dropped":false,"trace":"aps5-trace-9f86b47f65bc-1902128.json.gz"}That pair's frame p50 is 84.5 ms without this PR and 78.7 ms with it, but as simonbronk says, it isn't representative: his other runs overlap, so the pair doesn't show a reliable gain. Thanks to simonbronk for the runs and the lines.
python3 tools/check_conventions.py --base origin/main: passed.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)