Repository navigation
perf(libc): share one snapshot of the registered guest ranges between leases until the registry changes - #4698
Conversation
… leases until the registry changes
|
Windows test of #4698 (MinGW GCC 15.2, Release, Windows 10, RTX 4080 SUPER), built on main
Not tested: no game run and no frame-time comparison. One thing I read in the code and did not measure: the snapshot is dropped in AI-assisted (Claude Code). |
|
Thanks for testing it. Your reading is right: the snapshot only lives between mutations, so a title that maps hundreds of times per second gains less there. It never builds more copies of the range list than before, so nothing gets slower. About |
What
GuestAllocationsAcquireandGuestAllocationsAcquireAllbuilt a new vector ofshared_ptrs to every registered range on every call, under the registry lock. The driver takes such a lease several times per draw and dispatch: the import lookup inGuestBufferMemory::UploadFinish,HostImportForwhen the imports are stale or the range has no import yet, and address-based builds. In Ready or Not (PPSA18605) this was the largest single cost on the GPU packet thread: 7 of 13 gdb samples of that thread were insideGuestAllocationsAcquire, six in its loop over the registry and one inmallocunder the heap lock.GuestAllocations::Leaseis now a read-only view of an immutable, shared vector of ranges (Lease::Ranges). It keeps the reads its users rely on (iteration,size,empty,operator[],front,back), plusclear, which releases the snapshot. The ranges it holds cannot be modified. Code that narrowed a lease in place now copies the ranges it keeps into aLease::Ranges.use_count()therefore counts the snapshots that hold it, not the leases: it is above 1 while any lease holds the range, and 1 when none is alive. The pin check inGuestAllocationsRequireUnpinnedandfailedImportRemapTestsrely only on that.GuestAllocationsRequireUnpinned, and when it ends. The re-lock case matters because a snapshot taken while the wait had released the lock must not outlive the mutation's changes. A lease therefore never shares ranges that a later mutation replaced.leaseSnapshotTests(run fromRunGuestAllocationTests) checks three things: two leases taken without a registry change share one snapshot; a lease taken after a mutation gets a new snapshot that includes the new range; and once the leases are gone, no snapshot keeps the range alive, souse_count()is back to 1, the registry's own reference.GuestBufferMemory.cppstill lists "the lease's ~1200 shared_ptrs" among the costs of a build. After this change a build shares them instead of copying them. The conventions check doesn't let this PR edit that comment.Related:
HostImportFor(when the imports are not stale) and theUploadFinishlookup a one-range lease instead.AcquireAllinsceKernelVirtualQuerywithGuestAllocationsLookup. Its current head (327e3b8) merges cleanly with this branch.Overlaps with open PRs:
{range}. With this change they would returnLease(std::make_shared<const Lease::Ranges>(...))instead.std::erase_ifin a test. It would copy the ranges out instead, as the lease-wait tests here do.tests/GuestAllocations.cpp, where perf(memory): restrict registry work to affected ranges #1324, perf(agc): cache page permissions within registered allocations #1330, feat(memory): track changes to mapped address ranges #1360, perf(agc): reuse import decisions across mapping changes #1361 and perf(agc): reuse immutable buffer copies and batch uploads #1364 add theirs, so those get a text conflict there.Tested
Windows 11 Pro 10.0.22631, AMD Ryzen 7 7800X3D, AMD Radeon RX 7900 XTX, WinLibs GCC 15.2.0, Release,
BUILD_TESTING=ON, onmainffa1984. The branch was rebased ontomaind539606 for this pull request and is not yet rebuilt or re-tested there:ctest: 951/951 passed, as onmain(8 tests skip there too).With the snapshot sharing taken out of
GuestAllocationsAcquire,leaseSnapshotTestsfails with "two leases taken without a registry change do not share one snapshot".tools/check_conventions.py --base origin/mainpasses.Ready or Not (PPSA18605), standing in the LSPD station lobby with
APS5_PROFILE_DRAW=1. One run without this change and one with it, on the same bring-up build otherwise. Each row is the range over that run's 10 s windows; FPS is presents per 10 s divided by 10:The "without" row leaves out a 70 s dip to 0.9–1.6 FPS. The bring-up build is
mainbab7b32 plus fix(libkernel): restore Windows select build for Demons Souls testing #4021, fix(recompiler): include implicit inputs in relative vector windows #4057, feat(agc): render layered draws into 3D color targets #1077, 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, feat(shader): give reads the masked-select pass keeps the whole pixel ancillary word #4031, fix(libc): alias non-contiguous shared guest views on Windows #2254, perf(libc): remember clean pages once per run in Windows write collects #3864, perf(agc): clear integer-format storage images with DCC code 0000 on the GPU #3865, perf(agc): handle the zero dword fill kernel as a buffer fill #4012, perf(agc): sort guest buffer regions through indices before merging them #4248, fix(agc): order direct reads after copied writes #1359 and local changes, all listed in Ready or Not (PPSA18605) #4184. It was not measured on this branch alone, because Ready or Not needs changes that are not inmainto reach that scene.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)