Repository navigation
perf(agc): store write-back changes sixteen bytes at a time - #3137
Conversation
GuestMemory::WriteChanged stores only the bytes the GPU changed, so that CPU writes to the bytes around them survive. It found them a byte at a time inside every changed 256-byte block and stored each run with memcpy. Where changed and unchanged bytes interleave (the low bytes of floats), every run is a few bytes long, and that loop took 43% of the GPU submission thread in Disney Epic Mickey: Rebrushed's first level (0.9-1.0 ms per MiB). Each 16-byte chunk is now compared with SSE2. A chunk with no change is skipped, a chunk that changed entirely is stored whole, and a mixed one is stored with MASKMOVDQU, which writes only the bytes selected by the mask. An SFENCE after the loop orders those non-temporal stores before the stores that follow. Bytes the GPU did not change are still never written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0134RWCyiyydJBbfKz8g9Bwe
Dextroid17
left a comment
There was a problem hiding this comment.
The invariant is the interesting part here, and the shape of the change respects it. WriteChanged exists so GPU-side changes are written back without clobbering CPU writes to the bytes around them, which means a fast path is only safe if it stays byte-exact. Chunking into 16 bytes, skipping untouched chunks and storing fully-changed ones whole, keeps that; the middle case — part of a chunk changed — is where the invariant actually lives, so that is the path worth reading closely, including whatever happens to a trailing chunk shorter than sixteen bytes.
The numbers are in the description, which is not the common case and is worth saying. A microbenchmark over interleaved changes — the low two bytes of every four: 0.89 → 0.20 ms, one byte in sixty-four: 0.46 → 0.21 ms, best of thirty — plus a measurement in a running title, where the write-back falls from 43% to 17% of the submission thread and the frame rate moves from 4.60 to 6.92 FPS. That is the figure a perf change needs, and the only question left is whether the interleaved pattern is representative: the low-bytes-of-a-float case you chose is precisely the one that makes every run short, so it exercises the old cost rather than flattering the new code.
And resubmitting what the intake workflow closed, rebased onto current main with the reason stated, is the right way to handle that gate — it puts the work back in the queue without pretending the earlier PR never existed.
Written with AI assistance.
|
Windows run, since the PR has no Intel or Windows results yet: Windows 11, WinLibs GCC 15.2.0 r7, Release, merged onto
Full AI-assisted (Claude Code); I ran this on my machine. |
Replaces #2481, which the intake workflow closed automatically; same change, rebased onto current
main.What
GuestMemory::WriteChangedstores only the bytes the GPU changed in a mirrored buffer, so that CPU writes to the bytes around them survive. It found those bytes one at a time inside every changed 256-byte block and stored each run withmemcpy. When changed and unchanged bytes interleave (for example, only the low bytes of floats change), every run is a few bytes long, so each costs a byte loop and amemcpycall.Each 16-byte chunk is now compared with SSE2:
_mm_storeu_si128);MASKMOVDQU(_mm_maskmoveu_si128), which writes only the bytes its mask selects.An
SFENCEafter the loop orders the non-temporal masked stores before the stores that follow. A tail shorter than 16 bytes still goes byte by byte. Bytes the GPU did not change are never written, as before, and the reported changed range is the same.Tested
Linux (Ubuntu 24.04, GCC 13.3), i3-12100, RX 7600 (RADV).
New
testWriteChangedStoresExactlyTheChangedRuns(agc_driver_pm4_tests): 1037 bytes of random runs (changed, unchanged, and changed with some bytes left equal) over guest bytes that differ from both copies. Checks that exactly the changed bytes are stored and that the 13-byte tail is handled.Rebased onto current
main(7250918). Fullctest -j8: all pass exceptagc_driver_image_pipe_bank_xor, which failed once under parallel GPU load and passes when run alone; the 7 VideoOut display tests skip over SSH.tools/check_conventions.pyandtools/check_test_registration.pypass.Microbenchmark, 1 MiB, best of 30, one core:
Disney Epic Mickey: Rebrushed (PPSA15657, UE4) after Continue, with
APS5_NO_HOST_IMPORT=1so the game's compute buffers are mirrored. The stretch measured is the intro movie with the level's game logic and compute work running; the level's 3D view does not render yet (Disney Epic Mickey: Rebrushed (PPSA15657) on Linux #2255), so this is a write-back workload, not a level rendering benchmark.WriteChangedtook 43% of the GPU submission thread, the bottleneck thread here (the GPU was ~1.4% busy), at about 1.0 ms per MiB written back.Windows 11, i7-13700K, MinGW-w64 GCC 15.2.0 (WinLibs r7), Release:
agc_driver_pm4andagc_driver_pm4_failurepass, including the new test. SSE2 is part of the x86-64 baseline.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code