Skip to content

fix(agc): keep Windows host imports watched while late driver marks settle - #1847

Closed
JAB-dev wants to merge 2 commits into
boykopovar:mainfrom
JAB-dev:fix/agc-windows-import-settle
Closed

JAB-dev wants to merge 2 commits into
boykopovar:mainfrom
JAB-dev:fix/agc-windows-import-settle

Conversation

@JAB-dev

@JAB-dev JAB-dev commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

#1441 dropped PPSA22520 (Animal Well) on Windows from about 59 to 7.5 FPS. This restores the frame rate and keeps #1434 fixed.

Since #1441, Windows compares every host import instead of write-watching it, because the NVIDIA driver marks imported pages written again about 1.2-2 ms after the import window, and those marks dropped GPU results (#1434). The comparisons make every resource revalidation over an import a full walk. In PPSA22520 (Animal Well) that is 6.8 s of every 10 s and the frame rate falls from about 59 to 7.5 FPS.

Imports stay watched again, and the late marks are told apart from CPU stores by their bytes:

  • GuestMemory::ImportWatched keeps a copy of the import's bytes for a settle period after the import (1 s, APS5_IMPORT_SETTLE_MS). The late marks arrive about 2 ms after the import; the period only needs to outlast them, and a longer one only keeps the copy longer.
  • A page marked in that period whose bytes still equal the copy is stamped ImportWindow (maybe written), as the import window's own marks already are. Write-back keeps the GPU results over such a block.
  • A page whose bytes changed is a CPU store, stamped as before, and the copy takes its new bytes. A unit shadow over it is dropped as stale.
  • The driver's own stores into a settling import (StoreOwnBytes and the other storeOwn paths) refresh the copy, so a late mark over GPU results it wrote back still reads maybe written.
  • One walk of the import at the next collect after the period applies the same rule to the marks left, then the copy is dropped.
  • An import is compared as before when its copy would exceed 256 MiB, or 512 MiB together with the copies already kept, and when APS5_IMPORT_SETTLE_MS=0. Its import entry records that, so a refresh keeps it compared. Unwatching a range or giving it fresh private backing drops its copy.
  • APS5_WRITE_WATCH_IMPORTS is now read on Windows too: unwatch compares every import, as fix(agc): compare Windows host imports after late driver writes #1441 does; watch keeps imports watched without settling, the behaviour before fix(agc): compare Windows host imports after late driver writes #1441.

