Repository navigation
perf(libc): read private write watches per allocation without a region query - #9098
Conversation
|
This pull request is labeled |
|
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.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
CnRJay
left a comment
There was a problem hiding this comment.
Good PR: the cost is isolated and measured (VirtualQuery 379.2 us of a 383.9 us collect inside one 449 MiB commit, because Windows walks the rest of the region), the direct path trusts only the proof Collect already relied on, and the safety argument lists every path that clears it. The large-commit result (2,221 ms to 28.5 ms for 5,765 spans) is clear.
- The other row of its own table is a regression: 16 KiB placeholder replacements go from 171 ms to 197 ms (+15%), because each span now makes one failed
GetWriteWatchcall and then the old queries. A title whose arena is committed in many small pieces pays this on every collect. Remember the failure: keep the proof from being used for that allocation (or that range) until the arena's commit state changes, so the failed call happens once, not once per span. - The bench isn't part of the PR, and it measures
Collectin isolation. The effect on a title is the thing that matters: Village's load with #8926's rewatch, load time or presents, main against this PR, from the same machine, withoutAPS5_PROFILE_*. 2.2 s of collect time over the load is real, but a title measurement shows whether it's on the critical path. - "The one unguarded case: a page decommitted in place inside a still-proven write-watch allocation, by code that bypasses
WindowsMappings. Nothing in the tree does that." Add a debug check that the direct call's result matches the old path's on a sample (or under a verify switch), so a later caller that does that is caught, rather than relying on nothing doing it.
|
@CnRJay, thank you for the review. We ran that title measurement on Resident Evil Village (PPSA01556):
It was slower in both runs, so we don't claim a load-time gain. That's two runs per arm, with a 5-minute spread between the two base runs, and GPU clocks weren't held. A traced minute of each load showed a ~16 s stall with two queue workers in We'll add both of your other points: remembering a failed |
|
@CnRJay, your first and third points are in:
Each has its red with only its check reverted (run 39651317856, run 39651193432), and both pass ×10 on Windows (run 39648160729). One correction and one finding:
Your second point, the Resident Evil Village (PPSA01556) load, follows after the profiled pair. |
windows_shared_map_cost maps 256, 16384 and 65536 pages of shared memory (4 MiB, 256 MiB and 1 GiB) through WindowsMappings::Map, then releases them, five times each, and prints the median map and release times, the time per page and the number of mapped views. It only measures and always passes, so a build of this commit and of a later one can be compared on the same machine.
Mapping shared guest memory page by page queried the address space three times for every 16 KiB page: twice to find the placeholder the page splits from, and once more to see whether the section page still had to be committed, which it then committed on its own. For a range of fresh direct memory that is seven system calls a page, and with shared imports attached every direct mapping takes this path on the guest thread that maps it, under the mapping lock. A mapping that goes page by page now commits its section range once through one window, and splits each page off the placeholder the previous page left, whose extent it already knows; only a page after a resident run or a demotion looks the placeholder up again. A page now costs a placeholder split, the view and its protection. Tested: built; windows_write_watch gains a case that maps 256 shared pages and counts the address-space queries the mapping makes, then stores to and reads back every page. With the per-page lookups it fails (775 queries for 256 pages) and with this change it passes. agc_driver_committed_imports, agc_write_watch_coverage, agc_private_collect, agc_write_guards, agc_driver_dcc_keys_first_write, agc_driver_residency_dirty_mirrors, agc_driver_residency_driver, guest_memory and the host import, window import and alias store tests pass.
…ections Committing the section range through one read-write window before the pages are mapped works for direct and shared memory, whose sections are committable and at least as long as the mapping. A file mapping passes the bytes its file still has: the window can run past the end of a short file, and a read-only file section refuses a read-write window, so every file mapping would fail. Only mappings that pass no section size commit the range up front; file mappings keep committing each view as before. guest_memory's regular file mapping case maps a read-only file and a file tail shorter than a page on Windows. Adapts fable-void's 027bff5 to main's file mappings.
Mapping shared direct memory while shared maps are imported placed one
view per 16 KiB page: a placeholder split, a map and a protect for every
page. A 1 GiB map made about 200,000 system calls, held the mapping
thread for about 0.4 s and took and dropped the mappings mutex on every
page, so threads taking write faults queued behind it.
Map now maps each run of shared pages that need a view as one view: one
placeholder split, one map at read-write-execute (the cap later guest
protections need) and one protect per run, and builds the per-page
bookkeeping under the mutex while yielding to waiting faults. The views
move the run's page references instead of copying them, so releasing the
run does not hold the mutex. Protections and write tracking change
subpages inside the view as before. A view is split only where a release
or a remap cuts it: reset unmaps the view, maps the two pieces again over
the same section offsets and restores every 4 KiB protection, so armed
write tracking and guest protections survive the split. An alias of a
page elsewhere and a per-page protection leave the view whole.
Tested: windows_write_watch gains a case that maps 1 GiB of shared
memory (65536 views before, one after), then protects one page, aliases
one page, arms write tracking on one page, releases one page and remaps
another: the run splits only at the released and remapped pages (2 and
4 views), every other page keeps its bytes and protection, and the armed
page still faults and is collected. It fails before ("a shared gibibyte
was mapped as 65536 views instead of one") and passes after.
agc_driver_committed_imports: the 1 GiB shared map and import takes
190-196 ms on the mapping thread instead of 418-427 ms, and the longest
write fault on another thread stays at 46-97 us. Built; the write watch,
committed import, guest memory, write guard, host and window import,
residency and mapping tests pass (58 tests).
A run view maps the section at read-write-execute over the whole run and commits the run through a read-write window. A file mapping passes the protection its file allows and the bytes its file still has, so a read-only file or a file that ends inside the run cannot be mapped that way. File mappings now take the per-page path they took before the run views: one view per page at the file's view protection, with the last page cut at the end of the file. Mappings that pass no section size, direct and shared memory, still map each run as one view. guest_memory's regular file mapping case maps a read-only file and a file tail shorter than a page on Windows. Adapts fable-void's 3d695fb to main's file mappings.
The VirtualQuery counter that windows_write_watch_tests reads is now compiled only into that test, through APS5_COUNT_MAPPING_QUERIES, so the runtime's WindowsMappings::query does no atomic update per query.
Collecting guest writes over private memory asked VirtualQuery for every region before reading its write watch, and a region ends at every protection change, so each 16 KiB piece cost two system calls and an allocation with several protections was read in several parts. The hot sweep reads every unguarded private page at every ordering point, so this cost was paid per hot page per dispatch. The mappings now keep the extent of every private allocation they make (added when a placeholder is replaced, erased on release) and collect one GetWriteWatch per allocation from that map. A write watch read cannot span two allocations, so pieces committed one by one still take one read each; the query per piece is gone and the reported pages are unchanged. Tested: built; the new agc_private_collect test (256 MiB of 16 KiB pieces with scattered writes returns the same pages as a reference read, at most one system call per piece; one allocation in five protection regions takes one call) fails before (32768 calls for 16384 pieces) and passes after (16384). The new hot-sweep case of agc_driver_residency_dispatch_work fails before (51324 calls over 25660 pieces with 256 hot runs, about 1500 us per dispatch) and passes after (25660 calls, about 1160 us). 47 residency, write-watch, guest-memory, address-space and shared-write tests pass.
…ehind the mappings Collecting private guest writes from the allocation map trusts the extent the mappings recorded and calls GetWriteWatch without a query. When memory in that extent was released by code that bypasses WindowsMappings, the call fails and Collect threw "collect private guest writes". Before the map, the query found the memory no longer committed private and Collect returned false, which its callers already handle. A failed GetWriteWatch now queries the region at the cursor. If it is no longer committed private memory, Collect returns false as before; otherwise the failure is still fatal. windows_write_watch releases a collected private allocation with VirtualFree behind the mappings, then collects it again: it must return false without throwing. Without the query it throws. Adapts fable-void's 2760ad8.
…VERIFY_DIRECT_COLLECT A private collect read from the allocation map makes no query, so it can't see memory that code bypassing WindowsMappings changed inside a recorded allocation. A page decommitted in place there is the case it can't catch: GetWriteWatch still succeeds over it, so the collect reports no writes where a query would have refused the range. With APS5_VERIFY_DIRECT_COLLECT set, each such collect also queries its span's regions, and Collect throws, naming the span and the switch, when GetWriteWatch succeeded over memory that is no longer committed private. ENVIRONMENT.md lists the switch. windows_write_watch_verify_direct (the same test binary with --verify-direct, the switch set) collects a recorded private allocation, checks its write is still reported, decommits one page in place and requires the next collect not to succeed. Without the check the collect succeeds. Adapts fable-void's 2760ad8.
The private collect counted its GetWriteWatch calls and pieces on every call, under the mappings mutex, for agc_private_collect to read. They are now compiled only where APS5_COUNT_MAPPING_QUERIES is set, as the address-space query counter is: agc_private_collect_tests sets it, and the runtime's Collect updates no counter. libc's GuestArenaCollectWork export, which nothing calls (the test reads its own WindowsMappings), is removed. Adapts fable-void's 2760ad8.
47e1497 to
d2c46d9
Compare
|
@CnRJay, the Resident Evil Village (PPSA01556) load you asked about is in, and it changed this PR's head.
The numbers and their sources are in the description. |
…(PPSA01556), with their dispatch bounds
|
@EagleBearerYS one finding from a review of head
On the overlap with #11152: the two only conflict in the list of checks in AI-assisted: yes (Claude Code). |
The target was added with EXCLUDE_FROM_ALL and nothing turned it back on, so `cmake --build build` never built it, and a full ctest reported agc_private_collect as Not Run. Declare it like the other test targets in that block. Found by thuanquang's review.
|
@thuanquang Thanks, good catch. Fixed in 1f691a3: |
What
On Windows,
WindowsMappings::Collectwalks a range of guest memory and collects the pages written since the last collect. For the guest arena's private allocations, each step askedVirtualQueryfor the region before reading its write watch.RegionSizeby walking the rest of the region, so a collect deep in a large commit pays for the whole remainder. In one 449 MiB private commit, a query took 379.2 µs and theGetWriteWatch2.2 µs.The change (this head replaces the earlier one; the first commit is fable-void's, under his name):
perf(libc): read private write watches per allocation without a query(fable-void, from his 2760ad8).GetWriteWatchper allocation from that map. A read can't span two allocations, so pieces committed one by one still take one read each, but the query per piece is gone.fix(libc): return false from a private collect over memory released behind the mappings.WindowsMappingsreleases memory inside a recorded extent,GetWriteWatchfails there.Collectreturnsfalseas before; otherwise the failure is still fatal.test(libc): check private collects against a region query under APS5_VERIFY_DIRECT_COLLECT.WindowsMappings.GetWriteWatchstill succeeds over it there.Collectthrows, naming the span, if it succeeded over memory that's no longer committed private.perf(libc): count private collect calls only in the collect test. The call counters compile only whereAPS5_COUNT_MAPPING_QUERIESis set, as the address-space query counter does. The runtime'sCollectupdates no counter.Linux and macOS are unchanged:
WindowsMappings.hppis Windows-only.Measured on GitHub's Windows runner (a bench of Resident Evil Village (PPSA01556)'s traced load, 5,765 retired imports over 2,077 refreshes, on fork
exp/rewatch-phases-fixed, not part of this PR; runs 39697304565 and 39697362432):mainmain), with noVirtualQueryResident Evil Village (PPSA01556) in-game: loading isn't faster.
APS5_LAZY_SHADER_PREPARE=1,APS5_NO_GPL=1,APS5_HOST_IMPORT_MIB=6400, andAPS5_TRACE_SUMMARY_EVERY=1for per-second summary lines, with no other profiling switch. GPU clocks weren't held.main1ff5c8a plus the perf(agc): import adjacent host ranges as aligned blocks and stop re-trying budget refusals #6457 fix (tree d56bdc5);Driver.Packet.dispatchover the load (per-run table and sources):q3b_check.py, its output): C at most 956 ms, A 1,191, and B 1,190 and 1,191.A second session, B A A B (2026-10-24) (runs, table and script):
Driver.Packet.dispatchper frame over the load (presents-weighted as above, with the window starting at Yes):Overlap:
main0ea0f25), whose shared-view change it composes with.core/libs/tests/WindowsWriteWatch.cppcame in through refactor(libc): map a run of shared guest pages as one view on Windows #9505's merge, and this PR's case runs beside them (see Tested).main: merged in 98fd280, through refactor(libc): map a run of shared guest pages as one view on Windows #9505's merge.main's perf(libc): carve fresh private guest pages out of their placeholder #10070 moved the placeholder replacement inCommitinto areplacehelper and added a path that carves whole granules out of one placeholder. This PR's bookkeeping moved with it:replacerecords each private allocation, so allocations on either path are recorded, and the new path erases the records over the range it replaces, asresetdoes on the other path.Tested
agc_private_collect(new, fable-void's): 256 MiB of 16 KiB pieces with scattered writes returns the same pages as a reference read, with at most one system call per piece. One allocation in five protection regions takes one call.windows_write_watch: a collected private allocation released withVirtualFreebehind the mappings must collectfalsewithout throwing. Without the second commit it throws.windows_write_watch_verify_direct: under the switch, a collect after a page is decommitted in place must not succeed. Without the third commit's check it succeeds.agc_private_collectwith the guarded counters,windows_shared_map_costand both write-watch tests;main7b32a84 (run 39890788684):agc_private_collect,windows_shared_map_cost,windows_write_watchandwindows_write_watch_verify_directpass ×10. The 8agc_import_*tests skip, because the runner has no Vulkan device;agc_import_watchpasses ×10;APS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners;main's code, the Windows tests don't build, because they use the new calls.main1e20650 (run), with the bookkeeping moved as described under Overlap:agc_private_collect,windows_shared_map_cost,windows_write_watch,windows_write_watch_verify_direct,guest_memory,guest_shm_openandguest_jit_shared_memorypass ×10: 8 run, 7 pass, 1 skip, 0 fail.agc_import_watchskips, as the runner has no Vulkan device;agc_import_watch,guest_memory,guest_shm_openandguest_jit_shared_memorypass ×10: 4 run, 4 pass, 0 skip, 0 fail;APS5_SKIP_TIMING_TESTS=1. All pass except libkernel'sguest_raise_in_system_callon Windows, which timed out after 30 s as the suite's 211th of 213. Run alone 20 times in a row, it andguest_raise_exceptionpass on this head (run) and onmain1e20650 (run), as refactor(libc): map a run of shared guest pages as one view on Windows #9505 records for its own head.main0ea0f25 (run). One conflict, the list of cases inWindowsWriteWatch.cpp'smain(), where both sides are kept: this PR'sCheckReleasedPrivateAllocationand fix(agc): keep a rewatched import's memory compared once it is shared #9159's four compared-shared cases. This PR's own added and removed lines are unchanged.agc_private_collect,windows_shared_map_cost,windows_write_watch,windows_write_watch_verify_direct, fix(agc): keep a rewatched import's memory compared once it is shared #9159'sagc_rewatched_direct_shared,agc_import_alias_sharedandagc_compared_shared_generations,guest_memory,guest_shm_open,guest_jit_shared_memoryandguest_raise_exceptionpass ×10: 12 run, 11 pass, 1 skip (agc_import_watch, no Vulkan device), 0 fail;agc_import_watch,guest_memory,guest_shm_open,guest_jit_shared_memoryandguest_raise_exceptionpass ×10: 5 run, 5 pass, 0 skip, 0 fail;APS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.check_conventions.py: passes. No comment lines are added.agc_private_collect_testsbuilt in a plain build (run), after thuanquang's review: the target was added withEXCLUDE_FROM_ALLand nothing turned it back on, so a plaincmake --build buildnever built it, and a full ctest would reportagc_private_collectas Not Run. The flag is dropped. On Windows, a plain build ofallbuilds it, and the test passes 10 runs in a row: 1 run, 1 pass, 0 skip, 0 fail. Linux: 0 run, as the test is registered on Windows only. Suites: libc and libkernel pass on Windows (170 and 221). The suites ran withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.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)core/libs/prx/libc/include/WindowsMappings.hpp): this PR first, then perf(libc): keep no per-page physical state for resident guest pages on Windows #11152 rebases onto it