Vulkan: pool imports take the game pools across a chunk's end whole, and commit their pages in one call #1267

Merged
jordanella merged 1 commit from import-span into main 2026-11-09 23:03:54 +00:00
jordanella commented 2026-11-09 19:54:17 +00:00 (Migrated from github.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 at 0x1021ffff00+0x160 across 0x1022000000: 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_pools now 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_pool commits the range with one VirtualAlloc call 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:

  • the guest window is a single reservation;
  • committing pages that are already committed leaves them as they are;
  • guest pages are read-write (only the RX_NVN_WATCH_* diagnostics protect some);
  • pages under an import stay committed, as hos_mem.c and hos_svc.c zero 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.

main this PR
panning: frames over 100 ms (3 runs) 0 / 4 / 1 0 / 0 / 0
panning: worst frame 89 / 244 / 156 ms 86 / 88 / 77 ms
panning: 0.1% low 13.1 / 4.6 / 8.3 fps 12.0 / 11.6 / 13.1 fps
imports per session 90–97 45–47
time in imports per session 4.2–5.2 s 1.7–3.1 s
an import, on average (commit part) 89–103 ms (44–50 ms) 45–73 ms (~3 ms)
fps, rounds 1 and 3 (standing / panning) 39.8, 38.9 / 37.0, 36.5 39.5, 39.4 / 36.7, 36.9

Costs and limits

  • More host memory imported: ~1520 MB live against ~1410 MB (+7%), because a span takes in whole neighbouring chunks.
  • Larger first imports: a session makes many more 32–160 MB imports and far fewer 16 MB ones. In four sessions each, the slowest single import was 152 ms against 182 ms on main, and the one-call commit shortens them all.
  • One hitch remains: a chunk whose first binding happens during play. In Kokiri that's a 4 KB pool at 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.
  • Tested only on Windows with NVIDIA. On Linux the one-call commit may fall back to the query loop, and nothing else changes there. Nothing new runs in the background (background imports are what slowed AMD before).
  • Log line: the import line now names the range imported (... imported in X ms as lo..hi ...). Nothing in the repository parses it.

Tests

  • tests/test_import_span.py checks span_game_pools on 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.py now has the new functions in its fixture. It also exercises the span through pool_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 rejects volatile compound assignment). Everything else passes, including the new test.