TechnicalDebt records the settle period, that it assumes a driver finishes marking within it, that a CPU store of identical bytes in the period reads as maybe written, and that a GPU write straight into a settling import (MarkWritten) cannot refresh the copy, as its bytes land later, so a late mark over it reads as a CPU store, as without settling. Treating those pages as maybe written instead makes agc_driver_recorder_tests lose a CPU store made after a GPU write ("a CPU store into a unit with results pending was not seen by the refresh"). Settling an import on an event instead (a GPU read of it and its fence, as #1434 suggests) would remove the timing assumption, but it can't be checked on a driver that doesn't mark late, and the import sites hold the host import and tracker locks.

A settle period that only reclassified marks by time was not enough: agc_driver_recorder_tests then publishes a stale unit shadow over a real CPU store ("a stale unit was published"), because PublishShadow only drops BlockWritten units. The byte check is what keeps that test passing.

Tested

Windows 11, RTX 4070 SUPER (driver 610.88), WinLibs GCC 15.2.0 r7, Release.

  • AGC: on Windows (NVIDIA) a new host import's pages are marked written after ImportWatched's window, so write-backs drop GPU results #1434, deterministically: this driver does not show the late marking on its own (agc_driver_dcc_keys_first_write and agc_driver_srgb8_color_target pass 30 of 30 with plain watching). On Windows, the write-watched run of agc_driver_dcc_keys_first_write now rewrites every page of the imported block with its own bytes between the import and the write-back, as the driver does, and pins a long settle period so a slow runner cannot race it. With APS5_WRITE_WATCH_IMPORTS=watch it fails in every run with AGC: on Windows (NVIDIA) a new host import's pages are marked written after ImportWatched's window, so write-backs drop GPU results #1434's message (the first write: the write-back stored 0 written and 0 cleared bytes of 65536, expected 128 written); by default and with APS5_WRITE_WATCH_IMPORTS=unwatch it passes.

  • Frame rate, PPSA22520, APS5_PROFILE_DRAW=1, 10 s windows:

    [present] per 10 s [rescache] revalidate
    main 4b6ab6b 72-75 13016 calls, 6823 ms, 10725 full walks
    this PR 587-590 166612 calls, 303 ms, 0 full walks
    fe9beae (before fix(agc): compare Windows host imports after late driver writes #1441) 587-588 166756 calls, 299 ms, 0 full walks
  • agc_write_tracking: a new Windows case (with its settle period pinned to 100 ms) imports two blocks, then checks that a page rewritten with its own bytes in the settle period reads maybe written, that a changed page and a store next to the import read written, that the closing walk applies the same rule, that a mark after the period reads written, that an import watched without settling trusts its marks, that a late mark over the driver's own store reads maybe written, that a CPU store after a GPU write into a settling import reads written, and that unwatching a settling import leaves it unwatched.

  • agc_driver_recorder_tests (not registered in ctest) passes, including the unit-shadow stale publish case and the Windows import-watch default.

  • 8 mutations each fail a test: unchanged pages trusted as CPU stores, changed pages not trusted, no closing walk, imports not registered, the driver's stores not refreshing the copy, a settle period that never ends, Windows comparing by default, and Windows not settling by default.

  • Full ctest on this branch alone, on top of main: 479/479.

  • Linux and macOS: the change is Windows-only (#ifdef _WIN32), not built there.

Related: #1599 keeps GPU results over write-back blocks whose bytes the CPU left unchanged (images up to 1 MiB); it is compatible with this and does not restore watching. #1310 and #1361 speed up write tracking and import lookups on Linux.

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

…ettle

boykopovar#1441 compares every Windows host import because a driver can mark
imported pages written again shortly after the import window (1.2-2 ms on
the NVIDIA driver in boykopovar#1434), and those marks dropped GPU results. The
comparisons make PPSA22520 spend 6.8 s of every 10 s revalidating
resources: about 7.5 FPS instead of 59.

Imports stay watched. Each import keeps a copy of its bytes for a settle
period (1 s, APS5_IMPORT_SETTLE_MS): a page marked in that period whose
bytes still equal the copy is stamped maybe written, like the import
window's own marks, and a page whose bytes changed is a CPU store. One
walk of the import at the next collect after the period applies the same
rule to the rest. An import whose copy would exceed 256 MiB, or 512 MiB
with the copies already kept, is compared, as is every import under
APS5_IMPORT_SETTLE_MS=0 or APS5_WRITE_WATCH_IMPORTS=unwatch, which is now
read on Windows too; =watch keeps plain watching. Unwatching a range or
giving it fresh private backing drops its copy.

On Windows the DCC first-write test now rewrites the imported block with
its own bytes between the import and the write-back, so it reproduces
boykopovar#1434 on every driver: it fails in every run with plain watching.

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

TheAggie commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

A second title for this, with a different workload: ccleste (PPSA59322), a homebrew port that renders through RADV (Mesa's Vulkan driver on the PS5) and writes its frames through storage images. #1441 halved its frame rate here, and this PR restores it.

Windows 11 Pro (26200), RTX 5070 Ti (driver 610.88), WinLibs GCC 15.2.0 r7, Release, APS5_PROFILE_DRAW=1, 35 s runs on the title screen, 10 s windows. ccleste only renders on AnyPS5 with my open RADV PRs (#1732, #1733-#1736, #1819, #1825 and others), so both builds are main aa277b38 merged into the same local branch carrying them, with and without this PR:

FPS (title bar) [present] per 10 s GPU idle per present Graphics::Draw per draw [storage] uploads per 10 s
without this PR 15.5 152-154 21-22 ms 9672 us 3294 / 13994 MiB (3266 untracked)
with this PR 29.2 287 2.2 ms 193 us none
before #1441 (older base, for reference) 27.5 274-277 3.5-3.7 ms 185 us none

Without it, every draw re-uploads ccleste's storage images in full (reason untracked; [buffers] image mirrors reports 804 unwatched heaps), and draws hold the GPU mutex for about 10 ms each. With it, the uploads and unwatched heaps are gone and the log reads host imports stay watched; pages marked written while an import settles are compared, not trusted. The title screen renders the same in both.

To isolate the cause beforehand, I set only unwatchImports back to false in decideImportWatch on the slow build: 26.8 FPS. #1472 and #1474 made no difference.

I only measured the frame-rate side; I did not test the #1434 side here.

AI-assisted (Claude Code).

@fisherjoey

Copy link
Copy Markdown
Contributor

A possible gap in the byte check that I'd like to confirm with you. I found it by reading the code and can't run the Windows path here (Linux only), so it may not show up in practice.

Only Cpu walks refresh the settle copy. settlingKind returns early for StampKind::Driver (GuestMemory.cpp:1054), so after storeOwn writes GPU results into a settling import (Cpu walk, store, Driver walk at 1340-1342), the copy still has the pre-import bytes. #1434 says the late mark can come more than 150 ms after the import. If it lands after that store, the next collect compares the written-back bytes with the old copy, sees a difference and stamps the page Cpu. ChangedBlocks then reports the 64 KiB block as BlockWritten, and the next write-back over it skips the block (Texture.cpp:3423). That's the #1434 symptom again.

I ran a standalone copy of settlingKind: a late mark with unchanged bytes gives ImportWindow, and the same page after a storeOwn gives Cpu.

Minimal fix: let the Driver walk refresh the copy.

StampKind settlingKind(WriteTracker& tracker, std::uintptr_t page, StampKind kind) {
    if (kind == StampKind::ImportWindow) return kind;
    for (auto& entry : tracker.settling) {
        ...
        if (kind == StampKind::Driver) { std::memcpy(saved, current, WritePageBytes); return kind; }
        if (std::memcmp(saved, current, WritePageBytes) == 0) return StampKind::ImportWindow;
        ...

Could you run this on Windows? It's a test in the style of CheckImportSettle. If the gap is real it fails with the message below; if it passes, ignore the rest of this comment.

Require(ImportWatched(base, Block, [] { return true; }, true), "the import callback failed");
StoreOwnBytes(base + 8, 1, [&] { bytes[8] = 0x44; });
generations[0] = TrackerGeneration();
bytes[8] = bytes[8];
CollectWritesUncached(base, Block);
Require(state(base) == BlockMaybeWritten, "a late mark over the driver's own store read as a CPU store");

GPU writes that go straight into the import (MarkWritten, no walk) have the same problem. Refreshing the copy there would capture bytes before the GPU writes them, so I don't have a small fix for that case.

AI-assisted: yes

A store the driver makes into a settling import (storeOwn) left the copy
with the bytes from before it, so a late mark over the stored GPU results
compared as changed and read as a CPU store. The stored range now
refreshes the copy. A GPU write straight into the import (MarkWritten)
cannot, as its bytes land later; a late mark over it still reads as a CPU
store, as without settling, since treating it as maybe written loses CPU
stores made after the GPU write.

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

JAB-dev commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

@fisherjoey thanks, good catch, the storeOwn case was real. Without a fix your test fails here on Windows. 9a4b70f refreshes the copy for the stored range in storeOwn's stamping, so it also covers the store paths that don't walk, and your test is now part of CheckImportSettle.

For MarkWritten I left it as is: treating those pages as maybe written makes agc_driver_recorder_tests lose a CPU store made right after a GPU write, so it's noted in TechnicalDebt instead.

@popcoder565

popcoder565 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Ran this on the driver that shows #1434's late marking, as asked in #1434.

Setup: Windows 10 (19045), RTX 3060 Laptop GPU, NVIDIA 616.92, MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. This PR's head 9a4b70f4 merged onto main 6cc0503, plus #2093, which main currently needs to build. The earlier head ec961ed8 on main f145139 gave the same numbers.

agc_driver_dcc_keys_first_write agc_driver_srgb8_color_target
main (compares every import, #1441) 30/30 30/30
this PR, default 30/30 30/30
this PR, APS5_WRITE_WATCH_IMPORTS=watch 0/20 9/20
this PR, APS5_WRITE_WATCH_IMPORTS=unwatch 20/20 20/20
  • Late marking: this driver still marks imported pages late on its own. With plain watching, srgb8_color_target fails 11 of 20 runs without any rewrite from the test, for example a multiply blend into an 8_SRGB target beside a linear target: byte 1 inside the scissor is 7, expected 2 from 7. By default the settle period absorbs those marks.
  • dcc_keys_first_write with watch: it fails every run with the expected message (the write-back stored 0 written and 0 cleared bytes of 65536, expected 128 written).
  • Full ctest: 494/494, with the GPU tests on the RTX 3060.
  • agc_driver_recorder_tests: built explicitly, since it's EXCLUDE_FROM_ALL.
    • It passes 5/5 by default and passes with watch.
    • With unwatch it stops at its check of the Windows default (Windows host imports were compared by default). That's expected when the default is overridden.

No title was run.

🤖 Addressed by Claude Code

@Rohan03x

Rohan03x commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Tested on Windows with another title and GPU.

Setup: Windows 11 22621, Ryzen 7 5700X, RTX 3070, Dreaming Sarah (PPSA02929), GCC 15.2.0 UCRT Release. PR merged onto main f1451396; ctest 488/488.

Build FPS uploads / 10 s reason "untracked" refresh write-backs presents refreshed first
main f1451396 17.1 2716 / 21463 MiB 2710 / 21356 MiB 1449 / 11059 MiB 207 of 207
this PR 59.95 7 / 110.6 MiB none none 0 of 1782
#1994, APS5_WRITE_WATCH_IMPORTS=watch 59.97 none reported none none 0 of 1190

On main, APS5_PROFILE_GPU shows storage-upload 2200 ms and storage-writeback 1378 ms of about 3715 ms timed per 10 s; the two 3840x2160 display buffers are re-uploaded on every draw and present. Average recorded draw: 1019 µs on main, 19 µs with this PR.

Limits: runs were under a minute and stayed on the title screen and main menu, not gameplay. No crash and no visible difference in that time. agc_driver_recorder_tests is not registered in ctest, so it was not run.

AI-assisted: yes (build, log analysis and this write-up).

@JAB-dev

JAB-dev commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing this, #2096 fixed the same regression on main a different way (shared aliases keep their write tracking, direct imports compare against a texture snapshot). On main d70b899 Animal Well now runs at 587–588 presents/10 s with 0 full rescache walks, the same as with this PR, so there's nothing left for it to do.

Thanks @popcoder565 and @Rohan03x for testing on the 616.92 driver and Dreaming Sarah, and @fisherjoey for the review.

@JAB-dev JAB-dev closed this Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants