Skip to content

perf(agc): read a fast-cleared surface's guest bytes only when the import is compared - #2699

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
alfalfa-star:fix/agc-fast-clear-snapshot-watched
Oct 10, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
alfalfa-star:fix/agc-fast-clear-snapshot-watched

Conversation

@alfalfa-star

Copy link
Copy Markdown
Contributor

What

#2096 keeps a copy of a storage image's guest bytes so that Refresh can compare memory the write watch doesn't cover. Its host-import upload path takes that copy only when the range is unwatched (GuestMemory::Watched). The DCC fast-clear path, however, takes it on every upload: originalValid = false became an unconditional captureGuestBytes(nullptr).

Reading the whole surface goes through the flush hook. For a surface a unit shadow covers, every 64 KiB block records a publish and waits for the GPU (FlushForAccess → PublishShadowsOnly → SyncThrough). Stack from PPSA14632, sampled at those waits: Driver::dispatch → ShaderResources::Revalidate → cachedStorageTexture → StorageTexture::Refresh → upload → captureGuestBytes → ReadCommitted → FlushForAccess.

The clear path now resolves the surface's host import first, because creating it can unwatch the range (a compared host-pointer import does). It takes the copy only when the range is unwatched after that, like the import path. Write-watched memory, such as Linux direct memory behind a dma-buf import (which keeps write protection), skips the read. A range that becomes unwatched only later re-uploads instead of comparing, as it did before #2096.

agc_driver_dcc_keys_first_write now checks that a fast-cleared upload keeps the copy exactly when its range is unwatched at that point. In the default run, the test block's host-pointer import is compared, so it covers the case #2096 needs. With APS5_WRITE_WATCH_IMPORTS=watch the check fails without this change ("a fast-cleared upload of write-watched memory read its guest bytes"). That mode then stops at the existing "an unchanged pending image was uploaded again" check, on main as well, so it isn't a ctest variant.

Tested

Linux (Arch), RX 9070 XT, RADV, Release:

presents flush-hook recorded-store waits waited
main ~24 ~60,000, all "publish only" ~6.4 s
with this PR ~100 ~3,500 ~4.4 s

How I narrowed it down: on that setup, the merge right before #639 (2b29784) has about 230 such waits per 10 s, and #2347, the first later merge that boots the title, has about 61,900. Reverting #425, #1309's backing write, #1310, #1410/#1411 or #2575 on main didn't change the count. The stack above leads to this line.

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: none
  • AI-assisted: yes

🤖 Generated with Claude Code

…port is compared

boykopovar#2096 keeps a copy of a storage image's guest bytes so that Refresh can
compare memory the write watch does not cover. The host-import upload
path takes that copy only when the range is unwatched, but the DCC
fast-clear path took it on every upload. Reading the whole surface goes
through the flush hook, so each 64 KiB block of a surface that a unit
shadow covers recorded a publish and waited for the GPU.

The clear path now resolves the surface's host import first (the import
can unwatch the range, as a compared host-pointer import does) and takes
the copy only when the range is then unwatched, like the import path.
Write-watched memory, such as direct memory behind a dma-buf import,
skips it; a range that is unwatched later re-uploads instead of
comparing, as before boykopovar#2096.

agc_driver_dcc_keys_first_write checks that a fast-cleared upload keeps
the copy exactly when its range is unwatched at that point.

PPSA14632 (LEGO Horizon Adventures) on Linux, RX 9070 XT: the flush hook
waited ~60,000 times per 10 s for these publishes (6.4 s), now ~3,500,
and presents rise from ~24 to ~100 per 10 s.

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

Copy link
Copy Markdown
Contributor

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head 4f9ddffc, merged onto main 57f96fd in one build with 4 other new PRs.

  • This PR's test passes: agc_driver_dcc_keys_first_write.
  • Full ctest in that build: 711 of 711.

No title was run.

🤖 Addressed by Claude Code

@alfalfa-star

Copy link
Copy Markdown
Contributor Author

The one failing job, write-watch (macos-15-intel), is a 30 s timeout in write_watch_threads. This PR only changes libSceAgcDriver (Texture.cpp/.hpp and its test), which that job does not build, and the same job passed on the 10 other recent runs I checked. Could someone with rights rerun it?

@orbz22

orbz22 commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Tested on Windows with a second title, Hollow Knight: Silksong (PPSA12544).

Setup: Windows 11 Pro (26300), i7-11800H, RTX 3060 Laptop (driver 617.42), WinLibs GCC 15.2.0 r7, Release. main de5e541 + #846 (Silksong needs it for its plugins), with and without this PR merged on top. Each run used the same scripted input for 250 s (title menu, New Game, the opening, into Moss Grotto) with APS5_PROFILE_DRAW=1.

Guest memory reads ([guestmem] read, cumulative at about 180 s):

Build Calls MiB Time
main, run 1 2.03 M 68,665 9.7 s
main, run 2 2.03 M 68,191 9.8 s
main + #2791 (control) 2.03 M 68,728 9.9 s
main + #2792 (control) 2.03 M 68,353 9.6 s
main + this PR 1.11 M 11,342 2.0 s

Over the whole 250 s, reads went from 99,548 MiB in 14.7 s to 14,470 MiB in 3.3 s.

  • GPU waits: unchanged, about 13,100 to 13,900 syncs and 9 s by 180 s in every build. Almost all of them come from WaitForKeyWriters (DCC keys), not from this path.
  • Frame rate: unchanged. Here the game is paced at 30 or 60 FPS by vsync. At 180 s every build had presented 7,418 to 7,424 frames.
  • Picture: unchanged. Frames dumped every 600 presents (menu, profile select, opening, Moss Grotto) match the runs without the PR, apart from animation timing. No new log lines.

🤖 Generated with Claude Code

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the diff, the build and ctest of libSceAgcDriver on Linux (agc_driver_dcc_keys_first_write passes), and the Windows runs from @popcoder565 and @orbz22. The only red job is write-watch (macos-15-intel), a timeout in a job that doesn't build this code; I re-ran it. Merging once it is green.

@adriadam10
adriadam10 merged commit 926ad9e into boykopovar:main Oct 10, 2026
26 of 27 checks passed
milescoviello added a commit to milescoviello/AnyPS5 that referenced this pull request Oct 11, 2026
…ched memory

range is write-watched. Since boykopovar#2984, Refresh turns a BlockMaybeWritten
unit into BlockUnchanged only through compareUntracked, which needs that
copy. Where a host import leaves its range watched and making it stamps
the imported pages (the probe's writtenAtImport, 4 of 4 on NVIDIA's
Linux driver), the import window stamps the surface, so a pending
fast-cleared image counted as changed again and
agc_driver_dcc_keys_first_write failed on main (boykopovar#3179).

HostImports now keeps whether making each kind of import stamped the
probe's pages, and WatchedImportsStampPages reports whether an import
that leaves its range watched can stamp it. The clear path skips the
copy only when no such import can, as oneandonlydean proposed in boykopovar#3179.
Devices whose probe saw no pages written at the import, devices without
host imports and Windows (whose alias imports don't disarm the guest
views) keep boykopovar#2699's saving.
An unprobed watch (APS5_WRITE_WATCH_IMPORTS=watch) keeps the copy.

The fast-clear check in agc_driver_dcc_keys_first_write now expects the
copy exactly when the range is unwatched or such a stamp is possible.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adriadam10 pushed a commit that referenced this pull request Oct 11, 2026
…ched memory

range is write-watched. Since #2984, Refresh turns a BlockMaybeWritten
unit into BlockUnchanged only through compareUntracked, which needs that
copy. Where a host import leaves its range watched and making it stamps
the imported pages (the probe's writtenAtImport, 4 of 4 on NVIDIA's
Linux driver), the import window stamps the surface, so a pending
fast-cleared image counted as changed again and
agc_driver_dcc_keys_first_write failed on main (#3179).

HostImports now keeps whether making each kind of import stamped the
probe's pages, and WatchedImportsStampPages reports whether an import
that leaves its range watched can stamp it. The clear path skips the
copy only when no such import can, as oneandonlydean proposed in #3179.
Devices whose probe saw no pages written at the import, devices without
host imports and Windows (whose alias imports don't disarm the guest
views) keep #2699's saving.
An unprobed watch (APS5_WRITE_WATCH_IMPORTS=watch) keeps the copy.

The fast-clear check in agc_driver_dcc_keys_first_write now expects the
copy exactly when the range is unwatched or such a stamp is possible.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alfalfa-star
alfalfa-star deleted the fix/agc-fast-clear-snapshot-watched branch October 11, 2026 09:45
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.

4 participants