MagicBeanDev/Longshot
MagicBeanDev/Longshot
Watch
55
68
Fork
You've already forked Longshot
 71
Code Issues 125 Pull requests 195 Projects Releases 8 Packages Wiki Activity Actions 445

perf(vk): sparse VRAM mirrors, memory only where a game pool is (on by default) #250

Merged
HumanBean merged 3 commits from GummiNaomi/Longshot:perf/sparse-mirror into main 2026-11-14 02:14:56 +00:00
Conversation 4 Commits 3 Files changed 19 +1100 -32
GummiNaomi commented 2026-11-13 17:12:11 +00:00
Contributor

Why

On cards short of video memory, the buffer pools' VRAM mirrors hold hundreds of MB that no game pool is in: 243-533 MB of 1.1-1.6 GB at 2560x1440 DLAA. That pushes textures out and costs frame rate on 6-8 GB cards. This makes the mirrors sparse, on by default (the maintainer's decision), so memory is held only in the 64 KB blocks a game pool is in. It was https://github.com/NotRexed/Hookshot/pull/2794 before the move; it may also help #130 (a GPU reset at the VRAM limit on an 8 GB RX 6600).

What changed

  • gpu/vk/sparse_mirror.h (new): which blocks a game pool is in, block spans, runs, and copies clipped to blocks with memory.
  • gpu/vk/vk_vma.cpp: a sparse buffer, and its blocks' memory from VMA (counted in its budget). vmaFreeMemory joins the recorder's drain boundaries (gpu/vk/vk_recorder.h).
  • gpu/vk/vk_memory.cpp:
    • mirror_sparse_on: on unless RX_VK_MIRROR_SPARSE=0 (or off, false, no); off on its own, and logged, where the device lacks strict sparse residency for buffers.
    • mirror_make: a sparse buffer with memory for the blocks a game pool is in. The page table points blocks with none at guest memory, as for a pool with no mirror. Its "stays in system memory" line prints the pool's size and its mirror's.
    • gpu_buffer_bind: a binding whose blocks with no memory are all outside every game pool (sparse_gaps_outside_pools: a guessed size run past a pool's end) is served from the mirror as it is. Those blocks read 0 and drop writes, and nothing of the game's is there. Otherwise a binding that reaches a block with no memory is served from guest memory for that submission, and the block is wanted. The log counts both: st_sparse_mixed (served from guest memory and reaching blocks with memory too, reads and writes) and st_sparse_past (served from the mirror past every pool).
    • sparse_grow: at each submission's start, the wanted blocks and the blocks newly in a pool get memory in one batched bind, filled as a new mirror is (the late pages' fix, late_fix_emit, recorded before the fill copies captured pages out of a capture's buffer, as mirror_sync and mirror_make do since #248).
    • sparse_release: a block no pool or binding has reached for 600 submissions gives its memory back, once the GPU is past it.
    • Binds signal a timeline semaphore that the recordings wait for on the GPU, not on the CPU (RX_VK_SPARSE_WAIT=1: on the CPU, as before). The semaphore is made under dev.present_mu, which memory_sparse_wait reads it under.
    • Write-backs and a retired mirror's inheritance are clipped to blocks with memory. The write-back's "reached blocks with no memory" line is logged only where a game pool is (past every pool is expected, and silent).
  • gpu/vk/vk_replay.cpp: submissions of recordings wait for the binds' semaphore.
  • docs/development.md: RX_VK_MIRROR_SPARSE=0 and RX_VK_SPARSE_WAIT=1 in the switch reference.
  • docs/tested/: the Hookshot runs, one file each (2026-11-12-sparse-mirror-gumminaomi-run01.md to run04.md), not rows in the frozen docs/tested-configurations.md.
  • Tests: tests/test_sparse_mirror.py (+ tests/test_sparse_mirror.cpp) checks the default, the binding's two paths and where the semaphore is made, and takes run_all's {cxx}. test_direct_buffer_writes and test_readback_part were updated for the new code they slice.

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
RX_VK_MIRROR_SPARSE Sparse VRAM mirrors: memory only in the blocks a game pool is in (needs strict sparse residency for buffers; off on its own otherwise, logged). =0: whole mirrors, as before on on by default (the maintainer's decision); =0 to compare or if a device misbehaves cards near their VRAM limit (6-8 GB) cards with plenty of VRAM (no fps change at 12 GB) AMD (RADV, Windows), Intel, Linux; the default (no switch set) in a Longshot build
RX_VK_SPARSE_WAIT =1: each sparse bind waited for on the CPU, as before off off (an area change's longest frame was 1.2 s with it, 0.9 without) — — —

Documented in docs/development.md's switch reference.

Measurements

WIP: the Longshot A/B (main against this PR at its head, with no switch set now that it is on by default) is still to do. The numbers below are from Hookshot, before the move and before main moved on: the same code, one build, the switch off and on (=1, set explicitly). Rebased since onto main 76699aad (2026-11-14): they are not measured again.

main: not measured (Hookshot runs; Longshot main 76699aad still to measure)
this PR: perf/sparse-mirror @ 00c7fd24 (not measured); measured on Hookshot builds of this change, see Hardware
Baseline: Hookshot @ 564ea648 (to confirm), RX_VK_MIRROR_SPARSE unset (off then)
Why not main: measured on Hookshot before the move.
Hardware: RTX 4070 SUPER 12 GB, NVIDIA 617.42, 32 GB, Windows 11 (6 and 8 GB cards simulated with RX_VK_TEST_ALLOWED_MB). To be confirmed by the author: this description said i7-14700K, Windows 11 Pro 26200; the tested-configurations rows (now the docs/tested/ files) say Ryzen 9 9950X3D (SMT off). The builds disagree too: Hookshot 130ee5e6 here before, 990d477d in the first commit's message, v2.1.0-rc.3-249-g564ea648 and later on Hookshot main c544828f in the rows.
Scene: Hyrule Field walk, Castle Town to the field, Kokiri Forest; 2560x1440 DLAA, unattended
Method: frame trace and VRAM log lines, three pairs per scene where stated
How to reproduce: RX_VK_TEST_ALLOWED_MB=6144 RX_VK_MIRROR_MB=1536 (6 GB) or =6900 (8 GB), the scripted field walk; RX_VK_MIRROR_SPARSE=0 against unset (on) now

Hookshot @ 564ea648 (to confirm), RX_VK_MIRROR_SPARSE unset (off then) Hookshot @ 564ea648 (to confirm), RX_VK_MIRROR_SPARSE=1 (now the default)
6 GB allowance (RX_VK_TEST_ALLOWED_MB=6144, RX_VK_MIRROR_MB=1536), field walk, three pairs: fps 47.7, 51.5, 49.9 57.3, 55.3, 58.2
the same: frames over 25 ms 3,042, 1,840, 2,114 625, 1,160, 408
the same: VRAM peak 7,327, 6,254, 6,444 MB 6,394, 6,200, 6,387 MB
8 GB allowance (=6900), field walk: fps / VRAM peak 59.0-59.1 / 7,028-7,138 MB 59.1-59.3 / 6,676-7,018 MB
Castle Town to the field: the load's longest frame 816-902 ms 866-961 ms
Kokiri Forest: VRAM mean 8,215 MB 7,462-7,507 MB 
(the log lines are in the author's Hookshot runs; to add with the Longshot A/B)

A real 8 GB card (klikso, on https://github.com/NotRexed/Hookshot/pull/2794): RTX 3060 Ti 8 GB, 2560x1440 DLSS K. About 430 MB less in the mirrors, 120-370 MB less VRAM in Castle Town Market, and a higher texture budget. Frame rate and long frames were the same within three pairs; nothing broke.

Testing

Tested:

  • The author, on an earlier head (8cf52d3f, on main 2e1e2e8c): the game-free checks that read the changed files, 109 pass; 8 fail the same way on main on that PC (test_audio_volume, test_oot_fov, test_opening_skip, test_present_aspect_vulkan, test_present_thread, test_readback_wait_bounded, test_text_size, test_texture_staging_publication: they don't build with that PC's compilers).
  • In the game, on Hookshot builds of this change with RX_VK_MIRROR_SPARSE=1:
    • the runs above;
    • screenshots of Castle Town, Kokiri and the field look the same on and off;
    • no device lost and no write-back warning in any run;
    • the player's own sessions with it on, in Hookshot builds since 12 Nov.
  • Two independent code reviews of earlier versions found ten problems, all fixed.

Not tested:

  • This head (00c7fd24): no build, no game-free checks and no game run. The default-on change, the mixed-binding change (served from the mirror past every game pool), the semaphore's creation under the lock, the late pages' fix in the sparse fill and the log lines were made by reading only.
  • With no switch set (the new default) in any build.
  • Together with main's changes since 2e1e2e8c, among them the late pages' fix (#248), which reads the write-back shadows a sparse mirror's fill forgets.
  • AMD (RADV and Windows), Intel, Linux.
  • Long sessions on a real 6 GB card.

Needs other testers:

  • AMD RADV (Linux), a 6-8 GB card: the field walk with the default and with RX_VK_MIRROR_SPARSE=0, expecting lower VRAM, equal or higher fps, and no lost device (request to open).
  • AMD on Windows, a 6-8 GB card: the same, expecting the same (request to open).
  • Intel Arc: the same, expecting the same, or the log line saying it's off when the device lacks strict sparse residency (request to open).

Screenshots

Same spot, off and on: identical (Hookshot). Nothing on screen should change; to add from the Longshot A/B.

Risk and rollback

On by default, for everyone with strict sparse residency for buffers, though only NVIDIA on Windows has run it: AMD, Intel and Linux are untested. A block that is wanted but not bound yet is served from guest memory for one submission, as a pool with no mirror always is. A driver bug in sparse binding could lose the device. RX_VK_MIRROR_SPARSE=0 turns it off (whole mirrors, as before); reverting the commits is enough too.

Reviewer notes

Start with gpu/vk/sparse_mirror.h, then sparse_grow and sparse_release in vk_memory.cpp, then the two sparse branches in gpu_buffer_bind and sparse_gaps_outside_pools.

Issues

See #130

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above): not run on this head (nothing was run where it was rebased and changed); the author's partial run was of an earlier head.
  • 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): requests to open.
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. Pending: the Longshot A/B, with the default.
  • 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): the four Hookshot runs have one each, but their CPU and builds are to be confirmed (see Measurements), and the Longshot A/B's are still to add.
  • A check under tests/ that reads changed source updated in this PR, not weakened.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths: not built at this head; the runs above were Windows builds of earlier versions, and no Linux build is named.
  • Docs updated.
## Why On cards short of video memory, the buffer pools' VRAM mirrors hold hundreds of MB that no game pool is in: 243-533 MB of 1.1-1.6 GB at 2560x1440 DLAA. That pushes textures out and costs frame rate on 6-8 GB cards. This makes the mirrors sparse, **on by default** (the maintainer's decision), so memory is held only in the 64 KB blocks a game pool is in. It was https://github.com/NotRexed/Hookshot/pull/2794 before the move; it may also help [#130](https://git.magicbean.dev/MagicBeanDev/Longshot/issues/130) (a GPU reset at the VRAM limit on an 8 GB RX 6600). ## What changed - `gpu/vk/sparse_mirror.h` (new): which blocks a game pool is in, block spans, runs, and copies clipped to blocks with memory. - `gpu/vk/vk_vma.cpp`: a sparse buffer, and its blocks' memory from VMA (counted in its budget). `vmaFreeMemory` joins the recorder's drain boundaries (`gpu/vk/vk_recorder.h`). - `gpu/vk/vk_memory.cpp`: - `mirror_sparse_on`: on unless `RX_VK_MIRROR_SPARSE=0` (or `off`, `false`, `no`); off on its own, and logged, where the device lacks strict sparse residency for buffers. - `mirror_make`: a sparse buffer with memory for the blocks a game pool is in. The page table points blocks with none at guest memory, as for a pool with no mirror. Its "stays in system memory" line prints the pool's size and its mirror's. - `gpu_buffer_bind`: a binding whose blocks with no memory are all outside every game pool (`sparse_gaps_outside_pools`: a guessed size run past a pool's end) is served from the mirror as it is. Those blocks read 0 and drop writes, and nothing of the game's is there. Otherwise a binding that reaches a block with no memory is served from guest memory for that submission, and the block is wanted. The log counts both: `st_sparse_mixed` (served from guest memory and reaching blocks with memory too, reads and writes) and `st_sparse_past` (served from the mirror past every pool). - `sparse_grow`: at each submission's start, the wanted blocks and the blocks newly in a pool get memory in one batched bind, filled as a new mirror is (the late pages' fix, `late_fix_emit`, recorded before the fill copies captured pages out of a capture's buffer, as `mirror_sync` and `mirror_make` do since [#248](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/248)). - `sparse_release`: a block no pool or binding has reached for 600 submissions gives its memory back, once the GPU is past it. - Binds signal a timeline semaphore that the recordings wait for on the GPU, not on the CPU (`RX_VK_SPARSE_WAIT=1`: on the CPU, as before). The semaphore is made under `dev.present_mu`, which `memory_sparse_wait` reads it under. - Write-backs and a retired mirror's inheritance are clipped to blocks with memory. The write-back's "reached blocks with no memory" line is logged only where a game pool is (past every pool is expected, and silent). - `gpu/vk/vk_replay.cpp`: submissions of recordings wait for the binds' semaphore. - `docs/development.md`: `RX_VK_MIRROR_SPARSE=0` and `RX_VK_SPARSE_WAIT=1` in the switch reference. - `docs/tested/`: the Hookshot runs, one file each (`2026-11-12-sparse-mirror-gumminaomi-run01.md` to `run04.md`), not rows in the frozen `docs/tested-configurations.md`. - Tests: `tests/test_sparse_mirror.py` (+ `tests/test_sparse_mirror.cpp`) checks the default, the binding's two paths and where the semaphore is made, and takes run_all's `{cxx}`. `test_direct_buffer_writes` and `test_readback_part` were updated for the new code they slice. ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | `RX_VK_MIRROR_SPARSE` | Sparse VRAM mirrors: memory only in the blocks a game pool is in (needs strict sparse residency for buffers; off on its own otherwise, logged). `=0`: whole mirrors, as before | on | on by default (the maintainer's decision); `=0` to compare or if a device misbehaves | cards near their VRAM limit (6-8 GB) | cards with plenty of VRAM (no fps change at 12 GB) | AMD (RADV, Windows), Intel, Linux; the default (no switch set) in a Longshot build | | `RX_VK_SPARSE_WAIT` | `=1`: each sparse bind waited for on the CPU, as before | off | off (an area change's longest frame was 1.2 s with it, 0.9 without) | — | — | — | Documented in `docs/development.md`'s switch reference. ## Measurements **WIP: the Longshot A/B (main against this PR at its head, with no switch set now that it is on by default) is still to do.** The numbers below are from Hookshot, before the move and before main moved on: the same code, one build, the switch off and on (`=1`, set explicitly). Rebased since onto main 76699aad (2026-11-14): they are not measured again. main: not measured (Hookshot runs; Longshot main 76699aad still to measure) this PR: perf/sparse-mirror @ 00c7fd24 (not measured); measured on Hookshot builds of this change, see Hardware Baseline: Hookshot @ 564ea648 (to confirm), `RX_VK_MIRROR_SPARSE` unset (off then) Why not main: measured on Hookshot before the move. Hardware: RTX 4070 SUPER 12 GB, NVIDIA 617.42, 32 GB, Windows 11 (6 and 8 GB cards simulated with `RX_VK_TEST_ALLOWED_MB`). **To be confirmed by the author:** this description said i7-14700K, Windows 11 Pro 26200; the tested-configurations rows (now the `docs/tested/` files) say Ryzen 9 9950X3D (SMT off). The builds disagree too: Hookshot `130ee5e6` here before, `990d477d` in the first commit's message, `v2.1.0-rc.3-249-g564ea648` and later on Hookshot main `c544828f` in the rows. Scene: Hyrule Field walk, Castle Town to the field, Kokiri Forest; 2560x1440 DLAA, unattended Method: frame trace and VRAM log lines, three pairs per scene where stated How to reproduce: `RX_VK_TEST_ALLOWED_MB=6144 RX_VK_MIRROR_MB=1536` (6 GB) or `=6900` (8 GB), the scripted field walk; `RX_VK_MIRROR_SPARSE=0` against unset (on) now | | Hookshot @ 564ea648 (to confirm), `RX_VK_MIRROR_SPARSE` unset (off then) | Hookshot @ 564ea648 (to confirm), `RX_VK_MIRROR_SPARSE=1` (now the default) | |---|---|---| | 6 GB allowance (`RX_VK_TEST_ALLOWED_MB=6144`, `RX_VK_MIRROR_MB=1536`), field walk, three pairs: fps | 47.7, 51.5, 49.9 | 57.3, 55.3, 58.2 | | the same: frames over 25 ms | 3,042, 1,840, 2,114 | 625, 1,160, 408 | | the same: VRAM peak | 7,327, 6,254, 6,444 MB | 6,394, 6,200, 6,387 MB | | 8 GB allowance (`=6900`), field walk: fps / VRAM peak | 59.0-59.1 / 7,028-7,138 MB | 59.1-59.3 / 6,676-7,018 MB | | Castle Town to the field: the load's longest frame | 816-902 ms | 866-961 ms | | Kokiri Forest: VRAM mean | 8,215 MB | 7,462-7,507 MB | ``` (the log lines are in the author's Hookshot runs; to add with the Longshot A/B) ``` **A real 8 GB card** (klikso, on https://github.com/NotRexed/Hookshot/pull/2794): RTX 3060 Ti 8 GB, 2560x1440 DLSS K. About 430 MB less in the mirrors, 120-370 MB less VRAM in Castle Town Market, and a higher texture budget. Frame rate and long frames were the same within three pairs; nothing broke. ## Testing **Tested:** - The author, on an earlier head (8cf52d3f, on main 2e1e2e8c): the game-free checks that read the changed files, 109 pass; 8 fail the same way on main on that PC (`test_audio_volume`, `test_oot_fov`, `test_opening_skip`, `test_present_aspect_vulkan`, `test_present_thread`, `test_readback_wait_bounded`, `test_text_size`, `test_texture_staging_publication`: they don't build with that PC's compilers). - In the game, on Hookshot builds of this change with `RX_VK_MIRROR_SPARSE=1`: - the runs above; - screenshots of Castle Town, Kokiri and the field look the same on and off; - no device lost and no write-back warning in any run; - the player's own sessions with it on, in Hookshot builds since 12 Nov. - Two independent code reviews of earlier versions found ten problems, all fixed. **Not tested:** - This head (00c7fd24): no build, no game-free checks and no game run. The default-on change, the mixed-binding change (served from the mirror past every game pool), the semaphore's creation under the lock, the late pages' fix in the sparse fill and the log lines were made by reading only. - With no switch set (the new default) in any build. - Together with main's changes since 2e1e2e8c, among them the late pages' fix ([#248](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/248)), which reads the write-back shadows a sparse mirror's fill forgets. - AMD (RADV and Windows), Intel, Linux. - Long sessions on a real 6 GB card. **Needs other testers:** - AMD RADV (Linux), a 6-8 GB card: the field walk with the default and with `RX_VK_MIRROR_SPARSE=0`, expecting lower VRAM, equal or higher fps, and no lost device (request to open). - AMD on Windows, a 6-8 GB card: the same, expecting the same (request to open). - Intel Arc: the same, expecting the same, or the log line saying it's off when the device lacks strict sparse residency (request to open). ## Screenshots Same spot, off and on: identical (Hookshot). Nothing on screen should change; to add from the Longshot A/B. ## Risk and rollback On by default, for everyone with strict sparse residency for buffers, though only NVIDIA on Windows has run it: AMD, Intel and Linux are untested. A block that is wanted but not bound yet is served from guest memory for one submission, as a pool with no mirror always is. A driver bug in sparse binding could lose the device. `RX_VK_MIRROR_SPARSE=0` turns it off (whole mirrors, as before); reverting the commits is enough too. ## Reviewer notes Start with `gpu/vk/sparse_mirror.h`, then `sparse_grow` and `sparse_release` in `vk_memory.cpp`, then the two sparse branches in `gpu_buffer_bind` and `sparse_gaps_outside_pools`. ## Issues See [#130](https://git.magicbean.dev/MagicBeanDev/Longshot/issues/130) ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [ ] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above): not run on this head (nothing was run where it was rebased and changed); the author's partial run was of an earlier head. - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests): requests to open. - [ ] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. **Pending: the Longshot A/B, with the default.** - [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). - [ ] A file in `docs/tested/` for each new measured run (format in `docs/tested-configurations.md`; no appended row or assembled view): the four Hookshot runs have one each, but their CPU and builds are to be confirmed (see Measurements), and the Longshot A/B's are still to add. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [ ] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths: not built at this head; the runs above were Windows builds of earlier versions, and no Linux build is named. - [x] Docs updated.
perf(vk): sparse VRAM mirrors, memory only where a game pool is (RX_VK_MIRROR_SPARSE=1)
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 19s
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
8cf52d3f44 
Ported from Hookshot perf/sparse-mirror (990d477d, NotRexed/Hookshot#2794) onto Longshot
main as one change. Off by default.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>
HumanBean added this to the v3.0.0 milestone 2026-11-13 17:45:34 +00:00
HumanBean requested changes 2026-11-14 00:40:38 +00:00
HumanBean left a comment

Review of PR 250 at 8cf52d3f: code only. I didn't run any checks or builds. Its CI run (https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/1789) and the third-party run (https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/1790) were still pending when I looked, and the description check passed.

The design is careful. Released blocks are taken back still bound, release_gen keeps stale dead_blocks entries from acting, memory is freed after the binds' timeline, write-backs, inheritance and captured pages are clipped to blocks with memory, and an in-flight refill is cut out of inheritance. Signal-operation order on the one queue means waiting on the latest bind value covers the earlier binds too. It merges cleanly with current main. Asking for changes because of the description and one correctness question:

Must change before ready

  1. The Flags noted, and the switch reference updated box is ticked, but RX_VK_SPARSE_WAIT isn't in docs/development.md's switch reference. Only RX_VK_MIRROR_SPARSE is (inline).
  2. The hardware doesn't match. The Measurements say i7-14700K / Windows 11 Pro 26200. The new tested-configurations rows say Ryzen 9 9950X3D (SMT off). The commits named don't agree either: Hookshot 130ee5e6 in the description, 990d477d in the commit message, 564ea648 on c544828f in the rows (inline).
  3. If applicable boxes that apply are missing: Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths (the diff changes host code) and Docs updated (docs/development.md and docs/tested-configurations.md change). Add them, ticked only once true.
  4. NotRexed/Hookshot#2794 in the description and in the commit message should be the full GitHub URL. On the forge it doesn't link where it should (AGENTS.md).

Question (correctness)
5. A binding that reaches blocks both with and without memory is served wholly from guest memory (inline at gpu_buffer_bind). What the GPU wrote only into the mirror (stores through the page table, which the write-back doesn't copy) is missed by that read. For a write, the next submission's refill copies guest memory over that GPU-only data. What did st_sparse_mixed show in the runs? See the inline comment for a cheap way to avoid it in the common case.

Nits

  • sparse_tl is created outside dev.present_mu, but memory_sparse_wait reads it under that lock on another thread (inline).
  • The "stays in system memory" log's pool size now prints take (inline).
  • tests/test_sparse_mirror.py could use {cxx} rather than g++ (inline).

Still to do before pr ready, as the description already says: the Longshot A/B at named commits (rebased on current main), the full run_all.py, and the AMD, Intel and Linux test requests.

Review of PR 250 at 8cf52d3f: code only. I didn't run any checks or builds. Its CI run (https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/1789) and the third-party run (https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/1790) were still pending when I looked, and the description check passed. The design is careful. Released blocks are taken back still bound, `release_gen` keeps stale `dead_blocks` entries from acting, memory is freed after the binds' timeline, write-backs, inheritance and captured pages are clipped to blocks with memory, and an in-flight refill is cut out of inheritance. Signal-operation order on the one queue means waiting on the latest bind value covers the earlier binds too. It merges cleanly with current main. Asking for changes because of the description and one correctness question: **Must change before ready** 1. The *Flags noted, and the switch reference updated* box is ticked, but `RX_VK_SPARSE_WAIT` isn't in `docs/development.md`'s switch reference. Only `RX_VK_MIRROR_SPARSE` is (inline). 2. The hardware doesn't match. The Measurements say i7-14700K / Windows 11 Pro 26200. The new tested-configurations rows say Ryzen 9 9950X3D (SMT off). The commits named don't agree either: Hookshot `130ee5e6` in the description, `990d477d` in the commit message, `564ea648` on `c544828f` in the rows (inline). 3. *If applicable* boxes that apply are missing: *Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths* (the diff changes host code) and *Docs updated* (`docs/development.md` and `docs/tested-configurations.md` change). Add them, ticked only once true. 4. `NotRexed/Hookshot#2794` in the description and in the commit message should be the full GitHub URL. On the forge it doesn't link where it should (AGENTS.md). **Question (correctness)** 5. A binding that reaches blocks both with and without memory is served wholly from guest memory (inline at `gpu_buffer_bind`). What the GPU wrote only into the mirror (stores through the page table, which the write-back doesn't copy) is missed by that read. For a write, the next submission's refill copies guest memory over that GPU-only data. What did `st_sparse_mixed` show in the runs? See the inline comment for a cheap way to avoid it in the common case. **Nits** - `sparse_tl` is created outside `dev.present_mu`, but `memory_sparse_wait` reads it under that lock on another thread (inline). - The "stays in system memory" log's pool size now prints `take` (inline). - `tests/test_sparse_mirror.py` could use `{cxx}` rather than `g++` (inline). Still to do before `pr ready`, as the description already says: the Longshot A/B at named commits (rebased on current main), the full `run_all.py`, and the AMD, Intel and Linux test requests.
docs/development.md Outdated
@ -240,6 +240,7 @@ Pipeline-cache file ownership, temporary storage and failure behavior are descri
| `RX_VK_NATIVE_FADD=0` or `1` | The shaders' float adds to storage buffers: 1 the GPU's own atomic where it has `VK_EXT_shader_atomic_float`'s `shaderBufferFloat32AtomicAdd`, 0 a compare-exchange loop on every GPU, the extension not enabled. By default the GPU's own on NVIDIA's driver only, the one it was tried on in the game; on AMD and Intel the loop until runs there show the native add works (the log's `vk: float atomic add on storage buffers:` line says which, and why). The loop gives up after 1024 attempts, and an add that gives up is lost |
| `RX_VK_UB_WORD_HASH=0` | A/B test: use the old low-address-bit index for cached uniform words (default: mix upper address bits to reduce aligned-buffer collisions) |
| `RX_VK_MIRROR=0`, `RX_VK_MIRROR_MB=<MB>` | No VRAM mirrors of the game's memory pools (on for discrete cards); their budget |
| `RX_VK_MIRROR_SPARSE=1` | Sparse VRAM mirrors (`gpu/vk/sparse_mirror.h`): memory only for a mirror's 64 KB blocks that one of the game's pools is in, and blocks the game's pools have left given back. A block with none is reached in guest memory (the page table, and a binding until the next submission gives it memory). Needs strict sparse residency for buffers; off by default |
HumanBean commented 2026-11-14 00:40:37 +00:00
Owner

RX_VK_SPARSE_WAIT=1 (wait for each sparse bind on the CPU, as before) is read in sparse_bind_infos and named in the description's flag table, but it has no row in this switch reference. The Flags noted, and the switch reference updated box isn't true until it does.

`RX_VK_SPARSE_WAIT=1` (wait for each sparse bind on the CPU, as before) is read in `sparse_bind_infos` and named in the description's flag table, but it has no row in this switch reference. The *Flags noted, and the switch reference updated* box isn't true until it does.
docs/tested-configurations.md Outdated
@ -40,6 +40,10 @@ otherwise.
| 2026-11-12 | Same renderer and host; off run 2 | Windows 11 Home, build 26300 | RTX 4070 12 GB, NVIDIA 621.01 | i7-13700F, 32 GB | Same scene/settings as off run 1 | Same copied save, cold start, 30 s standing | Reproduced 0/600 NVIDIA FG sets and 600/600 missing-input fallbacks |
| 2026-11-12 | Same renderer and host; on run 2 | Windows 11 Home, build 26300 | RTX 4070 12 GB, NVIDIA 621.01 | i7-13700F, 32 GB | Same scene/settings as on run 1 | Same copied save, cold start, 30 s standing; then Vanilla/PsychoV35 switches and a camera turn | Reproduced 600/600 DLSS evaluations and NVIDIA FG sets in steady intervals; tone-mapper switching returned to RGBA16F, history reset; actor motion collected |
| 2026-11-12 | feat/auto-preset (the Auto (this PC) branch, on main), game and renderer built from it | Ubuntu 26.04.1, kernel 7.0, a private KWin virtual output 2560x1440 at 60 Hz (headless runs, scripted input) | RTX 4070 Ti 12 GB, 595.99.02 | Ryzen 7 9800X3D, 32 GB | Quality preset Auto (this PC); frame generation off; caps 60 and 90 (`HOS_FPS_MAX`), and 4K output set by hand for the step-down run | The test save's spawn point, then Kokiri Forest and Hyrule Field | First start: tier 4 (High), DLSS Quality, output 2560x1440. Same PC again: kept. `RX_VK_AUTO_PRESET_VRAM_MB=6144`: Low, DLSS Performance, output 2048x1152; back to 12 GB: High again. A row changed: Custom, and Auto left it. In play: 1440p at 60, the GPU at 8.4 of 16.7 ms: kept; 4K at a 90 cap, frames missed (about 80 fps), the GPU at 10.6 of 11.1 ms: 67% -> 58% -> 50%; 8 GB faked at 60, the GPU at 8.3 ms: Balanced (58%) -> Quality (67%). No validation, device-lost or "not made" line |
| 2026-11-12 | v2.1.0-rc.3-249-g564ea648 and later builds of this change on main c544828f (unmerged then); the last v2.1.0-rc.3 on 6dcc15c7 | Windows 11, no window (unattended, scripted input) | RTX 4070 SUPER 12 GB, 617.42 | Ryzen 9 9950X3D (SMT off), 32 GB | 2560x1440, DLAA, `RX_VK_COMPACT=1`, `RX_VK_COMPACT_PCT=85`, `RX_VK_ASTC_BC_ALL=1`, `RX_VK_IMAGE_BLOCK_MB=32`; nothing else running, `RX_VK_TEST_ALLOWED_MB=6144`, `RX_VK_MIRROR_MB=1536`; `RX_VK_MIRROR_SPARSE=1` against unset | Hyrule Field, a scripted five-minute walk | Three pairs: 57.3 / 55.3 / 58.2 fps against 47.7 / 51.5 / 49.9; frames over 25 ms 625 / 1,160 / 408 against 3,042 / 1,840 / 2,114; VRAM peak 6,394 / 6,200 / 6,387 MB against 7,327 / 6,254 / 6,444. No device lost |
HumanBean commented 2026-11-14 00:40:38 +00:00
Owner

These rows say Ryzen 9 9950X3D (SMT off), but the PR's Measurements say i7-14700K / Windows 11 Pro 26200 for the same runs. The builds don't line up either: 564ea648 on main c544828f here, Hookshot 130ee5e6 in the description, 990d477d in the commit message. Which one is right? The rows and the description should name the same PC and commits.

These rows say Ryzen 9 9950X3D (SMT off), but the PR's Measurements say i7-14700K / Windows 11 Pro 26200 for the same runs. The builds don't line up either: `564ea648` on main `c544828f` here, Hookshot `130ee5e6` in the description, `990d477d` in the commit message. Which one is right? The rows and the description should name the same PC and commits.
gpu/vk/vk_memory.cpp Outdated
@ -2504,0 +2589,4 @@
bool sparse_bind_infos(std::vector<VkSparseBufferMemoryBindInfo>& infos, uint64_t* signaled = nullptr) {
if (infos.empty()) return true;
const auto t0 = std::chrono::steady_clock::now();
if (!sparse_tl) {
HumanBean commented 2026-11-14 00:40:37 +00:00
Owner

Nit: sparse_tl is created here under pools_mu only. memory_sparse_wait reads it under dev.present_mu from the replay's submit paths, and !sparse_tl is evaluated before v. The first bind is therefore a formal data race on the handle. Creating it inside the present_mu block, or at device init, would close it.

Nit: `sparse_tl` is created here under pools_mu only. `memory_sparse_wait` reads it under `dev.present_mu` from the replay's submit paths, and `!sparse_tl` is evaluated before `v`. The first bind is therefore a formal data race on the handle. Creating it inside the `present_mu` block, or at device init, would close it.
gpu/vk/vk_memory.cpp Outdated
@ -3182,6 +3433,23 @@ BufferRef gpu_buffer_bind(uint64_t gpu, uint64_t size, bool written) {
note_in_place_read();
return remember({p->buffer, off, size, 0});
}
if (p->sparse && !sparse_has(*p, off, size)) {
HumanBean commented 2026-11-14 00:40:37 +00:00
Owner

A mixed binding: some of the blocks it reaches have memory and some don't (st_sparse_mixed). It is served wholly from guest memory. For the blocks with memory, the mirror can be newer than guest memory: stores through the page table land only in the mirror and aren't written back (the case retire_pool's EvilOOT inheritance exists for). A read here misses them. A write is copied guest -> mirror by the next submission's refill over the whole range, which overwrites that GPU-only data with stale guest bytes. How often did this happen in the runs?

The common case is a binding with a guessed size that runs past a pool's end into a block no game pool is in. In that case the mirror can serve the binding as it is, because the bytes past the pool read 0 (strict residency) and nothing meaningful is there. Something like this: if every block without memory in the span is outside all game pools (live_blocks), return the mirror and don't want those blocks. Only a true mixed case inside a pool would then fall back to guest memory, and that one might be worth a synchronous bind instead.

A mixed binding: some of the blocks it reaches have memory and some don't (`st_sparse_mixed`). It is served wholly from guest memory. For the blocks with memory, the mirror can be newer than guest memory: stores through the page table land only in the mirror and aren't written back (the case retire_pool's EvilOOT inheritance exists for). A read here misses them. A write is copied guest -> mirror by the next submission's refill over the whole range, which overwrites that GPU-only data with stale guest bytes. How often did this happen in the runs? The common case is a binding with a guessed size that runs past a pool's end into a block no game pool is in. In that case the mirror can serve the binding as it is, because the bytes past the pool read 0 (strict residency) and nothing meaningful is there. Something like this: if every block without memory in the span is outside all game pools (`live_blocks`), return the mirror and don't want those blocks. Only a true mixed case inside a pool would then fall back to guest memory, and that one might be worth a synchronous bind instead.
gpu/vk/vk_memory.cpp Outdated
@ -3673,3 +4007,3 @@
if (logged++ < 8)
logf("vk: pool %#llx (%llu MB) stays in system memory: the mirrors have %llu MB of %llu", (unsigned long long)p->lo,
(unsigned long long)(n >> 20), (unsigned long long)(mirror_bytes >> 20),
(unsigned long long)(take >> 20), (unsigned long long)(mirror_bytes >> 20),
HumanBean commented 2026-11-14 00:40:37 +00:00
Owner

Nit: the message is "pool %#llx (%llu MB)", which reads as the pool's size, but it now prints take. For a sparse mirror that is the blocks with memory, not n. Maybe print both ("%llu MB, %llu with memory").

Nit: the message is "pool %#llx (%llu MB)", which reads as the pool's size, but it now prints `take`. For a sparse mirror that is the blocks with memory, not `n`. Maybe print both ("%llu MB, %llu with memory").
tests/test_sparse_mirror.py Outdated
@ -0,0 +11,4 @@
import tempfile
from pathlib import Path
RUN_ALL_ARGS = ["--cxx", "g++"]
HumanBean commented 2026-11-14 00:40:38 +00:00
Owner

Nit: RUN_ALL_ARGS = ["--cxx", "{cxx}"] lets run_all use the C++ compiler it finds, clang++ under llvm-mingw for one. A hard-coded g++ is skipped on a PC that only has clang.

Nit: `RUN_ALL_ARGS = ["--cxx", "{cxx}"]` lets run_all use the C++ compiler it finds, clang++ under llvm-mingw for one. A hard-coded `g++` is skipped on a PC that only has clang.
HumanBean force-pushed perf/sparse-mirror from 8cf52d3f44
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 19s
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 ad887e4b35
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Details
Third-party parts / The same as upstream (pull_request) Has been skipped
Details
Website / Website on Cloudflare (pull_request) Has been skipped
Details
CI / Conventional Commit title (pull_request) Successful in 5s
Details
CI / Workflow lint (actionlint) (pull_request) Successful in 7s
Details
Third-party parts / As their notes say (pull_request) Successful in 14s
Details
PR description / Description follows the template (pull_request_target) Successful in 3s
Details
2026-11-14 02:00:32 +00:00 Compare
HumanBean commented 2026-11-14 02:02:06 +00:00
Owner

Rebased and changed by HumanBean's agent (commit ad887e4b on top of the author's, rebased linearly onto main 76699aad; the author's commit is unchanged apart from the tested-configurations rows, which moved). Nothing was built, checked or run here: no compiler, no run_all.py, no game.

On by default, per the maintainer. RX_VK_MIRROR_SPARSE is on unless =0 (or off, false, no); it still turns itself off, logged, where the device lacks strict sparse residency for buffers. The code comments, docs/development.md, tests/test_sparse_mirror.py's check of mirror_sparse_on, the title and the description's Flags, Measurements and Risk say so. The README never named it.

The review's points (#250 (comment)):

  1. RX_VK_SPARSE_WAIT=1 has its row in docs/development.md's switch reference now, next to RX_VK_MIRROR_SPARSE=0.
  2. Hardware and commits: not settled, @GummiNaomi please confirm. The description said i7-14700K, Windows 11 Pro 26200, Hookshot 130ee5e6; the first commit's message says 990d477d; the rows say Ryzen 9 9950X3D (SMT off), v2.1.0-rc.3-249-g564ea648 and later on Hookshot main c544828f. The rows' PC (RTX 4070 SUPER, 617.42, 9950X3D) matches many other frozen rows, so the new docs/tested/ files copy the rows as they were; the description marks it "to be confirmed". Which CPU and which builds were these runs?
  3. Boxes: Host code builds on both... and Docs updated are there; the first stays unticked (not built at this head).
  4. https://github.com/NotRexed/Hookshot/pull/2794 is a full URL in the description. The author's commit message wasn't rewritten.
  5. Mixed bindings: gpu_buffer_bind now serves a binding from the mirror when every block it reaches with no memory lies outside every game pool (sparse_gaps_outside_pools, from live_blocks over the binding's blocks: the guessed size run past a pool's end). Those blocks read 0 and drop writes, nothing of the game's is there, and the write-back is already clipped to blocks with memory. Those blocks aren't wanted. A binding that reaches a block with no memory inside a pool is still served from guest memory, as before. st_sparse_mixed now counts reads as well as writes, and the log line adds from the mirror past every game pool (st_sparse_past). No synchronous bind for the in-pool case.
  • Nit: sparse_tl is made inside sparse_bind_infos's dev.present_mu block, so memory_sparse_wait reads it under the same lock it was written under.
  • Nit: "stays in system memory" prints (<pool> MB, its mirror <take> MB).
  • Nit: tests/test_sparse_mirror.py uses RUN_ALL_ARGS = ["--cxx", "{cxx}"].

docs/tested move. After #96, docs/tested-configurations.md is frozen: the four rows are gone from it, and each run is a file of its own, self-contained (no "as above"): docs/tested/2026-11-12-sparse-mirror-gumminaomi-run01.md to run04.md.

Also rebased over #248 (the late pages fixed on the GPU), without conflicts. That combination hasn't been run either.

Still needed before ready: the Longshot A/B at named commits with no switch set (the new default) against RX_VK_MIRROR_SPARSE=0 or main, the full run_all.py, a Linux build, an answer on point 2, and runs on AMD, Intel and Linux at 6-8 GB (test requests to open).

Rebased and changed by HumanBean's agent (commit ad887e4b on top of the author's, rebased linearly onto main 76699aad; the author's commit is unchanged apart from the tested-configurations rows, which moved). Nothing was built, checked or run here: no compiler, no `run_all.py`, no game. **On by default, per the maintainer.** `RX_VK_MIRROR_SPARSE` is on unless `=0` (or `off`, `false`, `no`); it still turns itself off, logged, where the device lacks strict sparse residency for buffers. The code comments, `docs/development.md`, `tests/test_sparse_mirror.py`'s check of `mirror_sparse_on`, the title and the description's Flags, Measurements and Risk say so. The README never named it. The review's points (https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/250#issuecomment-15529): 1. `RX_VK_SPARSE_WAIT=1` has its row in `docs/development.md`'s switch reference now, next to `RX_VK_MIRROR_SPARSE=0`. 2. Hardware and commits: **not settled, @GummiNaomi please confirm.** The description said i7-14700K, Windows 11 Pro 26200, Hookshot `130ee5e6`; the first commit's message says `990d477d`; the rows say Ryzen 9 9950X3D (SMT off), `v2.1.0-rc.3-249-g564ea648` and later on Hookshot main `c544828f`. The rows' PC (RTX 4070 SUPER, 617.42, 9950X3D) matches many other frozen rows, so the new `docs/tested/` files copy the rows as they were; the description marks it "to be confirmed". Which CPU and which builds were these runs? 3. Boxes: *Host code builds on both...* and *Docs updated* are there; the first stays unticked (not built at this head). 4. https://github.com/NotRexed/Hookshot/pull/2794 is a full URL in the description. The author's commit message wasn't rewritten. 5. Mixed bindings: `gpu_buffer_bind` now serves a binding from the mirror when every block it reaches with no memory lies outside every game pool (`sparse_gaps_outside_pools`, from `live_blocks` over the binding's blocks: the guessed size run past a pool's end). Those blocks read 0 and drop writes, nothing of the game's is there, and the write-back is already clipped to blocks with memory. Those blocks aren't wanted. A binding that reaches a block with no memory inside a pool is still served from guest memory, as before. `st_sparse_mixed` now counts reads as well as writes, and the log line adds `from the mirror past every game pool` (`st_sparse_past`). No synchronous bind for the in-pool case. - Nit: `sparse_tl` is made inside `sparse_bind_infos`'s `dev.present_mu` block, so `memory_sparse_wait` reads it under the same lock it was written under. - Nit: "stays in system memory" prints `(<pool> MB, its mirror <take> MB)`. - Nit: `tests/test_sparse_mirror.py` uses `RUN_ALL_ARGS = ["--cxx", "{cxx}"]`. **docs/tested move.** After https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/96, `docs/tested-configurations.md` is frozen: the four rows are gone from it, and each run is a file of its own, self-contained (no "as above"): `docs/tested/2026-11-12-sparse-mirror-gumminaomi-run01.md` to `run04.md`. Also rebased over https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/248 (the late pages fixed on the GPU), without conflicts. That combination hasn't been run either. **Still needed before ready:** the Longshot A/B at named commits with no switch set (the new default) against `RX_VK_MIRROR_SPARSE=0` or main, the full `run_all.py`, a Linux build, an answer on point 2, and runs on AMD, Intel and Linux at 6-8 GB (test requests to open).
HumanBean changed title from WIP: perf(vk): sparse VRAM mirrors, memory only where a game pool is (RX_VK_MIRROR_SPARSE=1) to WIP: perf(vk): sparse VRAM mirrors, memory only where a game pool is (on by default) 2026-11-14 02:02:10 +00:00
HumanBean requested changes 2026-11-14 02:09:13 +00:00
HumanBean left a comment

Re-review of ad887e4b (the author's a57b2458 rebased onto main 76699aad, plus our fix commit), by reading only: nothing built or run. One bug from the rebase over #248, so this stays REQUEST_CHANGES and replaces #250 (comment).

Bug: the sparse fill copies out of capture buffers without late_fix_emit(). Since #248, a taken capture can hold late pages whose fix isn't recorded yet. Its contract (the comment above late_fix_on, and late_fix_emit's) is that every GPU reader of a capture's buffer calls late_fix_emit() first: mirror_sync and mirror_make do. sparse_grow_all is a third reader (cap_pages_over(p, ...) then vkCmdCopyBuffer(up, buf, p.mirror, ...), vk_memory.cpp ~4948-4960), written before #248 and not updated. A late page in a fill range gets its pre-write-back words into the mirror: the words that write-back put in guest memory go stale in VRAM. The refill ranges are where this can happen, because they cover blocks that already have memory and are written back by the diff pass. With sparse on by default this is everyone's path. Fix: late_fix_emit(); before the over copies (inside if (!over.empty()), as mirror_make does), and add "late_fix_emit();" to test_sparse_mirror.py's sparse_grow_all() needles, ordered before cap_pages_over(p,. Inline below.

The rest checks out:

  1. Bindings past a pool's end (sparse_gaps_outside_pools): correct. It runs under pools_mu, which gpu_buffer_bind holds there and guest_pools.swap takes. live_blocks gets the binding's blocks block-aligned from p.lo, so live[i - a] is block i's. The i - a < live.size() guard covers a last block past p.hi (has.size() from sparse_size can run past it); it is treated as outside every pool, which it is. e <= a and hi <= lo return early. A memory-less block that no game pool touches holds nothing of the game's. With residencyNonResidentStrict it reads 0 and drops writes. Bytes past a pool's end are garbage either way (a whole mirror had stale fill there), and the write-back is already clipped to blocks with memory. A block a pool touches at all is live, so it takes the guest path. The lock-free read path still requires sparse_has, so it never hands one out.
  2. On by default: !v || !env_value_off(v) is on when unset; "", 0, false, off, no (any case, trimmed) turn it off, the same as RX_VK_SHADOW_STAGE and RX_VK_LATE_FIX. Nothing else assumed opt-in. The sparse block in memory_begin_submission sits after if (!mirror_on()) return;, so iGPUs (Steam Deck), GCN and RX_VK_MIRROR=0 never reach it or the log line. The budget counts resident_bytes through mirror_size. No other source, profile or test reads the switch.
  3. #248 and the shadows sparse_fill forgets: no problem. shadow_forget only clears shadow_known bits, the same as for in-place writes, odd runs and a busy chunk table. The next write-back over those pages fills them from guest memory in its own command buffer before the diff pass. late_fix_build/late_fix_emit check that the shadow is still lp.shadow. Nothing new here for the shadows. The missing late_fix_emit above is the real #248 interaction.
  4. docs/tested/: all four files pass README.md's and tested_configurations.py's rules. Each is three lines with eight filled cells, the date matches the name, and the names have at least three lowercase parts. No "as above", "same host" or "ditto", and they don't repeat frozen rows. docs/tested-configurations.md is byte-for-byte main's.
  5. Rebase: a57b2458's added and removed lines are 8cf52d3f's exactly, apart from the four removed tested-configurations rows. Only the hunk context differs (main's new lines, such as late_fix_emit() in mirror_make, are kept). Nothing of main is reverted. By reading, the C++ compiles: st_sparse_past is declared both in vk_memory.cpp and in test_direct_buffer_writes' stubs (with GuestPool/guest_pools), the new branch is if ... else if, and the semaphore is made under dev.present_mu. The checks' needles match the new text: "if (p->sparse && !sparse_has(*p, off, size)) {" still matches inside } else if (...), after past.

CI on ad887e4b: title, actionlint, third-party notes and the description check pass. The Python source checks (both parts), the SPIR-V fixtures and the third-party diff were skipped, so no check of the code ran on this head.

Nits (not blocking): sparse_gaps_outside_pools allocates a vector per call on the locked path (fine unless st_sparse_past turns out large). A written past-the-end binding now trips the write-back's "reached blocks with no memory: left out" warning (logged 8 times), which reads like a fault for an expected case.

Still needed before ready, after the fix above: AMD (RADV and Windows), Intel and Linux runs at 6-8 GB with the default; a Longshot A/B with no switch set against RX_VK_MIRROR_SPARSE=0 or main, with docs/tested files; run_all.py; a Linux build; and @GummiNaomi confirming the hardware and builds of the Hookshot runs.

Re-review of ad887e4b (the author's a57b2458 rebased onto main 76699aad, plus our fix commit), by reading only: nothing built or run. One bug from the rebase over https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/248, so this stays REQUEST_CHANGES and replaces https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/250#issuecomment-15529. **Bug: the sparse fill copies out of capture buffers without `late_fix_emit()`.** Since #248, a taken capture can hold late pages whose fix isn't recorded yet. Its contract (the comment above `late_fix_on`, and `late_fix_emit`'s) is that every GPU reader of a capture's buffer calls `late_fix_emit()` first: `mirror_sync` and `mirror_make` do. `sparse_grow_all` is a third reader (`cap_pages_over(p, ...)` then `vkCmdCopyBuffer(up, buf, p.mirror, ...)`, vk_memory.cpp ~4948-4960), written before #248 and not updated. A late page in a fill range gets its pre-write-back words into the mirror: the words that write-back put in guest memory go stale in VRAM. The refill ranges are where this can happen, because they cover blocks that already have memory and are written back by the diff pass. With sparse on by default this is everyone's path. Fix: `late_fix_emit();` before the `over` copies (inside `if (!over.empty())`, as `mirror_make` does), and add `"late_fix_emit();"` to `test_sparse_mirror.py`'s `sparse_grow_all()` needles, ordered before `cap_pages_over(p,`. Inline below. **The rest checks out:** 1. **Bindings past a pool's end** (`sparse_gaps_outside_pools`): correct. It runs under `pools_mu`, which `gpu_buffer_bind` holds there and `guest_pools.swap` takes. `live_blocks` gets the binding's blocks block-aligned from `p.lo`, so `live[i - a]` is block i's. The `i - a < live.size()` guard covers a last block past `p.hi` (`has.size()` from `sparse_size` can run past it); it is treated as outside every pool, which it is. `e <= a` and `hi <= lo` return early. A memory-less block that no game pool touches holds nothing of the game's. With `residencyNonResidentStrict` it reads 0 and drops writes. Bytes past a pool's end are garbage either way (a whole mirror had stale fill there), and the write-back is already clipped to blocks with memory. A block a pool touches at all is live, so it takes the guest path. The lock-free read path still requires `sparse_has`, so it never hands one out. 2. **On by default**: `!v || !env_value_off(v)` is on when unset; `""`, `0`, `false`, `off`, `no` (any case, trimmed) turn it off, the same as `RX_VK_SHADOW_STAGE` and `RX_VK_LATE_FIX`. Nothing else assumed opt-in. The sparse block in `memory_begin_submission` sits after `if (!mirror_on()) return;`, so iGPUs (Steam Deck), GCN and `RX_VK_MIRROR=0` never reach it or the log line. The budget counts `resident_bytes` through `mirror_size`. No other source, profile or test reads the switch. 3. **#248 and the shadows `sparse_fill` forgets**: no problem. `shadow_forget` only clears `shadow_known` bits, the same as for in-place writes, odd runs and a busy chunk table. The next write-back over those pages fills them from guest memory in its own command buffer before the diff pass. `late_fix_build`/`late_fix_emit` check that the shadow is still `lp.shadow`. Nothing new here for the shadows. The missing `late_fix_emit` above is the real #248 interaction. 4. **docs/tested/**: all four files pass README.md's and `tested_configurations.py`'s rules. Each is three lines with eight filled cells, the date matches the name, and the names have at least three lowercase parts. No "as above", "same host" or "ditto", and they don't repeat frozen rows. `docs/tested-configurations.md` is byte-for-byte main's. 5. **Rebase**: a57b2458's added and removed lines are 8cf52d3f's exactly, apart from the four removed tested-configurations rows. Only the hunk context differs (main's new lines, such as `late_fix_emit()` in `mirror_make`, are kept). Nothing of main is reverted. By reading, the C++ compiles: `st_sparse_past` is declared both in vk_memory.cpp and in test_direct_buffer_writes' stubs (with `GuestPool`/`guest_pools`), the new branch is `if ... else if`, and the semaphore is made under `dev.present_mu`. The checks' needles match the new text: `"if (p->sparse && !sparse_has(*p, off, size)) {"` still matches inside `} else if (...)`, after `past`. **CI on ad887e4b**: title, actionlint, third-party notes and the description check pass. The Python source checks (both parts), the SPIR-V fixtures and the third-party diff were **skipped**, so no check of the code ran on this head. Nits (not blocking): `sparse_gaps_outside_pools` allocates a vector per call on the locked path (fine unless `st_sparse_past` turns out large). A written past-the-end binding now trips the write-back's "reached blocks with no memory: left out" warning (logged 8 times), which reads like a fault for an expected case. **Still needed before ready**, after the fix above: AMD (RADV and Windows), Intel and Linux runs at 6-8 GB with the default; a Longshot A/B with no switch set against `RX_VK_MIRROR_SPARSE=0` or main, with docs/tested files; `run_all.py`; a Linux build; and @GummiNaomi confirming the hardware and builds of the Hookshot runs.
gpu/vk/vk_memory.cpp
@ -4433,0 +4947,4 @@
for (const VkBufferCopy& c : fill) shadow_forget(p, c.dstOffset, c.size);
std::map<VkBuffer, std::vector<VkBufferCopy>> over;
for (const VkBufferCopy& c : fill) cap_pages_over(p, p.lo + c.dstOffset, p.lo + c.dstOffset + c.size, over);
if (!over.empty()) {
HumanBean commented 2026-11-14 02:09:13 +00:00
Owner

Missing late_fix_emit(); here, before the copies out of the capture buffers. Since #248, a taken capture's late pages hold their pre-write-back words until the fix is recorded, and every GPU reader of a capture buffer calls late_fix_emit() first (mirror_sync, and mirror_make at its own if (!over.empty()) {). Without it, a late page in a fill range (a refill covers blocks that have memory and are written back by the diff pass) puts stale words into the mirror. Suggest:

if (!over.empty()) {
    late_fix_emit();   // (the late pages' fix, before the copies out of a capture's buffer below)

and "late_fix_emit();" among test_sparse_mirror.py's sparse_grow_all() needles, ordered before cap_pages_over(p,.

Missing `late_fix_emit();` here, before the copies out of the capture buffers. Since https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/248, a taken capture's late pages hold their pre-write-back words until the fix is recorded, and every GPU reader of a capture buffer calls `late_fix_emit()` first (`mirror_sync`, and `mirror_make` at its own `if (!over.empty()) {`). Without it, a late page in a fill range (a refill covers blocks that have memory and are written back by the diff pass) puts stale words into the mirror. Suggest: ```cpp if (!over.empty()) { late_fix_emit(); // (the late pages' fix, before the copies out of a capture's buffer below) ``` and `"late_fix_emit();"` among `test_sparse_mirror.py`'s `sparse_grow_all()` needles, ordered before `cap_pages_over(p,`.
gpu/vk/vk_memory.cpp Outdated
@ -4973,0 +5610,4 @@
if (after != before) {
static int logged;
if (logged++ < 8)
logf("vk: sparse mirror of pool %#llx: %llu bytes of a write-back reached blocks with no memory: left out",
HumanBean commented 2026-11-14 02:09:13 +00:00
Owner

Nit: a written binding that runs past every game pool, now served from the mirror, ends up here as expected. This warning ("reached blocks with no memory: left out") reads like a fault. Maybe skip it when every left-out block is outside every game pool, or reword it.

Nit: a written binding that runs past every game pool, now served from the mirror, ends up here as expected. This warning ("reached blocks with no memory: left out") reads like a fault. Maybe skip it when every left-out block is outside every game pool, or reword it.
fix(vk): the sparse fill applies the late pages' fix before it reads the captures
Some checks failed
Website / Website on Cloudflare (pull_request_target) Has been skipped
Details
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
Details
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
Details
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Details
Third-party parts / The same as upstream (pull_request) Has been skipped
Details
Website / Website on Cloudflare (pull_request) Has been skipped
Details
CI / Conventional Commit title (pull_request) Successful in 10s
Details
CI / Workflow lint (actionlint) (pull_request) Successful in 14s
Details
Third-party parts / As their notes say (pull_request) Successful in 18s
Details
PR description / Description follows the template (pull_request_target) Failing after 12s
Details
00c7fd2482 
sparse_grow_all copies captured pages out of the capture buffers into a
sparse mirror's new blocks; since PR 248 every GPU reader of a capture
buffer records late_fix_emit() first, as mirror_sync and mirror_make do.
Also: sparse_gaps_outside_pools reuses its block list instead of
allocating one per call, and the write-back warns about blocks with no
memory only where a game pool is (past every pool is expected).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
HumanBean commented 2026-11-14 02:13:57 +00:00
Owner

Pushed 00c7fd24 (on main 76699aad), for the re-review's finding (comment, inline):

  • Fix: sparse_grow_all now calls late_fix_emit() inside if (!over.empty()) {, before the copies out of the capture buffers, as mirror_make does: since #248 every GPU reader of a capture buffer records the late pages' fix first (same upload command buffer, pools_mu held at the submission's start; its own full barrier after the fix covers the copies).
  • Check: tests/test_sparse_mirror.py's sparse_grow_all() check needs late_fix_emit(); and the capture copy, and checks the order cap_pages_over(p, < late_fix_emit( < vkCmdCopyBuffer(up, buf, p.mirror (the fix sits after the call that builds the copy list, like mirror_make, so it is ordered against the copies themselves). Nothing was taken out.
  • Nits (inline): sparse_gaps_outside_pools reuses a thread_local block list (a new live_blocks overload in sparse_mirror.h fills a given vector; the returning one calls it), its check's needle updated. The write-back's "reached blocks with no memory" line is logged only when one of the clipped runs reaches a block with no memory where a game pool is (sparse_gaps_outside_pools false); past every pool, the expected case, is silent. A needle for that too.

Nothing was built or run here (no checks, no game): the change was made by reading only. The description's What changed and Not tested now name this head.

Pushed 00c7fd24 (on main 76699aad), for the re-review's finding ([comment](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/250#issuecomment-16672), [inline](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/250#issuecomment-16669)): - **Fix:** `sparse_grow_all` now calls `late_fix_emit()` inside `if (!over.empty()) {`, before the copies out of the capture buffers, as `mirror_make` does: since [#248](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/248) every GPU reader of a capture buffer records the late pages' fix first (same upload command buffer, `pools_mu` held at the submission's start; its own full barrier after the fix covers the copies). - **Check:** `tests/test_sparse_mirror.py`'s `sparse_grow_all()` check needs `late_fix_emit();` and the capture copy, and checks the order `cap_pages_over(p,` < `late_fix_emit(` < `vkCmdCopyBuffer(up, buf, p.mirror` (the fix sits after the call that builds the copy list, like `mirror_make`, so it is ordered against the copies themselves). Nothing was taken out. - **Nits** ([inline](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/250#issuecomment-16671)): `sparse_gaps_outside_pools` reuses a `thread_local` block list (a new `live_blocks` overload in `sparse_mirror.h` fills a given vector; the returning one calls it), its check's needle updated. The write-back's "reached blocks with no memory" line is logged only when one of the clipped runs reaches a block with no memory where a game pool is (`sparse_gaps_outside_pools` false); past every pool, the expected case, is silent. A needle for that too. Nothing was built or run here (no checks, no game): the change was made by reading only. The description's What changed and Not tested now name this head.
HumanBean changed title from WIP: perf(vk): sparse VRAM mirrors, memory only where a game pool is (on by default) to perf(vk): sparse VRAM mirrors, memory only where a game pool is (on by default) 2026-11-14 02:14:32 +00:00
HumanBean deleted branch perf/sparse-mirror 2026-11-14 02:15:00 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
HumanBean
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
v3.0.0
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!250
No description provided.