Repository navigation
Vulkan: pool imports take the game pools across a chunk's end whole, and commit their pages in one call - #1267
jordanella wants to merge 1 commit into
Conversation
hckrman101
left a comment
There was a problem hiding this comment.
Nice work on this one. The hitch analysis is convincing and the code is easy to follow. Here's what I checked:
span_game_pools: the backwards scan fromupper_bound(e)with theg->lo + span <= ebreak is right even with overlapping host pools. Any pool that starts at or belowe - spanand crossesewould push the range past the cap anyway, so stopping there loses nothing. It always ends because each growth is strict and bounded by the span. The stale{lo, hi}initializer list gets handled by the outergrewloop.plan(true)→plan(false)retry:overlappedis rebuilt on each pass, and the 1 GB fallback still applies. A spanned range that's on hold (import_cooling) still falls through to the unspanned one, becauseimport_poolresets the flag.Pool pcomes back clean from every failure path (piecesis only assigned on success), so reusing it on the second pass is fine.- One-call commit: the guest window is a single
MEM_RESERVEinhos_main.c, so the call can't cross reservations. If it fails, Windows commits nothing and the old loop runs. WithHOS_LARGE_PAGESthe call may well fail, but the fallback is a single query over one committed region, so that's cheap too. - Tests: the fixture slices still line up (the commit sits after
p.lo = lo;, so the fixture never sees it). The new cases cover the Kokiri layout, chaining, overlap, the cap and the retry. CI is green.
A few things before I'd approve:
- The span isn't Windows-only. The PR text says nothing else changes on Linux, but
span_game_poolsruns inpool_lookupon every platform. Linux (dma-buf imports) and AMD on Windows both get the bigger 32-64 MB imports too, and neither has been measured. On Linux the one-call commit really is a no-op: thewin32_posix.cshim returns early for the guest window, so it never reaches the query loop. What does change there is the span. Bigger dma-bufs means more pages pinned, andhos_mem.cmemsets freed pages under an import instead of discarding them. On AMD, earlier notes in this file measured imports at 20-75 ms each because the driver locks the chunk's pages, so 4x larger imports there might not come out ahead. Could you either get a quick Linux/AMD run, or default the span off (RX_VK_IMPORT_SPAN_MB=0) on the setups you haven't measured? At minimum, please fix the description. - Smaller notes inline: the commit comment's claim about protection, its numbers not matching the PR text, and registering the new test in
run_all.py.
| // committed already is left alone: the call took milliseconds in the game's huge reservation) | ||
| { | ||
| // (pages the game has not touched yet may be only reserved: committed now, as the pool's memory is usable) | ||
| // (hookshot) In one call over the range: the guest window is one reservation, committing pages committed already |
There was a problem hiding this comment.
Small correction to the comment: I'm fairly sure VirtualAlloc(MEM_COMMIT) over pages that are already committed doesn't fail, but it does apply the new protection to them. So they don't stay exactly as they were. Today that only affects RX_NVN_WATCH_*/hos_watch_guest_writes: an import over a watched range silently disarms it until the next 2 ms re-arm. That's fine for a diagnostic, but it would be good to say so here so nobody later relies on VirtualProtect for something that isn't a diagnostic. Also, this comment says ~33 of ~70 ms and 1.3 ms, while the PR text says ~46 of ~95 ms and ~3 ms. Worth making them match.
| /// 0x1021ffff00+0x160 across 0x1022000000, 130-147 ms, the one hitch of a panning run in Kokiri. Up to this size an import | ||
| /// takes the chunks of the game pools across its ends too (RX_VK_IMPORT_SPAN_MB, default 64; 0: not) | ||
| uint64_t import_span() { | ||
| static const uint64_t span = (uint64_t)(getenv("RX_VK_IMPORT_SPAN_MB") ? std::max(0, atoi(getenv("RX_VK_IMPORT_SPAN_MB"))) : 64) << 20; |
There was a problem hiding this comment.
This is on for every platform and vendor, but it's only been measured on Windows + NVIDIA (see the review body). If you can't test Linux/AMD, a platform-dependent default would be the safe choice. Otherwise, a line in the comment that it applies everywhere would help.
| run across its ends, while it stays within the span, so that a binding across a chunk's end inside such a pool finds both | ||
| chunks in one import instead of importing them again as one on the replay thread. | ||
|
|
||
| python -I -B tests/test_import_span.py [--cxx clang++] |
There was a problem hiding this comment.
Nit: this isn't in ARGS in tests/run_all.py, so it runs with its default --cxx g++, and on a machine without g++ it fails instead of being skipped. Adding "tests/test_import_span.py": ["--cxx", "{cxx}"], to ARGS would make it pick whichever compiler is there, like the other fixture tests do.
…and commit their pages in one call A game pool across a 16 MB chunk's end had each chunk imported on its own, at the first binding in it, and a later binding across the end imported the two again as one, on the replay thread: OoT's 16 MB pool at 0x1021745000, bound at 0x1021ffff00+0x160 across 0x1022000000, 130-147 ms, the hitch of a panning run in Kokiri. span_game_pools grows an import's chunk-aligned range over the game pools across its ends, while it stays within 64 MB (RX_VK_IMPORT_SPAN_MB; 0: off), so those chunks come in one import. A spanned import that fails is made again without the span. import_pool commits the range with one VirtualAlloc instead of querying it a region at a time: the queries were ~46 ms of an import's ~95 ms in the game's 512 GB reservation, the call ~3 ms. The guest window is one reservation, committing committed pages leaves them as they are, and guest pages are read-write (only the RX_NVN_WATCH_* diagnostics protect some); where the call fails, the query loop as before. Ryzen 9 3900X, RTX 3080, Kokiri Forest panning, three rounds against main: no frame over 100 ms (main: 0, 4, 1), worst 77-88 ms (main 89-244); imports per session 45-47 (main 90-97), 1.7-3.1 s of them (main 4.2-5.2 s). fps the same. ~7% more host memory imported (~1520 against ~1410 MB live). tests/test_import_span.py: span_game_pools on its own; nx2/test_import_windows.py: its functions in the fixture, and pool_lookup's span and its retry. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two changes to how guest memory is imported into Vulkan, both aimed at the stalls a pool import puts on the replay thread.
1. A game pool across a chunk's end is imported in one piece. Imports are made in 16 MB chunks, at the first binding in each. A game pool that runs across a chunk's end therefore had its two chunks imported separately, at the first binding in each. The first binding that crosses the end then imported both chunks again as one pool, synchronously, on the replay thread. In Kokiri Forest this happens with a 16 MB pool at
0x1021745000, bound at0x1021ffff00+0x160across0x1022000000: 130–147 ms, the one big hitch of a panning run. Of the 4547 pools the game makes in two minutes, 88 cross a chunk's end.span_game_poolsnow grows an import's chunk-aligned range over any game pool that runs across its ends, so long as the range stays within 64 MB (RX_VK_IMPORT_SPAN_MB; 0 turns it off). Larger pools are chunked as before. If a spanned import fails, the import is made again without the span, so a pool that imported before still imports.2.
import_poolcommits the range with oneVirtualAlloccall instead of querying it one region at a time. In the game's 512 GB reservation the queries took ~46 ms of an import's ~95 ms, and the single call takes ~3 ms. This is safe because:RX_NVN_WATCH_*diagnostics protect some);hos_mem.candhos_svc.czero them instead of decommitting.Where the call fails, the old query loop runs as before.
Measured
Ryzen 9 3900X, RTX 3080, Windows 11, on main c94d80e. Kokiri Forest after loading a save, headless, input timed in game frames (
HOS_INPUT_FRAME_CLOCK=1), presents 3120–5520. Three rounds, each building main and then this branch, each run standing still and panning (RX_VK_SLOW_LOG=35). Something outside the game slowed both builds in round 2 (main panning 32.1 fps, this branch standing 34.1), so its fps numbers aren't comparable, but its hitches are included.Costs and limits
0x1034ef6000, made during the load, alone in its chunk. That import is still synchronous, but about half as long as before. Importing chunks when pools are made isn't a fix: the game's pools touch 419 chunks, against ~45 ever imported.... imported in X ms as lo..hi ...). Nothing in the repository parses it.Tests
tests/test_import_span.pychecksspan_game_poolson its own: the Kokiri layout from both sides, growth chained across several pools up to the cap, a long pool behind a shorter one (the host's pools can overlap), pools only touching an end, the cap, and 0 for off.nx2/test_import_windows.pynow has the new functions in its fixture. It also exercises the span throughpool_lookup: a pool across a chunk's end imported as both chunks, and a spanned import over the device limit made again without the span.Mutation-checked: removing the span, the retry, the cap, the overlap handling or the chunk alignment each fails a test.
The branch is rebased onto main 96fece9 (v2.0.11 merged). Built there, a panning run had no frame over 100 ms, with 50 imports and 1.7 s spent in them.
python -I -B tests/run_all.py: the same 14 failures as main c94d80e on this machine (missing tools, and an older g++ that rejectsvolatilecompound assignment). Everything else passes, including the new test.🤖 Generated with Claude Code