🤖 Generated with Claude Code

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 at `0x1021ffff00+0x160` across `0x1022000000`: 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_pools` now 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_pool` commits the range with one `VirtualAlloc` call** 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: - the guest window is a single reservation; - committing pages that are already committed leaves them as they are; - guest pages are read-write (only the `RX_NVN_WATCH_*` diagnostics protect some); - pages under an import stay committed, as `hos_mem.c` and `hos_svc.c` zero 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. | | main | this PR | |---|---|---| | panning: frames over 100 ms (3 runs) | 0 / 4 / 1 | **0 / 0 / 0** | | panning: worst frame | 89 / 244 / 156 ms | **86 / 88 / 77 ms** | | panning: 0.1% low | 13.1 / 4.6 / 8.3 fps | **12.0 / 11.6 / 13.1 fps** | | imports per session | 90–97 | **45–47** | | time in imports per session | 4.2–5.2 s | **1.7–3.1 s** | | an import, on average (commit part) | 89–103 ms (44–50 ms) | **45–73 ms (~3 ms)** | | fps, rounds 1 and 3 (standing / panning) | 39.8, 38.9 / 37.0, 36.5 | 39.5, 39.4 / 36.7, 36.9 | ### Costs and limits - **More host memory imported:** ~1520 MB live against ~1410 MB (+7%), because a span takes in whole neighbouring chunks. - **Larger first imports:** a session makes many more 32–160 MB imports and far fewer 16 MB ones. In four sessions each, the slowest single import was 152 ms against 182 ms on main, and the one-call commit shortens them all. - **One hitch remains:** a chunk whose first binding happens during play. In Kokiri that's a 4 KB pool at `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. - **Tested only on Windows with NVIDIA.** On Linux the one-call commit may fall back to the query loop, and nothing else changes there. Nothing new runs in the background (background imports are what slowed AMD before). - **Log line:** the import line now names the range imported (`... imported in X ms as lo..hi ...`). Nothing in the repository parses it. ### Tests - `tests/test_import_span.py` checks `span_game_pools` on 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.py` now has the new functions in its fixture. It also exercises the span through `pool_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 rejects `volatile` compound assignment). Everything else passes, including the new test. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hckrman101 (Migrated from github.com) reviewed 2026-11-09 20:04:30 +00:00
hckrman101 (Migrated from github.com) left a 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 from upper_bound(e) with the g->lo + span <= e break is right even with overlapping host pools. Any pool that starts at or below e - span and crosses e would 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 outer grew loop.
  • plan(true) → plan(false) retry: overlapped is 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, because import_pool resets the flag. Pool p comes back clean from every failure path (pieces is only assigned on success), so reusing it on the second pass is fine.
  • One-call commit: the guest window is a single MEM_RESERVE in hos_main.c, so the call can't cross reservations. If it fails, Windows commits nothing and the old loop runs. With HOS_LARGE_PAGES the 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:

  1. The span isn't Windows-only. The PR text says nothing else changes on Linux, but span_game_pools runs in pool_lookup on 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: the win32_posix.c shim 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, and hos_mem.c memsets 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.
  2. 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.
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 from `upper_bound(e)` with the `g->lo + span <= e` break is right even with overlapping host pools. Any pool that starts at or below `e - span` and crosses `e` would 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 outer `grew` loop. - `plan(true)` → `plan(false)` retry: `overlapped` is 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, because `import_pool` resets the flag. `Pool p` comes back clean from every failure path (`pieces` is only assigned on success), so reusing it on the second pass is fine. - One-call commit: the guest window is a single `MEM_RESERVE` in `hos_main.c`, so the call can't cross reservations. If it fails, Windows commits nothing and the old loop runs. With `HOS_LARGE_PAGES` the 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: 1. **The span isn't Windows-only.** The PR text says nothing else changes on Linux, but `span_game_pools` runs in `pool_lookup` on 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: the `win32_posix.c` shim 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, and `hos_mem.c` memsets 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. 2. 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`.
@ -354,0 +352,4 @@
// (hookshot) In one call over the range: the guest window is one reservation, committing pages committed already
// leaves them as they are, and the guest's pages are all read-write (only the RX_NVN_WATCH_* diagnostics protect
// some). The pages were queried one region at a time before, so as to commit only the reserved ones: the queries were
// ~33 ms of an import's ~70 ms in the game's huge reservation (on the replay thread, at a pool's first binding), the
hckrman101 (Migrated from github.com) commented 2026-11-09 20:04:30 +00:00

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.

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.
@ -1759,2 +1795,4 @@
}
Pool* pool_lookup(uint64_t guest, uint64_t size) {
if (guest_pools.empty()) return nullptr;
hckrman101 (Migrated from github.com) commented 2026-11-09 20:04:30 +00:00

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.

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.
@ -0,0 +2,4 @@
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++]
hckrman101 (Migrated from github.com) commented 2026-11-09 20:04:30 +00:00

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.

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.
lucasmazzoni45-del commented 2026-11-09 21:52:28 +00:00 (Migrated from github.com)

AMD / Windows data for this PR, since the numbers above are NVIDIA: RX 7800 XT 16 GB, Ryzen 7 5800X3D, Windows 11, 2560x1440, frame generation x2, v2.0.11 with this PR merged on top (it merged cleanly; I also had #1150 and a few AMD changes of my own in the same build, so this is not a clean single-PR A/B). Normal play with a save, not a scripted route, so the two sessions are not the same scenes. Both started within the same hour on the same machine, and the pool imports are counted by the log's own (N imports, M ms in all):

up to present ~9700 v2.0.11 without the PR with the PR
pool imports 106 71
time in imports (log's running total) 1654 ms 952 ms
imports of 30 ms or more 29 2
longest single import 100 ms 46 ms
mean import 19.5 ms 14.7 ms

A second session without it (21:45) is close to the first: 121 imports, 2081 ms, 30 imports of 30 ms or more, longest 90 ms. So on this card too the PR cuts the number of imports and nearly all of the 30-100 ms ones.

Frame times are less clear: frames over 40 ms went from 100 in 13615 presents to 50 in 12316, but intervals over 22 ms went from 4.9% to 11.2%. The session with the PR was the first one after a DLL change, so the pipeline cache was cold (compiles in the background), which probably explains the second number; I would not read it as a regression, nor as a result. No device lost and no new warning in the log.

I have not measured the 7% extra host memory the description mentions. RX_VK_IMPORT_SPAN_MB=0 turns it off if anyone wants to compare on the same save.

AMD / Windows data for this PR, since the numbers above are NVIDIA: RX 7800 XT 16 GB, Ryzen 7 5800X3D, Windows 11, 2560x1440, frame generation x2, v2.0.11 with this PR merged on top (it merged cleanly; I also had #1150 and a few AMD changes of my own in the same build, so this is not a clean single-PR A/B). Normal play with a save, not a scripted route, so the two sessions are not the same scenes. Both started within the same hour on the same machine, and the pool imports are counted by the log's own `(N imports, M ms in all)`: | up to present ~9700 | v2.0.11 without the PR | with the PR | |---|---|---| | pool imports | 106 | **71** | | time in imports (log's running total) | 1654 ms | **952 ms** | | imports of 30 ms or more | 29 | **2** | | longest single import | 100 ms | **46 ms** | | mean import | 19.5 ms | **14.7 ms** | A second session without it (21:45) is close to the first: 121 imports, 2081 ms, 30 imports of 30 ms or more, longest 90 ms. So on this card too the PR cuts the number of imports and nearly all of the 30-100 ms ones. Frame times are less clear: frames over 40 ms went from 100 in 13615 presents to 50 in 12316, but intervals over 22 ms went from 4.9% to 11.2%. The session with the PR was the first one after a DLL change, so the pipeline cache was cold (compiles in the background), which probably explains the second number; I would not read it as a regression, nor as a result. No device lost and no new warning in the log. I have not measured the 7% extra host memory the description mentions. `RX_VK_IMPORT_SPAN_MB=0` turns it off if anyone wants to compare on the same save.
hckrman101 (Migrated from github.com) approved these changes 2026-11-09 22:42:42 +00:00
hckrman101 (Migrated from github.com) left a comment

Re-reviewing after lucasmazzoni45-del's AMD data. No new commits since my first review, so the three small points from it are all still open, but none of them blocks this.

On the Linux/AMD question: the RX 7800 XT run on Windows covers the vendor side. Imports went 106 to 71, 29 imports over 30 ms down to 2, longest 100 to 46 ms, with no device loss and no new warnings. That is a good sign for AMD. It is not Linux data. Linux is still untested, but the span logic is platform-neutral bookkeeping. If a spanned import fails, it is retried without the span, and the PR description already says it was only tested on Windows. I'm fine with it on by default given that fallback. If someone on Linux/RADV can run it once with RX_VK_IMPORT_SPAN_MB=0 and once without, that would close it.

Still open, all nits (see my earlier inline comments):

  • The comment on the one-call commit says already-committed pages are "left as they are". VirtualAlloc(MEM_COMMIT) re-applies the protection, so an import silently disarms an RX_NVN_WATCH_* range until the next re-arm. Fine for a diagnostic, but the comment and the PR text should say so.
  • The numbers in that comment (~33 of ~70 ms, 1.3 ms) don't match the PR text (~46 of ~95 ms, ~3 ms).
  • tests/test_import_span.py isn't in ARGS in tests/run_all.py, so it fails instead of being skipped on a machine without g++.

CI is green. Good to merge from my side.

Re-reviewing after lucasmazzoni45-del's AMD data. No new commits since my first review, so the three small points from it are all still open, but none of them blocks this. On the Linux/AMD question: the RX 7800 XT run on Windows covers the vendor side. Imports went 106 to 71, 29 imports over 30 ms down to 2, longest 100 to 46 ms, with no device loss and no new warnings. That is a good sign for AMD. It is not Linux data. Linux is still untested, but the span logic is platform-neutral bookkeeping. If a spanned import fails, it is retried without the span, and the PR description already says it was only tested on Windows. I'm fine with it on by default given that fallback. If someone on Linux/RADV can run it once with `RX_VK_IMPORT_SPAN_MB=0` and once without, that would close it. Still open, all nits (see my earlier inline comments): - The comment on the one-call commit says already-committed pages are "left as they are". `VirtualAlloc(MEM_COMMIT)` re-applies the protection, so an import silently disarms an `RX_NVN_WATCH_*` range until the next re-arm. Fine for a diagnostic, but the comment and the PR text should say so. - The numbers in that comment (~33 of ~70 ms, 1.3 ms) don't match the PR text (~46 of ~95 ms, ~3 ms). - `tests/test_import_span.py` isn't in `ARGS` in `tests/run_all.py`, so it fails instead of being skipped on a machine without g++. CI is green. Good to merge from my side.
Commenting is not possible because the repository is archived.
No description provided.