MagicBeanDev/Longshot
MagicBeanDev/Longshot
Watch
54
67
Fork
You've already forked Longshot
 71
Code Issues 124 Pull requests 196 Projects Releases 8 Packages Wiki Activity Actions 442

WIP: perf(vk): a pool import does not pay a walk over the game's heap #334

Draft
DUDUYG wants to merge 3 commits from DUDUYG/Longshot:perf/pool-import-heap-walk into main
pull from: DUDUYG/Longshot:perf/pool-import-heap-walk
merge into: MagicBeanDev:main
Conversation 2 Commits 3 Files changed 14 +322 -5
DUDUYG commented 2026-11-14 01:37:30 +00:00
First-time contributor

All credit to @jarynclouatre. This is their NotRexed/Hookshot PR 3043 (Hookshot commit 771a9e46). It was merged into Hookshot on GitHub on 2026-11-13, but Longshot branched from 4dd972a8 before that night's merges, so it never reached Longshot. I'm re-sending their commit as it was, with them as the commit's author (8672a495 here). The only change is a one-line #include conflict, resolved. The first-touch fix, tests, docs and runs on top are mine, from the review. jarynclouatre, if you'd rather open it yourself, say so and I'll close this one.

Measured alone on current main (below), this shows no gain on my PC. With the lazy heap from #324, main's pool imports already take mostly 2-7 ms, and the fence doesn't change their number or time. I'm keeping it as work in progress with the review's fix in, in case other hardware shows a gain. If nobody does, I'd rather close it than have it merged for nothing.

Why

There was a short stall when something first came into view, for example when the Market appeared after a load. Most of it was one pool import on the thread that replays the frame. Before #324, nearly all of that import's time was Windows walking the game's heap, page by page, for the driver.

What changed

From the original PR (jarynclouatre's commit, 8672a495):

  • gpu/vk/import_fence.h (new): while the driver is asked for an import, the page right after the import's range is set to PAGE_EXECUTE_READWRITE instead of PAGE_READWRITE. That ends the run of identical pages, so Windows' region walk stops at the range. Right after the call the page gets its own protection back.
    • Only a committed PAGE_READWRITE page is changed.
    • Not used with the CPU write watches, which change protections themselves.
    • Linux is unchanged: there is no region walk there.
  • gpu/vk/vk_memory.cpp import_pool: the fence is held around vkAllocateMemory.
  • Tests: tests/import_fence_test.cpp and tests/test_import_fence.py, plus a line in tests/import_buffer_binding_test.cpp.
  • Conflict: on Longshot's main, vk_memory.cpp gained #include "mirror_budget.h" at the same place; both includes are kept.

Added for the review (76393403, 975df705):

  • host/mkw_glue.c commit_on_touch: a data fault on a page that is already committed is tried again when its protection is PAGE_EXECUTE_READWRITE too, not only PAGE_READWRITE. With the lazy heap, a guest thread can fault on the page just past an import; another thread commits it and the importer fences it, all before the pending fault reaches VirtualQuery. Main's handler then sent that recoverable first touch on to the crash handler. It still takes exact protection values, so not a guard or no-cache page, and still not an execute fault.
  • tests/test_import_fence.py: on Windows a second fixture runs the real commit_on_touch against real pages. It delivers read and write faults on the fenced page while the real ImportFence stands, and after it: both are tried again. An execute fault, a guard page and a read-only page are not. With main's handler it fails on the two data faults.
  • tests/test_commit_limit.py: its retry assertion follows the new condition.
  • docs/development.md: RX_VK_IMPORT_FENCE in the switch reference.
  • docs/tested/2026-11-14-market-entry-duduyg-{main,pr334}-run0{1,2,3}.md: the six runs below. The three main files are the same files in PR 335, so they merge as one.

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
RX_VK_IMPORT_FENCE=0 Turns the fence off, so imports work as before on (Windows) Undecided: no gain measured on current main Windows before the lazy heap (the original author's runs) Linux (no region walk there); RTX 5070 on current main (below) AMD and Intel on Windows

Measurements

The original author's runs (from Hookshot PR 3043), before Longshot existed and before the lazy heap: RTX 4070 Ti SUPER (621.24), Ryzen 7 9800X3D, 32 GB, Windows 11, about 6 minutes each, main and branch twice in turn. Route: a save, a teleport to the Market, then walking there. Imports of 2 ms or more went from 56 and 48 to 12 and 13, their time from 788 and 525 ms to 41 and 55 ms, and the longest from 35.7 and 17.1 to 9.2 and 7.7 ms.

This PR alone, on Longshot's current main with the lazy heap, as the review asked:

main: 5e072fc8 (2026-11-14), plus three env-gated test-logging commits of mine, b2698fae (RX_VK_PRESENT_LOG), a8988ce3 (the sampler on the same clock) and 0e78b98f (the sampler's own stalls): branch meas/main on my fork
this PR: meas/334 @ ca68a2f7, the same plus this PR's two commits (meas/334: 8104b642 and ca68a2f7, the code of 8672a495 and 76393403 here)
Baseline: main @ 5e072fc8, with the same logging commits.
Hardware: RTX 5070 (621.14), Ryzen 7 9800X3D, 32 GB, Windows 11 Pro 26300. The game copy on NVMe, the romfs on SATA.
Scene: a save in the Market, loaded from the title, then the camera turned left 4 s, right 8 s, left 4 s. 1920x1080 output, frame generation 4 and no cap in longshot_options.ini; the lazy heap on (HOS_HEAP_COMMIT unset).
Method: one warm-up run of each build, then main, this PR and PR 335 in turn, three times (09:24-09:43 local), with the disks idle before each. Input by script (HOS_INPUT_EXCLUSIVE=1). Frame times are each present's time since the one before (RX_VK_PRESENT_LOG), from 120 presents before the first log window that drew the Market to the run's end; the rest is longshot_log.txt lines. No run excluded: all nine drew the Market and stopped normally.
How to reproduce: build both branches, copy a Market save, then from the title press A every 4 s until the Market draws and turn the camera as above, with RX_VK_FRAME_TRACE=1 HOS_INPUT_EXCLUSIVE=1.

Market entry, 3 runs each: median [range] main @ 5e072fc8 this PR @ ca68a2f7
Pool imports (vk: import lines) per run 12 [12-12] 12 [11-12]
their time in all 80 [73-94] ms 97 [62-106] ms
the median import 4.6 [3.2-5.2] ms 3.7 [3.6-5.2] ms
the longest 26 [14-45] ms 47 [13-50] ms
Worst frame 445 [253-458] ms 255 [38-475] ms
Frames over 50 ms 7 [6-9] 6 [0-10]
Frames over 100 ms 4 [2-4] 2 [0-3]
Biggest slow submit 172 [166-174] ms 159 [149-163] ms
Median fps, Market windows 60.0 60.0

What it shows: the imports this fence works on take as long as on main, and there are as many of them. Before #324, main's imports at the Market entry took 7-15 ms each on this PC; now they take mostly 2-7 ms with or without the fence. The lazy heap commits the heap 64 KB at a time as it is touched, which may already break the run of identical pages the walk went over. I haven't checked that directly. One import of 45-54 ms shows in most runs of both builds. The frame times swing more within each build than between them: one run of this PR had no frame over 50 ms, another had a 475 ms frame. I don't read them as a gain.

M1_main  imports >= 2 ms: 3.3 2.2 3.0 6.1 2.7 2.7 26.4 5.8 3.4 6.0 6.5 11.6
M2_main  imports >= 2 ms: 4.3 2.8 3.3 2.8 14.1 6.1 3.8 6.1 6.2 9.2 3.2 11.4
M3_main  imports >= 2 ms: 4.3 3.1 2.4 2.8 3.2 2.4 45.1 12.1 3.3 6.2 6.4 2.6
M1_334   imports >= 2 ms: 3.1 3.6 2.3 2.7 2.9 46.8 11.8 3.3 6.1 6.2 7.8
M2_334   imports >= 2 ms: 4.3 2.5 3.0 3.0 50.0 6.2 4.0 7.4 6.3 9.6 7.8 2.2
M3_334   imports >= 2 ms: 3.7 2.6 2.2 2.5 12.8 4.6 3.7 9.0 8.5 2.4 6.8 3.6
M1_main  vk: import 3.3 ms: commit+properties 0.1, buffer 0.0, allocate 3.3, bind+address 0.0

Per run (worst frame / frames over 100 ms / import time in all): main 445/4/80, 253/2/73, 458/4/94 ms; this PR 255/2/97, 475/3/106, 38/0/62 ms.

The earlier combined runs (this PR together with Hookshot PR 2785, on 514591a3 before the lazy heap) are in the PR's history; they measured both changes, not this one.

Testing

Tested:

  • tests/test_import_fence.py on Windows: the original fixture, and the new first-touch fixture with the real fence and the real handler. It passes here and fails on main's handler with the two data faults.
  • tests/test_commit_limit.py with its updated assertion; tests/run_all.py import_fence import_buffer_binding commit_limit pass.
  • The full python -I -B tests/run_all.py on this branch (Windows, llvm-mingw clang 23): 513 passed, 8 failed, 41 skipped. The 8 fail the same way on main 5e072fc8: test_module_ids, test_pad_layout, test_bake_portability, test_framegen_resources, test_tested_configurations (its ASCII-pipe case gets CRLF on Windows), test_oot_state_tap, test_lift_cache, test_bug_report.
  • In the game: the nine Market runs above, with no crash.

Not tested:

  • The first-touch race in the game: it is a source-proven interleaving, not a crash I reproduced; the fixture delivers it deterministically.
  • AMD and Intel on Windows, and Linux.
  • Long sessions.

Needs other testers:

  • AMD or Intel on Windows: Market entry after a load, main against this PR, expecting vk: import lines that are shorter with the fence than without. No test request opened yet: on my PC it shows no gain, so I'd only ask testers for their time if a maintainer thinks it's still worth checking (request to open).

Screenshots

Nothing on screen.

Risk and rollback

The page after an import's range is executable for the length of one driver call. Nothing runs from guest memory, because the game's code is lifted host code. A fault on that page in that time is now tried again like one on a read-write page. RX_VK_IMPORT_FENCE=0 turns the fence off; reverting both commits is enough too.

Issues

See #324: its lazy heap, merged before these runs, appears to have taken most of what this fence saved.

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main (5e072fc8).
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above). 8 checks fail here and on main alike, named under Testing.
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). No test request opened: see Needs other testers.
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • No game files, dumps, build output or generated files.
  • third_party/ untouched (a new folder with a PROVENANCE.md instead).
  • The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags noted, and the switch reference updated (docs/development.md, the README).
  • A file in docs/tested/ for each new measured run (format in docs/tested-configurations.md; no appended row or assembled view).
  • A game-free test added for the fix, failing on main and passing here.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Docs updated.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. Built and run on Windows only; Linux not built.

🤖 Generated with Claude Code

> **All credit to @jarynclouatre.** This is their NotRexed/Hookshot PR 3043 (Hookshot commit 771a9e46). It was merged into Hookshot on GitHub on 2026-11-13, but Longshot branched from 4dd972a8 before that night's merges, so it never reached Longshot. I'm re-sending their commit as it was, with them as the commit's author (8672a495 here). The only change is a one-line `#include` conflict, resolved. The first-touch fix, tests, docs and runs on top are mine, from the review. jarynclouatre, if you'd rather open it yourself, say so and I'll close this one. > **Measured alone on current main (below), this shows no gain on my PC.** With the lazy heap from #324, main's pool imports already take mostly 2-7 ms, and the fence doesn't change their number or time. I'm keeping it as work in progress with the review's fix in, in case other hardware shows a gain. If nobody does, I'd rather close it than have it merged for nothing. ## Why There was a short stall when something first came into view, for example when the Market appeared after a load. Most of it was one pool import on the thread that replays the frame. Before #324, nearly all of that import's time was Windows walking the game's heap, page by page, for the driver. ## What changed From the original PR (jarynclouatre's commit, 8672a495): - `gpu/vk/import_fence.h` (new): while the driver is asked for an import, the page right after the import's range is set to `PAGE_EXECUTE_READWRITE` instead of `PAGE_READWRITE`. That ends the run of identical pages, so Windows' region walk stops at the range. Right after the call the page gets its own protection back. - Only a committed `PAGE_READWRITE` page is changed. - Not used with the CPU write watches, which change protections themselves. - Linux is unchanged: there is no region walk there. - `gpu/vk/vk_memory.cpp` `import_pool`: the fence is held around `vkAllocateMemory`. - Tests: `tests/import_fence_test.cpp` and `tests/test_import_fence.py`, plus a line in `tests/import_buffer_binding_test.cpp`. - Conflict: on Longshot's main, `vk_memory.cpp` gained `#include "mirror_budget.h"` at the same place; both includes are kept. Added for the review (76393403, 975df705): - `host/mkw_glue.c` `commit_on_touch`: a data fault on a page that is already committed is tried again when its protection is `PAGE_EXECUTE_READWRITE` too, not only `PAGE_READWRITE`. With the lazy heap, a guest thread can fault on the page just past an import; another thread commits it and the importer fences it, all before the pending fault reaches `VirtualQuery`. Main's handler then sent that recoverable first touch on to the crash handler. It still takes exact protection values, so not a guard or no-cache page, and still not an execute fault. - `tests/test_import_fence.py`: on Windows a second fixture runs the real `commit_on_touch` against real pages. It delivers read and write faults on the fenced page while the real `ImportFence` stands, and after it: both are tried again. An execute fault, a guard page and a read-only page are not. With main's handler it fails on the two data faults. - `tests/test_commit_limit.py`: its retry assertion follows the new condition. - `docs/development.md`: `RX_VK_IMPORT_FENCE` in the switch reference. - `docs/tested/2026-11-14-market-entry-duduyg-{main,pr334}-run0{1,2,3}.md`: the six runs below. The three main files are the same files in PR 335, so they merge as one. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `RX_VK_IMPORT_FENCE=0` | Turns the fence off, so imports work as before | on (Windows) | Undecided: no gain measured on current main | Windows before the lazy heap (the original author's runs) | Linux (no region walk there); RTX 5070 on current main (below) | AMD and Intel on Windows | ## Measurements The original author's runs (from Hookshot PR 3043), before Longshot existed and before the lazy heap: RTX 4070 Ti SUPER (621.24), Ryzen 7 9800X3D, 32 GB, Windows 11, about 6 minutes each, main and branch twice in turn. Route: a save, a teleport to the Market, then walking there. Imports of 2 ms or more went from 56 and 48 to 12 and 13, their time from 788 and 525 ms to 41 and 55 ms, and the longest from 35.7 and 17.1 to 9.2 and 7.7 ms. **This PR alone, on Longshot's current main with the lazy heap**, as the review asked: main: 5e072fc8 (2026-11-14), plus three env-gated test-logging commits of mine, b2698fae (`RX_VK_PRESENT_LOG`), a8988ce3 (the sampler on the same clock) and 0e78b98f (the sampler's own stalls): branch [meas/main](https://git.magicbean.dev/DUDUYG/Longshot/src/branch/meas/main) on my fork this PR: meas/334 @ ca68a2f7, the same plus this PR's two commits ([meas/334](https://git.magicbean.dev/DUDUYG/Longshot/src/branch/meas/334): 8104b642 and ca68a2f7, the code of 8672a495 and 76393403 here) Baseline: main @ 5e072fc8, with the same logging commits. Hardware: RTX 5070 (621.14), Ryzen 7 9800X3D, 32 GB, Windows 11 Pro 26300. The game copy on NVMe, the romfs on SATA. Scene: a save in the Market, loaded from the title, then the camera turned left 4 s, right 8 s, left 4 s. 1920x1080 output, frame generation 4 and no cap in `longshot_options.ini`; the lazy heap on (`HOS_HEAP_COMMIT` unset). Method: one warm-up run of each build, then main, this PR and PR 335 in turn, three times (09:24-09:43 local), with the disks idle before each. Input by script (`HOS_INPUT_EXCLUSIVE=1`). Frame times are each present's time since the one before (`RX_VK_PRESENT_LOG`), from 120 presents before the first log window that drew the Market to the run's end; the rest is `longshot_log.txt` lines. No run excluded: all nine drew the Market and stopped normally. How to reproduce: build both branches, copy a Market save, then from the title press A every 4 s until the Market draws and turn the camera as above, with `RX_VK_FRAME_TRACE=1 HOS_INPUT_EXCLUSIVE=1`. | Market entry, 3 runs each: median [range] | main @ 5e072fc8 | this PR @ ca68a2f7 | |---|---|---| | Pool imports (`vk: import` lines) per run | 12 [12-12] | 12 [11-12] | | their time in all | 80 [73-94] ms | 97 [62-106] ms | | the median import | 4.6 [3.2-5.2] ms | 3.7 [3.6-5.2] ms | | the longest | 26 [14-45] ms | 47 [13-50] ms | | Worst frame | 445 [253-458] ms | 255 [38-475] ms | | Frames over 50 ms | 7 [6-9] | 6 [0-10] | | Frames over 100 ms | 4 [2-4] | 2 [0-3] | | Biggest `slow submit` | 172 [166-174] ms | 159 [149-163] ms | | Median fps, Market windows | 60.0 | 60.0 | What it shows: the imports this fence works on take as long as on main, and there are as many of them. Before #324, main's imports at the Market entry took 7-15 ms each on this PC; now they take mostly 2-7 ms with or without the fence. The lazy heap commits the heap 64 KB at a time as it is touched, which may already break the run of identical pages the walk went over. I haven't checked that directly. One import of 45-54 ms shows in most runs of both builds. The frame times swing more within each build than between them: one run of this PR had no frame over 50 ms, another had a 475 ms frame. I don't read them as a gain. ``` M1_main imports >= 2 ms: 3.3 2.2 3.0 6.1 2.7 2.7 26.4 5.8 3.4 6.0 6.5 11.6 M2_main imports >= 2 ms: 4.3 2.8 3.3 2.8 14.1 6.1 3.8 6.1 6.2 9.2 3.2 11.4 M3_main imports >= 2 ms: 4.3 3.1 2.4 2.8 3.2 2.4 45.1 12.1 3.3 6.2 6.4 2.6 M1_334 imports >= 2 ms: 3.1 3.6 2.3 2.7 2.9 46.8 11.8 3.3 6.1 6.2 7.8 M2_334 imports >= 2 ms: 4.3 2.5 3.0 3.0 50.0 6.2 4.0 7.4 6.3 9.6 7.8 2.2 M3_334 imports >= 2 ms: 3.7 2.6 2.2 2.5 12.8 4.6 3.7 9.0 8.5 2.4 6.8 3.6 M1_main vk: import 3.3 ms: commit+properties 0.1, buffer 0.0, allocate 3.3, bind+address 0.0 ``` Per run (worst frame / frames over 100 ms / import time in all): main 445/4/80, 253/2/73, 458/4/94 ms; this PR 255/2/97, 475/3/106, 38/0/62 ms. The earlier combined runs (this PR together with Hookshot PR 2785, on 514591a3 before the lazy heap) are in the PR's history; they measured both changes, not this one. ## Testing **Tested:** - `tests/test_import_fence.py` on Windows: the original fixture, and the new first-touch fixture with the real fence and the real handler. It passes here and fails on main's handler with the two data faults. - `tests/test_commit_limit.py` with its updated assertion; `tests/run_all.py import_fence import_buffer_binding commit_limit` pass. - The full `python -I -B tests/run_all.py` on this branch (Windows, llvm-mingw clang 23): 513 passed, 8 failed, 41 skipped. The 8 fail the same way on main 5e072fc8: `test_module_ids`, `test_pad_layout`, `test_bake_portability`, `test_framegen_resources`, `test_tested_configurations` (its ASCII-pipe case gets CRLF on Windows), `test_oot_state_tap`, `test_lift_cache`, `test_bug_report`. - In the game: the nine Market runs above, with no crash. **Not tested:** - The first-touch race in the game: it is a source-proven interleaving, not a crash I reproduced; the fixture delivers it deterministically. - AMD and Intel on Windows, and Linux. - Long sessions. **Needs other testers:** - AMD or Intel on Windows: Market entry after a load, main against this PR, expecting `vk: import` lines that are shorter with the fence than without. No test request opened yet: on my PC it shows no gain, so I'd only ask testers for their time if a maintainer thinks it's still worth checking (request to open). ## Screenshots Nothing on screen. ## Risk and rollback The page after an import's range is executable for the length of one driver call. Nothing runs from guest memory, because the game's code is lifted host code. A fault on that page in that time is now tried again like one on a read-write page. `RX_VK_IMPORT_FENCE=0` turns the fence off; reverting both commits is enough too. ## Issues See #324: its lazy heap, merged before these runs, appears to have taken most of what this fence saved. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main` (5e072fc8). - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). 8 checks fail here and on main alike, named under Testing. - [x] Tested in the game, or what wasn't is named under Testing. - [ ] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). No test request opened: see Needs other testers. - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md, the README). - [x] A file in `docs/tested/` for each new measured run (format in `docs/tested-configurations.md`; no appended row or assembled view). - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Docs updated. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. Built and run on Windows only; Linux not built. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
perf(vk): a pool import does not pay a walk over the game's heap
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 6s
Details
Website / Website on Cloudflare (pull_request_target) Has been skipped
Details
CI / Conventional Commit title (pull_request) Has been cancelled
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
Details
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Details
Third-party parts / As their notes say (pull_request) Has been cancelled
Details
Third-party parts / The same as upstream (pull_request) Has been cancelled
Details
f6ea23908f 
The short stall when something first comes into view (#3013) is one pool
import on the thread that replays the frame, and nearly all of an
import's time is Windows walking the game's heap page by page for the
driver. With this change the imports in play take under 2 ms in place of
15 to 29 ms.

For #3013 (it takes the cost out of each import; it does not change how
many there are or where they run).

- All of an import's time is in `vkAllocateMemory` for the host pointer
(the `vk: import ... allocate` lines; buffer and bind are 0.0 ms).
- That time goes with the address, not the size. In six runs of one
route, imports in the heap's low 2 GB took a median 15 ms, the same for
4 MB and for 88 MB; imports above took 2 to 6 ms for 8 to 140 MB. With
every heap page touched at the start (`HOS_PREFAULT=1`) the low ones
took longer, 21 to 33 ms, not shorter.
- A build that times `VirtualQuery` at each import's start says where it
goes: in the low heap the call takes a median 13 to 17 ms and reports a
region of 5,838 MB. The heap is one run of committed read-write pages,
and Windows answers the question by walking from the address to the end
of the run of pages with the same state and protection. The lower the
address, the longer the walk, and the more of the heap is populated, the
slower each step.
- The imports that land in play are for the game's small pools: 64 bytes
or so in a page of their own, somewhere in the low heap, first bound
(read or written, in a compute dispatch) when what they belong to comes
into view. `RX_VK_ASYNC_IMPORT=1` leaves them where they are (5 on the
replay thread against 6).

While the driver is asked, the page after the import's range has
`PAGE_EXECUTE_READWRITE` in place of `PAGE_READWRITE`, so the region
that starts at the range ends with it. After the call the page has its
own protection back (`gpu/vk/import_fence.h`, used around
`vkAllocateMemory` in `import_pool`).

- Only a committed `PAGE_READWRITE` page is changed. A reserved page, or
a read-only one, already ends the run and is left alone; one changed by
someone else while the fence stands keeps what they gave it.
- Not with the CPU write watches (`RX_NVN_WATCH_TEX` and the others
`import_pool` already steps around), which change protections
themselves.
- Execute is the only protection that differs from read-write and still
reads and writes the same. Nothing runs from guest memory, with it or
without: the game's code is the lifted host code. It is on one page for
the length of one driver call.
- `RX_VK_IMPORT_FENCE=0` turns it off. Linux is unchanged (no region
walk there; the class does nothing).

- Windows 11, RTX 4070 Ti SUPER 16 GB (driver 621.24), Ryzen 7 9800X3D,
32 GB. Renderer builds of main e9975d6c and of this branch (llvm-mingw,
without DLSS) under one executable (main e0e60920), after a full
`--prepare-shaders`, the game running without a window (presents drawn
offscreen at 1920x1080) at 60 fps with scripted input. One route of
about six minutes, four times in a row (main, branch, main, branch): a
save loaded, the teleport cheat to the Market as a child, then walking
and turning there by script.

  | | main | this branch |
  | --- | --- | --- |
  | imports of 2 ms or more, whole run | 56 and 48 | 12 and 13 |
  | their time in all | 788 and 525 ms | 41 and 55 ms |
  | the longest | 35.7 and 17.1 ms | 9.2 and 7.7 ms |
| of them on the replay thread after the teleport | 10 (eight of 14.5 to
29.3 ms) and 6 (15.5 to 17.1 ms) | none and none |
  | `slow submit` lines after the teleport | 25 and 10 | 4 and 3 |
| of those, `dispatch` (where the small pools are bound) | 5 and 4 |
none and none |
  | frame rate at the end | 60.0 | 60.0 |

What is left on the branch: the first import of a run, whose time is in
the one-off `vkGetMemoryHostPointerPropertiesEXT` (9.0 and 7.3 ms; its
allocation takes 0.2 ms), and ranges of 20 to 90 MB at 2 to 8 ms, during
the loads. The slowest frame of a ten-second window is unchanged where
something else is the slowest: the teleport itself, and two `memory
sync` captures of 40 to 60 ms that both builds have.
- The measurements under Why are from the same PC and route: main's
renderer with `RX_VK_ASYNC_IMPORT=1` and with `HOS_PREFAULT=1` (two runs
each beside runs without), and a build that logs `VirtualQuery`'s time
and region at each import, with and without the page after the range
changed (two runs each). In those the changed page was left in place;
with it `VirtualQuery` from the range's start took 0.01 ms and reported
4 MB, and the allocations took 68 and 77 ms in all against 488 and 763
ms.

- `python -I -B tests/run_all.py`, Linux, with the Vulkan headers: 481
passed, 48 skipped, 1 failed. `nx2/test_texture_init_barrier` fails the
same way on main.
- New: `tests/test_import_fence.py`. On Windows its fixture runs the
fence on real pages: `VirtualQuery` from the range's start reports the
whole run without it and the range alone with it; the page reads and
writes while it stands and has its protection back after; a reserved or
read-only page after the range is left as it is; a watch armed on the
page meanwhile stays. On Linux it checks that the header builds and does
nothing. It also checks that `import_pool` holds the fence over its
`vkAllocateMemory`. It fails on main. The Windows fixture, built with
llvm-mingw, passes on Windows 11 on the PC above.
- `tests/test_import_buffer_binding.py` compiles `import_pool` itself,
so its fixture has a stand-in for the fence and now also checks that a
host-pointer allocation is made behind one, a dma-buf's without, and
none is left standing (37 cases pass).

**Not tested:** AMD and Intel cards on Windows (the comment at
`prefetch_chunks` reports 20 to 75 ms imports on an RX 9060 XT, which
may be the same walk; not measured here); Linux in play; a build with
DLSS; a long session.

Originally NotRexed/Hookshot PR 3043 by jarynclouatre, merged there on 2026-11-13 and not carried into Longshot when it moved
(Hookshot commit 771a9e46). Cherry-picked onto Longshot main; author kept. One conflict resolved: both sides added an #include at the same place in
gpu/vk/vk_memory.cpp (mirror_budget.h, import_fence.h), and both are kept.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
mooki requested changes 2026-11-14 04:44:18 +00:00
mooki left a comment

Read head f6ea2390, the complete five-file diff and description, the empty comments/reviews, the import callers and current main cf605c85's lazy-commit handler; six relevant Linux test drivers pass, with GCC and Clang for the compiled fixtures. No game or renderer build was run.

What's good

  • The change is small and surrounds the existing host-pointer allocation, including its failure return. It leaves the DMA import path and Linux page protections alone.
  • The four existing CPU write-watch switches disable the fence. The binding test checks that allocations happen inside its lifetime and that failure paths leave no fence standing.
  • The description distinguishes the original isolated comparison from the Longshot runs with two changes together.

Must change

  1. Preserve first-touch retries while the fence makes a page executable. gpu/vk/import_fence.h, lines 33-35, introduces PAGE_EXECUTE_READWRITE into guest memory, but current main's host/mkw_glue.c, lines 98-101, retries an already-committed data fault only when its protection is exactly PAGE_READWRITE. With the lazy Windows heap now on main, this interleaving fails: a guest thread faults on the page just beyond an import; another thread commits that page; the importer fences it before the pending fault reaches VirtualQuery. The handler now sees committed executable/read-write memory, misses its retry branch and returns EXCEPTION_CONTINUE_SEARCH, sending an otherwise recoverable first touch towards the crash handler. Without the fence, the same sequence retries successfully. The clean merge retains this mismatch. Make the Windows data-fault retry recognize the fence's readable/writable protection without accepting execute faults or protected pages, and add a deterministic case that delivers the pending fault while the real fence stands. Update the existing retry assertion in tests/test_commit_limit.py rather than removing it. This is a source-proven interleaving, not a reproduced game crash.

Measurement and smaller points

  • The original author's repeated import counts and times support a reduction on the named older NVIDIA/Windows build. The Longshot table measures this together with another change, not this PR alone, as disclosed. It does not establish this change's current-main frame-time gain. Before readying it, provide an isolated, interleaved comparison with the lazy heap enabled, report medians and repeat spread, and retain the excluded run and actual log lines. The heap change at #324 is already merged, not merely a possible future overlap.
  • Document RX_VK_IMPORT_FENCE in the switch reference and link the requested Windows AMD/Intel tests before ticking those checklist items. Put new measured runs under docs/tested/; the old table is frozen. This patch adds no table rows to move.
  • The new fence fixture passes Windows-target syntax checking. The binding fixture compiles too, but its additional CRT-policy check fails identically on main, so that check is not a new defect. Neither was executed on Windows here. The modified binding test against main's source fails at the new fence assertion with both compilers, rather than at compilation.

Not checked

No Windows execution, game, GPU, long-session or full-suite testing here. The new Linux fence test exercises the no-op branch, not Windows memory protection or the first-touch interaction above. I have not inspected the original raw captures; the original GitHub PR returned 404. The current source-check jobs are blocked, not green: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/3024.

Read head `f6ea2390`, the complete five-file diff and description, the empty comments/reviews, the import callers and current main `cf605c85`'s lazy-commit handler; six relevant Linux test drivers pass, with GCC and Clang for the compiled fixtures. No game or renderer build was run. ### What's good - The change is small and surrounds the existing host-pointer allocation, including its failure return. It leaves the DMA import path and Linux page protections alone. - The four existing CPU write-watch switches disable the fence. The binding test checks that allocations happen inside its lifetime and that failure paths leave no fence standing. - The description distinguishes the original isolated comparison from the Longshot runs with two changes together. ### Must change 1. **Preserve first-touch retries while the fence makes a page executable.** `gpu/vk/import_fence.h`, lines 33-35, introduces `PAGE_EXECUTE_READWRITE` into guest memory, but current main's `host/mkw_glue.c`, lines 98-101, retries an already-committed data fault only when its protection is exactly `PAGE_READWRITE`. With the lazy Windows heap now on main, this interleaving fails: a guest thread faults on the page just beyond an import; another thread commits that page; the importer fences it before the pending fault reaches `VirtualQuery`. The handler now sees committed executable/read-write memory, misses its retry branch and returns `EXCEPTION_CONTINUE_SEARCH`, sending an otherwise recoverable first touch towards the crash handler. Without the fence, the same sequence retries successfully. The clean merge retains this mismatch. Make the Windows data-fault retry recognize the fence's readable/writable protection without accepting execute faults or protected pages, and add a deterministic case that delivers the pending fault while the real fence stands. Update the existing retry assertion in `tests/test_commit_limit.py` rather than removing it. This is a source-proven interleaving, not a reproduced game crash. ### Measurement and smaller points - The original author's repeated import counts and times support a reduction on the named older NVIDIA/Windows build. The Longshot table measures this together with another change, not this PR alone, as disclosed. It does not establish this change's current-main frame-time gain. Before readying it, provide an isolated, interleaved comparison with the lazy heap enabled, report medians and repeat spread, and retain the excluded run and actual log lines. The heap change at https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/324 is already merged, not merely a possible future overlap. - Document `RX_VK_IMPORT_FENCE` in the switch reference and link the requested Windows AMD/Intel tests before ticking those checklist items. Put new measured runs under `docs/tested/`; the old table is frozen. This patch adds no table rows to move. - The new fence fixture passes Windows-target syntax checking. The binding fixture compiles too, but its additional CRT-policy check fails identically on main, so that check is not a new defect. Neither was executed on Windows here. The modified binding test against main's source fails at the new fence assertion with both compilers, rather than at compilation. ### Not checked No Windows execution, game, GPU, long-session or full-suite testing here. The new Linux fence test exercises the no-op branch, not Windows memory protection or the first-touch interaction above. I have not inspected the original raw captures; the original GitHub PR returned 404. The current source-check jobs are blocked, not green: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/3024.
DUDUYG force-pushed perf/pool-import-heap-walk from f6ea23908f
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 6s
Details
Website / Website on Cloudflare (pull_request_target) Has been skipped
Details
CI / Conventional Commit title (pull_request) Has been cancelled
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
Details
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Details
Third-party parts / As their notes say (pull_request) Has been cancelled
Details
Third-party parts / The same as upstream (pull_request) Has been cancelled
Details
to 975df705ac
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
Details
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Details
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Details
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Details
Website / Website on Cloudflare (pull_request) Blocked by required conditions
Details
PR description / Description follows the template (pull_request_target) Successful in 4s
Details
Website / Website on Cloudflare (pull_request_target) Has been skipped
Details
2026-11-14 12:46:05 +00:00 Compare
DUDUYG commented 2026-11-14 12:50:20 +00:00
Author
First-time contributor

@mooki thanks for the review. Pushed 76393403 and 975df705, rebased onto 5e072fc8 (jarynclouatre's commit is 8672a495 now).

  • First-touch retry: commit_on_touch now also retries a committed data fault on PAGE_EXECUTE_READWRITE. It still takes exact values only, and execute faults still go on.
    • tests/test_import_fence.py has a Windows fixture that runs the real handler on real pages with the real ImportFence standing. A read fault and a write fault are tried again, during the fence and after it. An execute fault, a guard page and a read-only page are not.
    • On main's handler that fixture fails on the two data faults.
    • The assertion in tests/test_commit_limit.py follows the new condition (updated, not removed).
  • Docs: RX_VK_IMPORT_FENCE is in docs/development.md.
  • Isolated runs: on 5e072fc8 with the lazy heap on, three interleaved runs of each build (main, this PR and PR 335 in turn).
    • The description has the medians, the ranges, the per-run numbers and the import lines. The six runs are in docs/tested/.
    • The measured builds are on my fork: meas/main and meas/334.
    • No run was excluded this time.
  • Result: no gain I can see.
    • The imports are as many and as long as on main: 12 per run, mostly 2-7 ms. Before #324 they were 7-15 ms on this PC.
    • The frame times vary more within a build than between the two builds.
    • It looks like the lazy heap already took what the fence saved.
  • What I'm doing with it: I'm keeping this WIP and haven't opened an AMD/Intel test request, so it doesn't use testers' time unless you think it's worth checking. If nobody sees a gain elsewhere, I'll close it.
@mooki thanks for the review. Pushed 76393403 and 975df705, rebased onto 5e072fc8 (jarynclouatre's commit is 8672a495 now). - **First-touch retry:** `commit_on_touch` now also retries a committed data fault on `PAGE_EXECUTE_READWRITE`. It still takes exact values only, and execute faults still go on. - `tests/test_import_fence.py` has a Windows fixture that runs the real handler on real pages with the real `ImportFence` standing. A read fault and a write fault are tried again, during the fence and after it. An execute fault, a guard page and a read-only page are not. - On main's handler that fixture fails on the two data faults. - The assertion in `tests/test_commit_limit.py` follows the new condition (updated, not removed). - **Docs:** `RX_VK_IMPORT_FENCE` is in `docs/development.md`. - **Isolated runs:** on 5e072fc8 with the lazy heap on, three interleaved runs of each build (main, this PR and PR 335 in turn). - The description has the medians, the ranges, the per-run numbers and the import lines. The six runs are in `docs/tested/`. - The measured builds are on my fork: `meas/main` and `meas/334`. - No run was excluded this time. - **Result: no gain I can see.** - The imports are as many and as long as on main: 12 per run, mostly 2-7 ms. Before #324 they were 7-15 ms on this PC. - The frame times vary more within a build than between the two builds. - It looks like the lazy heap already took what the fence saved. - **What I'm doing with it:** I'm keeping this WIP and haven't opened an AMD/Intel test request, so it doesn't use testers' time unless you think it's worth checking. If nobody sees a gain elsewhere, I'll close it.
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
Details
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Details
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Details
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Details
Website / Website on Cloudflare (pull_request) Blocked by required conditions
Details
PR description / Description follows the template (pull_request_target) Successful in 4s
Details
Website / Website on Cloudflare (pull_request_target) Has been skipped
Details
This pull request is marked as a work in progress.
Some workflows are waiting to be reviewed.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'perf/pool-import-heap-walk:DUDUYG-perf/pool-import-heap-walk'
git switch 'DUDUYG-perf/pool-import-heap-walk'
Sign in to join this conversation.
Reviewers
No reviewers
mooki
Labels
Clear labels   
area/accessibility
Barriers for people with disabilities

  
area/audio
Sound and music

  
area/input
Controllers, keyboard, mouse

  
area/install
Installing, updating, building from your game

  
area/menu
In-game menu and settings

  
area/renderer
Vulkan renderer, shaders, effects

  
area/saves
Save files

  
area/upscaling
DLSS, FSR, frame generation

  
good first issue
A good start for a new contributor

  
gpu/amd
AMD GPUs

  
gpu/igpu
Integrated GPUs

  
gpu/intel
Intel GPUs

  
gpu/nvidia
NVIDIA GPUs

  
help wanted
Extra hands or knowledge wanted

  
kind
bug
 Something doesn't work as it should

  
kind
chore
 Build, CI, dependencies, tidying

  
kind
docs
 Documentation

  
kind
feature
 A new feature or an improvement

  
kind
performance
 Frame rate, frame time, loading, memory

  
kind
question
 A question rather than a change

  
needs measurements
Waiting for before/after measurements

  
platform/linux
Linux only

  
platform/steam-deck
Steam Deck

  
platform/windows
Windows only

  
preview
Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  
priority
critical
 Crashes, data loss, or blocks most players

  
priority
high
 Important: next in line

  
priority
low
 Nice to have

  
status
blocked
 Waits on something else first

  
status
confirmed
 Reproduced or agreed on

  
status
duplicate
 Already reported or proposed

  
status
invalid
 Not a problem in this project

  
status
needs-info
 Waiting for more information from the reporter

  
status
wontfix
 Will not be worked on

  
testing wanted
Wanted: a test on other hardware (posts a test request in #test-requests)

No labels
area/accessibility
area/audio
area/input
area/install
area/menu
area/renderer
area/saves
area/upscaling
good first issue
gpu/amd
gpu/igpu
gpu/intel
gpu/nvidia
help wanted
kind
bug
kind
chore
kind
docs
kind
feature
kind
performance
kind
question
needs measurements
platform/linux
platform/steam-deck
platform/windows
preview
priority
critical
priority
high
priority
low
status
blocked
status
confirmed
status
duplicate
status
invalid
status
needs-info
status
wontfix
testing wanted
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!334
No description provided.