Skip to content

perf(agc): mirror a sparse descriptor heap instead of copying it per dispatch - #3167

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
CnRJay:perf/agc-sparse-heap-mirror
Oct 11, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
CnRJay:perf/agc-sparse-heap-mirror

Conversation

@CnRJay

@CnRJay CnRJay commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

What

PPSA28416 (STAR WARS: Galactic Racer, UE5) binds a 4 GiB GPU heap of which 528 MiB is committed. With the cut from #2992 the heap is copied only up to its last backed byte, but it is still copied for every dispatch that binds it: 528 MiB at ~90 ms each, over thousands of dispatches during boot, which is the 0.1 FPS the title showed.

A sparse descriptor heap region is now bound to a persistent heap mirror of the registered range that backs it, instead of being copied:

  • The registered range is looked up for the part of the region that is needed (the later of its last committed byte and the start of its last view, plus 4 bytes), not for the whole 4 GiB reservation, which no registered range covers.
  • The mirror is a sparse mirror (pages still committed on demand) or a plain heap mirror of a fully committed range. A plain one is refreshed from the write watch like the heap mirrors of an address-based build.
  • servesView checks the view start against the extent of any mirror, so a view past the mirror falls back to the copy.
  • Writes go back through the mirror's own write-back; only changed 64 KiB blocks are read again.

This branch carries the commit of #2992 (rebased, as that PR conflicts with main); drop it when #2992 merges.

Tested

Windows 11, WinLibs GCC 15.2.0, Release, RTX 4080.

  • agc_driver_graphics (sparseHeapMirrorTests in BdaResources.cpp): a committed range smaller than the heap descriptor bound over it is mirrored, not copied (sparseBinds +1), the view is cut at the mirror, a GPU store is written back. Without the change it fails with a heap bound whole over a smaller registered range was copied.
  • Full suite: 756/756 pass (video-out tests skipped, as on main).
  • PPSA28416: [resources] guest upload ... copied drops from 4095 MiB (528 MiB really read) to 0.0 MiB per dispatch. Device time per dispatch during boot: 10.7 ms before (9500 dispatches, 101.8 s), 2.6 ms after (3600 dispatches, 9.5 s). The title then stops on garbage indirect dispatch arguments, which is a separate problem.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: fix(agc): copy a sparse descriptor heap only up to its last backed byte #2992 (its commit is included)
  • AI-assisted: yes

@Dextroid17 Dextroid17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two are one line of work on one title, and the second is only safe because of the first — so they want to land in order: #2992 cuts what a copied sparse region costs to its last backed byte, and #3167 removes the per-dispatch copy of a bound heap altogether.

The measurement chain is complete, which is unusual. PPSA28416 binds a 4 GiB GPU heap with 528 MiB committed; before the cut, every dispatch that bound it allocated the whole 4 GiB and failed with vkAllocateMemory … -2; after it, the copy was still 528 MiB per dispatch at ~90 ms, thousands of times during boot, which is the 0.1 FPS the title showed. Stating the intermediate step as well as the final one is what makes the second change's value checkable rather than asserted.

The mirror design is the right answer where a copy is not. Binding a sparse descriptor heap to a persistent mirror of the registered range that actually backs it — looked up for the part that is needed rather than the whole reservation, no part of which any registered range covers — removes the copy instead of shortening it, and committing pages on demand keeps the mirror proportional to what is really used. Refreshing a fully-committed plain mirror from the write watch, the way an address-based build does, is what keeps it honest, and servesView bounding a view start against each mirror's extent is the check that would otherwise be missing.

One thing worth naming in the descriptions: #2992 is currently conflicting, and this change's cut depends on it. So the sequence is a rebase of the first and then this one — and with GuestBufferMemory.hpp touched by both, that order is not optional.

Written with AI assistance.

@github-actions github-actions Bot added the agc libSceAgc* label Oct 11, 2026
hypermoist added a commit to hypermoist/AnyPS5 that referenced this pull request Oct 11, 2026
…dispatch (open PR boykopovar#3167)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silverash-i8

Copy link
Copy Markdown
Contributor

Windows run, since the PR has no Intel or Windows results yet: Windows 11, WinLibs GCC 15.2.0 r7, Release, merged onto main d31b57a in one build with #3217, #3202, #3167, #3137, #2863, #2849 and #2847, on an RTX 5060 Laptop and the Arrow Lake iGPU (ANYPS5_GPU=Intel).

  • agc_driver_bda_device and agc_driver_bda_span_reads (BdaResources.cpp) pass on both GPUs.

Full ctest of that build on both GPUs has no failure that isn't also on plain main here (guest_filesystem needs the symlink privilege, #3169; the Intel ones are listed on #2114 and #3207).

AI-assisted (Claude Code); I ran this on my machine.

@Mouhand-Kaddo

Copy link
Copy Markdown
Contributor

Additional platform validation of standalone PR head 2a430701e4a29733769ecf922f1604ee34a6f178 (2026-10-11):

Linux: passed — Arch Linux in WSL2 (kernel 6.6.87.2), GCC 16.2.1 20260810; CMake 4.3.1 / Ninja; Release, BUILD_TESTING=ON.

1 checks, 0 failures, 0 CTest skips. Built the selected targets and dependencies; this is focused validation, not a full build or full-suite run.

Checks: agc_driver_graphics.

Command after building: ctest --test-dir <build> --verbose --no-tests=error --timeout 30 -R "^agc_driver_graphics$".

This scope checks CPU-side behavior; no GPU execution result is claimed.

Internal limitation: heap-mirror subcases returned early because the guest arena was not write-watched in WSL. Zero CTest skips does not include that early return.

No title/gameplay validation. These results cover the standalone head, not a merge with current main.

@adriadam10
adriadam10 force-pushed the perf/agc-sparse-heap-mirror branch from 2a43070 to 38012ec Compare October 11, 2026 08:02
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main, dropping the first commit (the original #2992 commit): #2992 landed in a different form, so that commit conflicted and is no longer needed. The two commits of this PR are untouched.

Checked on the rebased tree: scoped build + ctest of libSceAgcDriver and neighbours; agc_driver_graphics, agc_driver_bda_device and agc_driver_bda_span_reads pass. sparseHeapMirrorTests is _WIN32-only like the existing heap mirror tests, so it doesn't run on Linux; Windows runs by @Silverash-i8 (RTX 5060 + Arrow Lake iGPU) and the author's own cover it. Merging once CI is green.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 11, 2026
@adriadam10
adriadam10 merged commit 6170ce8 into boykopovar:main Oct 11, 2026
14 checks passed
CnRJay added a commit to CnRJay/AnyPS5 that referenced this pull request Oct 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants