Skip to content

perf(agc): keep a fast clear's guest bytes while they match unwatched memory - #6108

Open
Gashrod1 wants to merge 1 commit into
boykopovar:mainfrom
Gashrod1:fix/agc-clear-keeps-snapshot
Open

Gashrod1 wants to merge 1 commit into
boykopovar:mainfrom
Gashrod1:fix/agc-clear-keeps-snapshot

Conversation

@Gashrod1

@Gashrod1 Gashrod1 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What

A fast clear of a storage image over unwatched memory (upload, IsDccClear(uploadedKeys)) calls captureGuestBytes(nullptr) so that a later Refresh can compare the texels. It did so on every clear, even when the bytes it already kept still matched the memory, and captureGuestBytes resets comparedGuestBytes, so the next Refresh compared the whole surface again instead of answering from the memo.

The clear now checks the kept copy with the memoized compareUntracked first. It copies again only when a block changed, as before, and also, conservatively, when a unit kept at unwatch would read as written or a write-back's saved bytes are still pending (savedTicket). In those cases the old copy (which cancels the pending save) still runs. The state after the clear is the same as before: the same bytes in original, the same layerOriginal. The memo now survives.

GuestSnapshots() counts captureGuestBytes calls, like UntrackedCompares(), for the regression test RunClearKeepsSnapshot in DccKeysFirstWrite.cpp (unwatched block):

  1. a clear over unchanged memory takes no new copy (it fails on main with "copied its guest bytes 1 times");
  2. a clear after a CPU store into the texels copies once, and after a GPU write a Refresh does not upload again (the copy holds the CPU store).

Tested

GPU tests. Linux (Intel Alder Lake-P iGPU, Mesa 26.2.4), on ba3ffba: every agc_driver* and multisample test passes except agc_driver_multisample_fill_host_only, which fails the same way without this change (vkWaitSemaphoresKHR -4). Under ctest -j8, agc_driver_gated_dispatch_capture failed once after the last rebase and agc_driver_oversized_descriptor_sets once before it; both pass alone and in 3 repeats. Windows 11 (RTX 5060, 596.36): agc_driver_dcc_keys_first_write passes, the new case included (only that test was built there).

DAVE THE DIVER (PPSA19115), underwater scene, Windows 11, RTX 5060 (596.36), Ryzen 7 5800X. This title needs the MSAA stack listed in #5963 to render, so it does not run on main alone. The base is main 956269a plus that stack, measured with and without this change; that base predates 69702ef, so the measured change has no savedTicket check.

Captured as the Performance report form describes: APS5_TRACE_SECONDS=180:30 and no other APS5_* variable, log not redirected, warm caches (the scene ran, then the title was restarted), same underwater scene, same machine and session; only libs/ changed between the two captures.

Build presents in 30 s frame p50 p90 p99 GPU busy / present Driver.Draw / frame Graphics.Draw / frame
base 9a14a74bd 821 35.9 ms 40.2 ms 49.1 ms 9.4 ms 32.3 ms 24.9 ms
base + this change (cc690190b) 1062 27.8 ms 30.8 ms 36.7 ms 9.3 ms 24.2 ms 16.4 ms

tools/performance_compare.py: frame p50 -22.6 %, p99 -25.3 %, Driver.Draw.draw_and_resource_release -33.8 %; the only recorded configuration difference is window.first_present. The GPU time per present is unchanged; the queue worker is busy about 99 % of the window in both.

{"commit":"9a14a74bd275","dirty":false,"env":{"APS5_TRACE_SECONDS":"180:30"},"gpu":"NVIDIA GeForce RTX 5060","driver":"NVIDIA 596.36","os":"Windows 10.0.26200","resolution":"1920x1080","window":{"spec":"seconds 180:30","first_present":5225,"ms":30008.092},"presents":821,"frame_ms":{"p50":35.928,"p90":40.175,"p99":49.084,"max":57.341},"gpu_busy_ms_per_present":9.384,"worker_busy_pct":{"queue worker 0x0":99.045},"top_phases_ms_per_frame":{"Driver.Packet":35.298,"Driver.Packet.draw":32.502,"Driver.Draw":32.323,"Driver.Draw.draw_and_resource_release":25.214,"Graphics.Draw":24.932,"Graphics.Draw.draw_and_resource_release":21.428,"Driver.Draw.shader_compile_and_link":4.382,"Graphics.Draw.validate":3.325},"trace_dropped":true,"trace":"aps5-trace-9a14a74bd275-105296.json.gz"}
{"commit":"cc690190b21a","dirty":false,"env":{"APS5_TRACE_SECONDS":"180:30"},"gpu":"NVIDIA GeForce RTX 5060","driver":"NVIDIA 596.36","os":"Windows 10.0.26200","resolution":"1920x1080","window":{"spec":"seconds 180:30","first_present":6683,"ms":30016.633},"presents":1062,"frame_ms":{"p50":27.826,"p90":30.790,"p99":36.663,"max":44.595},"gpu_busy_ms_per_present":9.272,"worker_busy_pct":{"queue worker 0x0":98.772},"top_phases_ms_per_frame":{"Driver.Packet":26.853,"Driver.Packet.draw":24.344,"Driver.Draw":24.169,"Driver.Draw.draw_and_resource_release":16.682,"Graphics.Draw":16.420,"Graphics.Draw.draw_and_resource_release":12.829,"Driver.Draw.shader_compile_and_link":4.845,"Graphics.Draw.validate":3.409},"trace_dropped":true,"trace":"aps5-trace-cc690190b21a-88920.json.gz"}

Traces, attached in the comment below: aps5-trace-9a14a74bd275-105296.json.gz (base) and aps5-trace-cc690190b21a-88920.json.gz (change). Both report trace_dropped: the queue worker's default event buffer filled after about 4.6 s (base) and 3.5 s (change) of the window, so the event tracks of that thread cover only those first seconds. The summary lines come from the per-thread totals and cover the whole window. I kept the default buffer because the form asks for no other APS5_TRACE_* switch.

A local counter in that path (not part of this PR), per 10 s, in an earlier run:

  • before: ~790 clear snapshots of 8 MiB, every one byte-identical to the previous copy (~6.3 GB copied per 10 s); ~1,950 full compares, ~790 of them only because the snapshot had dropped the memo
  • after: 0 snapshots (~1,300 clears kept their copy), 270-500 full compares, none differing

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 under Silent stubs in their library's technical debt file
  • 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
  • Unblocks: none (speeds up PPSA19115, which renders with the MSAA stack of DAVE THE DIVER (PPSA19115) #5963)
  • GPUs tested: Intel Alder Lake-P iGPU, Mesa 26.2.4, Arch Linux (all GPU tests); NVIDIA GeForce RTX 5060, 596.36, Windows 11 (agc_driver_dcc_keys_first_write)
  • AI-assisted: yes

… memory

A fast clear of an unwatched surface copied its whole guest memory so
Refresh could compare it later, even when the copy it already kept still
matched; the copy also dropped the compare memo, so the next Refresh read
the whole surface again. In DAVE THE DIVER (PPSA19115) that was ~790 copies
of 8 MiB per 10 s, every one identical to the last.

The clear now checks the kept copy with the memoized compare first and
copies only when a block changed, a unit is kept at unwatch, or a
write-back's saved bytes are still pending (as before).
GuestSnapshots() counts the copies for the regression test.
@Gashrod1
Gashrod1 force-pushed the fix/agc-clear-keeps-snapshot branch from 0717f6f to a69c307 Compare October 14, 2026 22:33
@github-actions github-actions Bot added the agc libSceAgc* label Oct 14, 2026
@Gashrod1

Copy link
Copy Markdown
Contributor Author

Traces of the two captures in the description (base, then this change).

aps5-trace-9a14a74bd275-105296.json.gz
aps5-trace-cc690190b21a-88920.json.gz

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 1368 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 1395 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 4448 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 5609 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6187 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6188 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6192 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6197 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6202 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6209 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6280 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6481 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6539 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6946 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6947 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6960 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6971 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6973 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6977 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6987 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6989 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 6995 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 7002 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 7048 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 7256 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 7449 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
PR 7514 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 7604 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 7642 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 7800 core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp
PR 8484 core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp
core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked build and the AgcDriver ctest scope on the PR merged onto main: agc_driver_dcc_keys_first_write passes, and with the guestBytesStillHeld() condition removed the new case fails ("copied its guest bytes 1 times"). agc_driver_tessellation segfaults here with and without this change. Kept-copy conditions (savedTicket, keptAtUnwatch, memoized compare) fall back to the old capture, so the state after a clear is unchanged. Merging once the full build is green.

@github-actions github-actions Bot added perf Performance change unblocks title Names a title ID on the Unblocks line and removed unblocks title Names a title ID on the Unblocks line labels Oct 18, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* perf Performance change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants