Repository navigation
perf(agc): compare Windows direct imports only after a write-watch mark - #4878
Conversation
|
Windows RTX5070Ti cross-test of #4878 afafb68 + #4875a643768d7, author-preserving on codex/first-light-import-hints-cross-test at9011fc1961421bdb46c59c4871d82435e1d648e3, parentb45e164b0. This is an older experimental #757/#758 title integration with additional tracking/rendering changes, not clean main. WMI NVIDIA driver32.0.16.1714; Release/toolsOFF/Vulkan required. 13 focused checks PASS9.68s: tracking, Windows backend, guest memory, full mocked graphics, DCC (including hinted import and uniform-key follow-up), pending snapshot, storage baseline/upload lifetime/upload failure/unit shadow/CPU snapshot, host import unmap/retirement. Production-only negatives, fixtures retained:
Only patched graphicsA5F3F87D81398AE98663A00C845736B28DAC11916AF679883DFCF8A2100EBA43 and libc09A93A80BE788D344ECCD9824B1F2D640A229B7DDEC2A04664EA819C83CF18CA staged; kernel unchanged. Warm007 comparison starting separately with normal asynchronous draw/cache/profile/guard settings. No title FPS/rendering gain claimed yet; prior b45 run's compare counter was only0.4s cumulatively. Linux/AMD not tested. AI-assisted(Codex); no title code/captures/dumps/assets uploaded. |
|
Rebase handoff — rebased onto current Conflicts resolved: main's Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: Head is |
afafb68 to
a367b7c
Compare
|
Thanks @Scramblrrrr for the RTX 5070 Ti cross-test and the negative controls, and @blawesom for the rebase handoff. I rebased #4875 and this PR onto Native Windows 11, RTX 5080 (616.92): AI-assisted: yes. |
a367b7c to
c09a6a8
Compare
|
No measurable change on AMD in two light 2D catalog titles. Setup: Windows 11 (26300), Radeon 860M (RDNA 3.5, AMD proprietary driver 24.30.58.08), WinLibs GCC 15.2.0, Release, 1920x1080. Base is
In every build the queue worker is busy only 35-37 % of the window, and |
|
Thanks for measuring. No change is the expected result for these two titles. This PR only changes storage images that sit over a Windows direct host import. Those imports leave the write watch (#1434, #1441), so such an image's bytes are compared at every collect epoch. In #4432 (GTA V on an RTX 4090) those compares took 7 to 8.6 s of every 10 s. Titles that fill the scanout from the CPU and wait on a fence do little or none of that work, so it's good to know nothing regressed on AMD there. Where the PR should show is a scene with large compute-written storage images in flexible memory or the guest heap, like the #4432 one. |
|
@nikowolf123 GTA V (PPSA04264) data for this PR, the title #4432 measured. Windows 11, RTX 4090, MinGW-w64 GCC 15.2.0, Release. Base: our GTA stack (main 37dd483 plus #757 and the other open PRs it needs). Same build with and without this PR's three commits, cherry-picked (44767a7, f6adb17, c09a6a8): only the debt line and one test hunk conflicted. Story prologue, 120 s after Story starts, FPS from the frame counter delta:
Rendering is correct (same scene, HUD and minimap), with no new error in the log. The compare is still the largest cost: about 4 s of every 10 s. The call count roughly doubled with the frame rate, so per frame it went from ~14k refreshes and ~1.2 s of compares to ~14k refreshes and ~0.35 s. AI-assisted (Claude Code). |
|
Thank you, that's the in-game measurement this PR was missing: on the #4432 scene the frame rate goes from 0.60 to 1.09 FPS and the whole-surface compare from about 7 s to 3.6-4.3 s per 10 s, with the same picture. I added it to the description, including your note that the compare is still the largest cost. |
|
#4875 is in, so please rebase this onto |
|
@oneandonlydean @nikowolf123 these are the GTA V numbers in the Performance report format: Scene: PPSA04264, Story prologue (inside the bank), Commits
The draw path is where the time goes away. That is the untracked compare of imported render targets that #4432 measured. How the captures were taken
Summary lines {"commit":"664e755132cb","dirty":false,"env":{"APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"240:60"},"gpu":"NVIDIA GeForce RTX 4090","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","resolution":"3840x2160","window":{"spec":"seconds 240:60","first_present":3877,"ms":60098.995},"presents":44,"frame_ms":{"p50":1371.978,"p90":1492.146,"p99":1518.717,"max":1518.717},"gpu_busy_ms_per_present":256.798,"worker_busy_pct":{"queue worker 0x0":99.888,"queue worker 0x38":56.706,"queue worker 0x20":84.776,"queue worker 0x30":22.117,"queue worker 0x28":70.304},"top_phases_ms_per_frame":{"Driver.Packet":4517.149,"Driver.Packet.memory_wait":2712.849,"Driver.Packet.dispatch":956.816,"Driver.Dispatch":943.692,"Driver.Packet.draw":834.360,"Driver.Draw":833.877,"Vulkan.Dispatch":780.215,"Vulkan.Dispatch.shader_resources":776.050},"trace_dropped":true,"trace":"aps5-trace-664e755132cb-50444.json.gz"}
{"commit":"17d7f5318ac5","dirty":false,"env":{"APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"240:60"},"gpu":"NVIDIA GeForce RTX 4090","driver":"NVIDIA 616.92","os":"Windows 10.0.26200","resolution":"3840x2160","window":{"spec":"seconds 240:60","first_present":3739,"ms":60508.366},"presents":70,"frame_ms":{"p50":856.370,"p90":989.576,"p99":1150.443,"max":1150.443},"gpu_busy_ms_per_present":190.537,"worker_busy_pct":{"queue worker 0x0":99.820,"queue worker 0x38":40.486,"queue worker 0x20":91.806,"queue worker 0x28":59.208,"queue worker 0x30":57.036},"top_phases_ms_per_frame":{"Driver.Packet":2976.443,"Driver.Packet.memory_wait":1782.620,"Driver.Packet.dispatch":663.483,"Driver.Dispatch":652.577,"Vulkan.Dispatch":600.397,"Vulkan.Dispatch.shader_resources":596.388,"Driver.Packet.draw":521.369,"Driver.Draw":520.883},"trace_dropped":true,"trace":"aps5-trace-17d7f5318ac5-56092.json.gz"}Traces:
AI-assisted (Claude Code). |
On Windows every direct host import leaves the write watch, and a storage image over it compares its whole surface with its guest-byte copy at every collect epoch. boykopovar#4432 measured 7-8.6 s of every 10 s in those compares on a GTA V scene, with 98% of the compared surfaces unchanged. Tracker blocks that leave the watch now keep their write-watch marks as a hint. Leaving the watch gives a block a fresh hint stamp, and UntrackedHint walks a range of such blocks, at most once per collect epoch and thread, stamping the blocks whose pages a CPU store marked; it returns the newest stamp of the range. compareUntracked puts that stamp in its memo key in place of the collect epoch, so an image is compared again after a mark in its range, a driver store, a mapping change or a recorded GPU write. The byte compare still decides whether anything changed: a mark the driver makes without a CPU store costs one compare and drops nothing. Nothing else reads the hint. Collects, Watched, UnchangedSince, ChangedBlocks and NewestStamp still treat those blocks as untracked, and Linux is unchanged. agc_driver_dcc_keys_first_write refreshes an imported storage image over 50 collect epochs with no store: it compared the image 50 times before and 0 times now. A CPU store is still uploaded on the next refresh, and a store of the bytes already there costs one compare and keeps the pending results. agc_write_tracking checks the hint itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three ways to break the untracked hint passed every test: - hinted() without its coverage check walks, and resets, the marks of blocks that fresh backing put back under the watch; - walkHints stamping the walk's first block instead of the marked page's block hides a store from an image over another block; - compareUntracked without NewestStamp in a hinted key misses a GPU write into an import, which sets no mark and is only recorded by MarkWritten. CheckUntrackedHint now walks the whole range after a store into its second block and reads each block's hint on its own. It also recommits fresh backing over the hinted range: the range must have no hint, and a CPU store must still reach its collect. RunHintedImport hides a store's mark and records it with MarkWritten, and the image must still be uploaded at the next epoch. Each of the three changes above now fails agc_write_tracking or agc_driver_dcc_keys_first_write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased onto main (only the debt entry conflicted) so it carries just its own two commits; merging once the build is green. |
c09a6a8 to
c99c899
Compare
What
On Windows every direct host import of private guest memory (flexible memory and the guest heap) leaves the write watch:
decideImportWatchsetsunwatchImportsandimportAllocationcallsGuestMemory::Unwatch. The reason is in #1434 and #1441: NVIDIA's driver can mark imported pages after the synchronous import window. A storage image over such a range then ends inStorageTexture::compareUntracked. Its memo is keyed onCollectEpoch, so the image compares its whole surface with its guest-byte copy at every new collect epoch. #4432 measured this on GTA V with an RTX 4090. The compares took 7-8.6 s of every 10 s, 40k memo misses came only from a new epoch, and 98% of the compared surfaces were unchanged.This change keeps the write-watch marks of those blocks, but only to decide whether to compare:
unwatchLockedgives each tracker block that newly leaves the watch a fresh hint stamp. Unwatching a block again does not move it.GuestMemory::UntrackedHint(address, bytes)(Windows only) walks the range with a resettingGetWriteWatch. It walks at most once per collect epoch and thread, with the same memo rule as collects. It stamps the blocks whose pages a store marked and returns the newest stamp of the range. It returns 0 when a block of the range is still watched, the range is outside the arena or overlaps a host region, or a page of it cannot be walked (uncommitted or no-access memory); the image is then compared at every epoch, as before.compareUntrackedputs that stamp in its memo key in place ofCollectEpoch.NewestStamp,ForgetSerialandNewestWriteNotestay in the key, so a driver store, a mapping change or a recorded GPU write still forces a compare. A recommitted range is stamped by the private-mapping observer, so it is compared again too.A GPU write into an import sets no mark (the probe below confirms it), so it is caught the way a watched range catches it: the driver records it with
MarkWritten, which movesNewestStamp, and with a pending-write note while it is in flight, which movesNewestWriteNote. Before this change the next epoch's compare would also have caught a GPU write that neither recorded; now a missingMarkWrittenwould show here as it already does in watched memory.The byte compare stays the arbiter. A mark only decides whether to compare, so a mark the driver makes without a CPU store, as in #1434, costs one compare and drops nothing. This keeps the rule from #1441 that bytes, not marks, decide what changed. Nothing else reads the hint. Collects,
Watched,UnchangedSince,ChangedBlocks,NewestStampandStoredOverstill treat these blocks as untracked, so snapshot reuse and unit shadows stay off there, as before. On LinuxUntrackedHintreturns 0 and nothing changes.This PR builds on #4875 (merged as 7a0c838), which sizes each
GetWriteWatchquery to the region it walks. Without it the hint walk over flexible memory, committed in 16 KiB allocations, costs about 7 times the compare it replaces.The design rests on one fact: every CPU store to an imported page sets its write-watch bit. I checked it on Windows 11 26200 with the RTX 5080 (driver 616.92), using a standalone probe that imports the way
bindImportdoes (outside the repository; I can attach its source). Other Windows builds and AMD or Intel drivers are untested:VirtualAlloc(MEM_WRITE_WATCH)memory and in placeholder arenas replaced in one piece, in 16 KiB pieces and in 16 KiB RWX pieces.lock xadd,movnti, SSE,rep movsb,rep stosb, page-straddling and other-core stores.Overlap with open PRs: I test-merged every open PR that touches
GuestMemory.cpp,Texture.cpp,WindowsMappings.hpporcore/libs/tests/GuestMemory.cpp. Of those that merge withmain, only #3874 conflicts with this branch, and only in the adjacent entries ofdocs/dev/debt/libSceAgcDriver.md:BlockMaybeWrittenand the written-stamp table. This change uses neither.unwatchLocked, which perf(agc): compare an import window's pages when the window closes #3874'sImportWatched(..., staysWatched = false)also calls.mainindocs/dev/TechnicalDebt.md.Tested
Windows 11 Pro 26200, i9-14900KF, RTX 5080 (NVIDIA 616.92), WinLibs GCC 15.2, Release with tests:
agc_driver_dcc_keys_first_write(Windows, needs a GPU with host imports). It refreshes an imported private storage image over 50 collect epochs with no store, andStorageTexture::UntrackedCompares()must not move. Onmainwith only the counter and the test added, it fails with "50 collect epochs without a store compared the image 50 times"; on this branch it counts 0. A CPU store is still uploaded at the next refresh. A store of the bytes already there, which is what a late driver mark looks like, costs exactly one compare and keeps the image's pending GPU results. A store whose write-watch mark the test takes away and records withMarkWritteninstead, as a GPU write into an import is recorded, is still uploaded at the next epoch.CheckUntrackedHintinagc_write_tracking(no GPU needed, so the Windows CI job runs it when a full build is started). It checks that an unwatched range starts a hint and a watched or outside range has none, and that the range still reads as untracked. It also checks that the hint stays put across 8 epochs without a store, is walked once per epoch, moves on a store, on a same-byte store and on a store from another thread, and leavesNewestStampalone. A store found by a walk of the whole range moves the hint of its own block and not the other's. Fresh backing committed over the range puts it back under the watch: it then has no hint, and a CPU store still reaches its collect.GuestMemory.cpporTexture.cppwith the tests unchanged:unwatchLockedfails with "memory that left the watch has no untracked hint".agc_write_trackingwith "a CPU store into a hinted range did not move its hint", and failsagc_driver_dcc_keys_first_writewith "a CPU store into the imported image was missed".hinted()without its coverage check, which would reset marks that a watched collect still needs, fails with "watched fresh backing has an untracked hint".compareUntrackedwithoutNewestStampin a hinted key failsagc_driver_dcc_keys_first_writewith "a recorded store without a write-watch mark was missed".agc_driver_dcc_keys_first_writeandagc_driver_srgb8_color_target, the AGC: on Windows (NVIDIA) a new host import's pages are marked written after ImportWatched's window, so write-backs drop GPU results #1434 guards, plusagc_write_trackingandguest_memory: 30 of 30 runs each on the previous base (ctest --repeat until-fail:30, the slowest 0.30, 0.29, 0.15 and 1.36 s), and 10 of 10 each on this head.ANYPS5_REQUIRE_VULKAN=1 ctest --timeout 120 --parallel 4) on this branch's head, stacked on perf(libc): size each write-watch query to the region it covers #4875 overmain9ddb59f: 1021 tests, 0 failed, 7 skipped. One skip is because the driver lacksVK_EXT_shader_stencil_export; the other six are video-out tests that need a display, and this session has none.tools/check_skips.py --job windowsandtools/check_test_registration.pypass.ImportWatched, and the new tests still count 0 compares there. The head of this branch, merged with perf(agc): compare an import window's pages when the window closes #3874 and separately with fix(agc): defer write tracking for fresh Windows shared imports #4535 (652cb0b), passes the write-tracking, import, storage and shared-memory tests: 13 and 17 tests, 0 failed.agc_write_tracking,agc_write_watch_coverage,agc_write_data_patch_tests,agc_unchanged_cpu_stamps, theagc_storage_*,agc_host_import_*andagc_shared_write*tests,agc_parked_import_unmap,agc_unit_shadow,agc_recorder_kept_bytes,agc_driver_dcc_keys_first_write,agc_driver_srgb8_color_target,guest_memory). The new checks are Windows-only, and on LinuxUntrackedHintreturns 0.In-game measurement by @tibzejoker in GTA V (PPSA04264), the title #4432 measured (comment): Windows 11, RTX 4090, MinGW-w64 GCC 15.2.0, Release, the same build with and without this PR's three commits, story prologue 120 s after Story starts:
Rendering was the same (scene, HUD and minimap) with no new error in the log. The compare is still the largest cost, about 4 s of every 10 s, so this PR halves it rather than removing it.
Performance: I have no PS5 and no game files, so there is no trace from the Performance report form. Instead, a benchmark (outside the repository; I can attach its source) refreshes one 3840x2160 RGBA8 storage image (33,423,360 bytes) over an imported arena range for 1000 collect epochs with no store, which is what
Refreshdoes for the #4432 surfaces between stores. Each run is a separate process, andmain43e3a70 and this branch ran alternately, 11 rounds, in one session (load average 17-21 on 32 threads). This branch was faster in every round.Median per epoch, microseconds:
An earlier 7-round run under heavier load (19-29) also ran this branch without the first commit. Its medians were 2,381 (
main), 1,606 (this branch) and 16,098 (without the first commit) for flexible memory, and 2,384, 257 and 203 for one allocation. In 2 of its 7 rounds this branch was slower thanmainon flexible memory (3,767 and 3,874 against 2,051 and 2,381); the quieter run above did not repeat that.In flexible memory the walk still makes two
VirtualQuerycalls (the check that the whole range can be reset, then the walk) and oneGetWriteWatchper 16 KiB allocation, so it saves about a third of the compare there. Without the first commit it is about 7 times slower than the compare. In one allocation the walk is a single query, so the first commit changes little there. The walk holds the write tracker's mutex and the arena's mapping mutex, as every collect does; the memcmp it replaces held neither. That is about 1.3 ms per 33 MiB of flexible memory and 0.18 ms per 33 MiB in one allocation, once per thread and collect epoch, which other queue workers and guest threads faulting on shared pages can wait for. The benchmark has one thread, so it does not show that cost. Which kind of memory the GTA V surfaces live in, and what that means for its frame time, I could not measure. @tibzejoker offered in #4432 to run a branch on that scene; a trace from there would show it.The Windows CI job runs on
mainand when a maintainer starts a full build, not on every pull request push. Its runner has no Vulkan driver, so the new check inagc_driver_dcc_keys_first_writeis skipped there, whileagc_write_trackingandguest_memoryrun. I can attach the native log from the RTX 5080.main2f969f1, which now contains fix(agc): keep CPU writes when a pending storage result is stored over untracked memory #5024: the saved guest bytes are valid per unit (originalHolds) instead of for the whole image. The compare memo this PR keys on the hint is reached only aftercompareUntrackedhas checked that every unit of the range holds saved bytes, andcaptureGuestBytesclears it whenever it saves new ones, so a unit fix(agc): keep CPU writes when a pending storage result is stored over untracked memory #5024 invalidates or saves again is compared. The only conflict was the debt file, where this PR's host-pointer entry is kept and the rest ismain's. Native fullctest: 1064 tests, 0 failed. The four AGC: on Windows (NVIDIA) a new host import's pages are marked written after ImportWatched's window, so write-backs drop GPU results #1434 and write-tracking guards and fix(agc): keep CPU writes when a pending storage result is stored over untracked memory #5024'sagc_driver_store_over_untracked: 10 of 10 runs each.maincac4e96 with perf(agc): compare an import window's pages when the window closes #3874: the conflicts were the newpendingSingleblock inDccKeysFirstWrite.cpp(both blocks kept) and the debt entries (theBlockMaybeWrittenentry perf(agc): compare an import window's pages when the window closes #3874 removed stays removed). Native fullctest: 1047 tests, 1 failure:patched_libraries, which failed the same way on unmodified cac4e96 (feat(libkernel_sys): implement renameat #4877; fixed by fix(libkernel_sys): reach rename through a _nid_no_patch export #5025).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code