Repository navigation
perf(libkernel): find direct mappings that share physical memory through an index - #4321
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
|
The current conventions check has one actionable failure: “Based on current main; no other open PR implements the same functions” is unchecked. The description notes that #4216 changes the same loop, so please state the dependency/stacking plan and update that checklist response. The build and other checks reported here are passing. |
|
Ran Cyberpunk 2077 (PPSA04029) on bare AI-assisted: yes (Claude Code, Claude Fable 5.1), operating under my supervision; I am accountable for this comment and will respond to review. |
What
Fixes #3308 (its stop 3, the first stop on
mainsince #3428, #3465, #3455 and #3476).WatchMapping(DirectMemory.cpp, Linux) registers a new direct mapping with the write watch and then unregisters every range whose physical memory is also mapped somewhere else. To find those it walked all ofg_directMappingsfor each mapping added, so N mappings cost N(N+1)/2 visits. The report measured 23.2 s of 24.3 s there over 42,412sceKernelBatchMap2entries of 64 KiB, after which the title's watchdog stops it.Direct mappings are now also indexed by physical range:
g_physicalSpanscuts the physical offsets into disjoint spans, and each span lists the mappings that cover it.InsertMappingandRemoveMappingreplace the eleven places that added to or erased fromg_directMappings, so the index always matches it; adjacent spans with the same mappings are merged again on removal.WatchMappingreads the spans under the new mapping's physical range and gets exactly the mappings that overlap it, in address order as before, then runs the same loop body on them. The same ranges are registered and unregistered, in the same order; only the search changed. The cost of one map is now the number of mappings that really share its physical memory, whatever their sizes.The index is Linux-only, like
WatchMapping.ErasePhysMappingsandDirectMemoryViewsstill walk every mapping (once per release, not per map) and are not changed here.#4216 changes the same loop with an index by physical start that scans back by the longest mapping ever recorded. In the benchmark below it matches this change while every mapping is small, and with one live 4 GiB mapping it spends 38.8 s in
WatchMapping(4.4 s onmain, 0.04 s here).@snoozedDev could you confirm on PPSA04029? I don't have the title.
Tested
Linux 7.2.3, GCC 16.2.1, Release,
-DBUILD_TESTING=ON -DANYPS5_ENABLE_SPIRV_TOOLS=ON(the Linux job's flags without the ccache launcher), RTX 4090, NVIDIA 610.57.04.Benchmark without a device: one allocation of 2.6 GiB, then 42,412
sceKernelBatchMap2calls of oneOpMapDirectentry of 64 KiB each, no address given. A temporary counter and timer inWatchMapping(not in this pull request), basemain34a16b7:WatchMappingmainmain, 10,000 visits and 1.1 s here; on both, the same 37,412 of the first mappings stay watched and no alias is watched.GuestArenaAllocatechoosing each address (about 90 % of the samples inperf); it is outsideWatchMappingand not changed here. With fixed addresses the walks are inGuestAllocationsCovers,GuestAllocationsOverlapsandGuestAllocationsRequireAvailableinstead. The report's timers put everything outsideWatchMappingat about 1.1 s for the title.guest_memorygains two cases. One maps 16,384 separately allocated pages with one call (one direct mapping per page) and requires that it costs at most 48 times the same call over 1,024 pages, in thread CPU time, best of three. This is a relative timing bound, not a count: the tree has no operation counters for tests. Onmainthe ratio is 150 to 240 and the case fails (5 of 5 runs); with this change it is 17 to 19 (24 runs, 12 of them with all 32 cores busy). The other checks what the write watch covers after aliases are partly unmapped, replaced by a fixed mapping and mapped again; it passes onmaintoo.ctest -R "guest_|kernel|memory|write|direct": 325 of 325 pass onmain34a16b7; with this change 325 of 325 there and 326 of 326 after rebasing onto b42ef24. Fullctestwith this change on 34a16b7: 925 of 925.tools/check_conventions.py --base origin/mainandgit diff --checkpass.Not tested: Cyberpunk 2077 (PPSA04029) itself; Windows and macOS (the index and
WatchMappingare compiled on Linux only; the two helpers reduce to the previousemplaceanderaseelsewhere, not built here).Checklist
main; no other open PR implements the same functions (perf(libkernel): find guest allocations and direct mappings by address #4216 changes the same loop with a different index, see above; this PR does not build on it, and whichever lands second rebases)NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)