Repository navigation
perf(agc): compare an import window's pages when the window closes - #3874
Merged
oneandonlydean merged 2 commits intoOct 13, 2026
Merged
Conversation
This was referenced Oct 11, 2026
This was referenced Oct 11, 2026
Closed
oneandonlydean
force-pushed
the
up4/import-window-compare
branch
from
October 11, 2026 23:32
5989932 to
58eaf5a
Compare
10 tasks done
Merged
9 tasks done
This was referenced Oct 12, 2026
10 tasks done
9 tasks done
This was referenced Oct 12, 2026
A host import that leaves its range watched can resolve the write protection of the imported pages without storing anything, as NVIDIA's Linux driver does for every page. ImportWatched stamped those pages as a possible CPU store (BlockMaybeWritten), and only a storage image tracked by 64 KiB unit could clear the stamp, by comparing with a copy of the guest bytes it was uploaded from. Since boykopovar#3372 a fast-cleared upload therefore reads its whole surface from guest memory wherever such an import is possible, although nothing but that compare uses the copy. ImportWatched now takes whether the import leaves its range watched. If it does, the range is copied before the stores made so far are collected, and after the import each page whose write protection was resolved is compared with that copy. A page with other bytes is stamped as a CPU store. A page with the same bytes keeps only the CPU-side stamp the label rules read, so UnchangedSince holds for it for every consumer: storage images tracked by layer, snapshots and caches as well. If it does not (a host-pointer import on a device whose imports are compared, a direct import on Windows), nothing is copied or compared: the pages resolved in the window get the CPU-side stamp and the range leaves the watch before the tracker is unlocked. Where host-pointer and dma-buf imports differ in this, the dma-buf import that follows a failed host-pointer import is a window of its own, so each window knows its kind before it starts. BlockMaybeWritten, the written-stamp table, WatchedImportsStampPages and the unit compare in StorageTexture::Refresh are gone. The fast-clear upload skips the guest-byte copy whenever its range is watched, as before boykopovar#3372.
The second import-window case of agc_write_tracking makes one page unreadable with VirtualProtect or mprotect and expects an import that stays watched to throw before it copies its range. On Windows CheckRange answers from its table of committed arena pages, which is kept current by the registry's invalidator. The first case had checked the same allocation, so the table still said the page was readable, the check passed and the copy faulted on the page. ProtectPage now reports the change through GuestAllocationsInvalidate, as the registry does for a guest protection change and as tests/BdaResources.cpp does after VirtualProtect.
oneandonlydean
force-pushed
the
up4/import-window-compare
branch
from
October 12, 2026 19:42
58eaf5a to
1d3f642
Compare
This was referenced Oct 12, 2026
11 tasks done
9 tasks done
Contributor
|
A Windows run after the merge, since the description lists Windows as not compiled or run here. Setup: Windows 11 (26200), WinLibs GCC 15.2.0 r7, Release, CI configure flags, an RTX 4070 Laptop GPU and an Intel Raptor Lake-S iGPU,
Not reached: an import through a shared alias. Written with AI assistance. |
9 tasks done
10 tasks done
adriadam10
pushed a commit
that referenced
this pull request
Oct 14, 2026
An import that leaves its range watched copies the whole range first, so that each page the import resolves can be compared with its bytes when the window closes (#3874). Ghost of Tsushima (PPSA02225) maps and imports 4.9 GB of direct memory at start-up, and that copy is the largest allocation the title makes. Pages that are all zero are no longer copied. Each page of the range gets a slot, the index of its copy, or none for a page that was all zero, and such a page is compared with a zero page. Every page is compared with the same bytes as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A host import that leaves its range write-watched can resolve the write protection of the pages it imports without storing anything. NVIDIA's Linux driver does that for every imported page (
4 of 4 ... after the importin the probe line).GuestMemory::ImportWatchedstamped such pages as a possible CPU store (BlockMaybeWritten), and the only code that could clear the stamp wasStorageTexture::Refreshfor an image tracked by 64 KiB unit, by comparing with a copy of the guest bytes the image was uploaded from (#2984). So that this compare has its copy, a fast-cleared upload reads its whole surface from guest memory wherever such an import is possible (#3372). That is the read #2699 removed for watched memory (measured there on PPSA14632); on NVIDIA it is back. Nothing else uses the copy on watched memory, and a surface that is cleared every frame pays for it every frame: in a title we test with, 139 MiB of guest memory were read per frame in the menu, almost all of it here.The compare now happens once, where the stamp is made, and only for an import whose range stays watched.
ImportWatchedtakes that as a parameter (staysWatched). The caller knows it before the import is made:unwatchImportsis false).unwatchDmaBufImportsis false).For an import that stays watched,
ImportWatchedcopies the range, collects the stores made so far (stamped as CPU stores, as before), runs the import, and then compares each page whose write protection was resolved in the window with the copy:UnchangedSinceCollectedandStoredOverread, as before. Its block stamp doesn't move, soUnchangedSinceandChangedBlockshold for it.A store between the copy and the first collect is in that collect and stamped as a CPU store. A store after it is in the window and is compared with bytes copied before it. The collect re-arms a page before the page is compared, so a store after the compare is in the next collect.
For an import that leaves the watch, nothing is copied or compared. The stores made so far are collected as before, the pages resolved in the window get the CPU-side stamp as before, and the range is taken out of the watch before the tracker is unlocked (the caller did that after the call before). Two things differ from
mainthere:mainread them as possibly written.main. Where the kinds agree it is one window, as before.This replaces code from three merged PRs. Their cases stay covered:
Refresh): a pending image whose units an import window resolved without new bytes must not be uploaded again. The window no longer stamps those units, for any consumer, so the compare inRefreshhas nothing left to resolve.agc_driver_dcc_keys_first_write(an unchanged pending image was uploaded again) and its import-window part pass unchanged.WatchedImportsStampPages, the twoHostImportsflags, the fast-clear copy where a window can stamp): the copy existed only for fix(agc): ignore unchanged imported texture blocks #2984's compare. The test fix(agc): keep a fast-clear copy where an import window can stamp watched memory #3372 added stays as it is and passes: after a window that stored nothing new a pending image is not uploaded again, and a store of a new byte inside a window is seen.agc_driver_dcc_keys_first_writeis back toGuestSnapshotValid() == !stillWatched.Also removed:
BlockMaybeWrittenand the tracker's written-stamp table (it now always equals the block stamp). A store of new bytes inside a window now reads asBlockWrittenat a write-back as well, whereBlockMaybeWrittenwas stored over. The technical-debt entry for the import window (now indocs/dev/debt/libSceAgcDriver.md) is shorter: the store of identical bytes inside a window remains, the layer-tracked images are gone from it.Cost: a range the tracker covers is copied once before its import when the import leaves it watched, and only then. The copy is held while the import is made and the tracker mutex is held as before.
APS5_WRITE_WATCH_IMPORTS=watch. In the title above on NVIDIA that was 62 imports and 14.6 GiB over one run, the largest 2.2 GiB, measured on the previous revision of this branch, which copied the same ranges on this device; it was not measured again. The copy is now made in one pass (assigninstead ofresizeandmemcpy).APS5_WRITE_WATCH_IMPORTS=unwatch, and ranges the tracker doesn't cover.Copying only the pages a collect has armed is not done here.
ImportWatchedthrows for a range it tracks, is asked to keep watched and can't read; its caller checks that the range is accessible first. The check inside isCheckRange, so on Windows it answers from the table of committed arena pages that the registry's invalidator keeps current, as every otherCheckRangedoes.Tested
Linux (CachyOS, kernel 7.2.3, GCC 16.2.1, Release with CI's configure flags and
ANYPS5_ENABLE_SPIRV_TOOLS=ON), RTX 4090, NVIDIA 610.57.04 (host imports stay watched;4 after the import; dma-buf imports unavailable). Two commits, rebased ontomaind539606: 406c72d, and 1d3f642, which changes only the setup of the second new test case (two lines intests/WriteTracking.cpp). The rebase conflicted only indocs/dev/TechnicalDebt.md, whose entries moved todocs/dev/debt/;tools/debt_split.py resolvemoved this entry todocs/dev/debt/libSceAgcDriver.md. The code applied unchanged on top of #4385, #4291, #4108, #2802, #1357, #4221 and #4186, and nothing they added readsBlockMaybeWritten, the written-stamp table orWatchedImportsStampPages. An import parked for another device (#3976, #4291, #4385) is not touched here: its range keeps the watch state it had, as onmain, and a new import of that range is a window of its own. The head merges intomain682ef0c without conflicts. The table and the lines marked d539606 are from this head. The lines marked a11c8f7 and the profiled runs were measured on earlier bases with the same change and were not run again.maind539606 with only the test changesagc_write_tracking(two new cases, no device)a page an import window resolved without changing its bytes reads as writtenagc_driver_dcc_keys_first_writea fast-cleared upload of write-watched memory read its guest bytesagc_driver_recorder_tests, full run (not in ctest)(w) an import that stored nothing reads as a change to a cache taken before itagc_write_trackingcase: an import window that stays watched, in which one page gets the byte it already held and a page in another block gets a new byte. The first block must read unchanged and keep its CPU-side stamp, the second must read written, a third untouched, and a store after the window must be seen.guest memory is not readable; with the window stamping stores it fails witha store to a page that left the watch reads as a store to the watched pages of its block. The case changes the page's protection withVirtualProtectormprotectand reports the change withGuestAllocationsInvalidate, as the registry does for a guest protection change and astests/BdaResources.cppdoes.agc_write_tracking,agc_driver_dcc_keys_first_write,agc_unit_shadow,agc_host_import_unmap,agc_unchanged_cpu_stamps,agc_storage_baseline_settledandagc_driver_depth_fast_clear_submit5 times each, 35 of 35 pass; the full recorder run 3 of 3.ctest(--parallel 4,ANYPS5_REQUIRE_VULKAN=1), 979 tests, onmaind539606 and on this head: 971 pass, 7 skip (agc_driver_stencil_ref_export,video_out_flip_lifetime,_reopen,_controls,_overlay,_unopened,video_out_frame_dump_queue), and the same one fails on both,guest_kernel_fcntl(fcntl: descriptor 0 is neither a file, a directory, a pipe nor a character device; the run had no terminal on its standard input).python3 tools/check_conventions.py --base origin/mainandpython3 tools/check_test_registration.py buildpass.maind539606 was not run for comparison.ctest -R "agc|shader|write", 391 tests) on lavapipe (Mesa 26.2.3, LLVM 22.1.8): the same two fail onmaina11c8f7 and on 101db8e (agc_driver_emulated_color_compare,agc_driver_image_plane_address_on_line_implicit_lod), the same 28 skip. There host-pointer imports are compared and dma-buf imports stay watched (0 of 4 scratch pages written after a GPU read, 4 after a GPU write;dma-buf imports keep write protection: yes), so the import-window part ofagc_driver_dcc_keys_first_writeprints that it is not tested.SetImportWatchAPS5_PROFILE_DRAW=1, one run each. The base ismain2220eff with feat(agc): depth planes sampled as 2D arrays, cubes and integer views #425, fix(libSceAgcDriver): retire a depth surface once another writer covers its memory #454, perf(agc): keep unchanged guest layers of a depth plane copy #2490 and perf(agc): skip the depth plane copy when no slice changed #2491, which the title needs to get past its intro; the change is that revision on top. It could not be measured onmaind539606: with fix(libc): round mspace block sizes to the 32-byte alignment #4407 on top the title runs its intro and then terminates withstorage image access to depth/stencil surface ... is not implemented, before the title screen, so there is nothing to compare there.The target lookup per draw went from 37.7 to 8.5 us at the title screen. Guest memory read over the whole run: 323 GiB before, 26 GiB after. Two more runs of the same sequence with one more fix on top gave 22.9 / 20.75 / 22.3 and 22.7 / 21.0 / 22.2 fps, 0 kernel Xid lines, and frames that match the base by eye. The title screen was reached at 32.0 s, against 34.9 s on the base, with the copies included.
Not tested:
agc_write_trackingcase passed (the Windows walk's page compare), and the second crashed in the copy instead of throwing. The cause was the test: it made the page unreadable withVirtualProtectalone, the first case had already checked the same allocation, soCheckRangeanswered from its page table that the page was readable. The second commit (now 1d3f642) reports the change; it was not run on Windows here. A title doesn't reach this:importAllocationqueries every page of the range withVirtualQuerybefore it callsImportWatchedand makes no import over a page that is not committed,PAGE_NOACCESSorPAGE_GUARD, and a guest protection change goes through the registry. Still not run on Windows: an import through a shared alias (the alias conditionentry.alias != nullptr || !state.unwatchImports), and the second half of the second case, which collects a range that holds aPAGE_NOACCESSpage.maind539606 without the change.main(see above).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)