WIP: perf(vk): no pool import for one GPU unswizzle, and the CPU unswizzle threaded from 4 MB #335

Draft
DUDUYG wants to merge 3 commits from DUDUYG/Longshot:perf/unswizzle-no-pool-import into main
First-time contributor

All credit to @FIocker. This is their NotRexed/Hookshot PR 2785 (Hookshot commit 5192732a). 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; it applies to Longshot's main without changes. The tests, docs and runs on top are mine, from the review. FIocker, if you'd rather open it yourself, say so and I'll close this one.

Why

Turning the camera toward something new, or entering an area, could hitch for tens of milliseconds while textures uploaded. Two things on the replay thread caused it:

  • a GPU unswizzle imported the guest memory under one texture just for that upload, and each such import took 22-25 ms;
  • screen-size render targets were unswizzled on one CPU thread, about 20 ms each.

What changed

From the original PR (FIocker's commit, 17a3ef23 here):

  • gpu/vk/vk_texture.cpp upload_unswizzle_gpu and gpu/vk/vk_memory.cpp memory_guest_contained: the GPU path is taken only where one already-imported pool holds the whole range. Everything else takes the CPU path, so an upload no longer imports a pool for itself.
  • gpu/vk/vk_texture.cpp upload_inner: the CPU unswizzle is threaded from 4 MB, not 16 MB. A 15 MB scene-size level takes a few ms on several threads instead of ~20 ms on one.
  • gpu/vk/vk_internal.h: the declaration of memory_guest_contained.

Added for the review (46cbcc94, 3344ade7):

  • tests/test_unswizzle_upload.py and tests/unswizzle_upload_test.cpp: the choice and the threaded rows cut out of vk_texture.cpp and run against a model of guest memory (see Testing).
  • docs/development.md: RX_VK_UNSWIZZLE_IMPORT in the switch reference.
  • docs/tested/2026-11-14-market-entry-duduyg-{main,pr335}-run0{1,2,3}.md: the six runs below. The three main files are the same files in PR 334, 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_UNSWIZZLE_IMPORT=1 Lets a GPU unswizzle import a pool for itself, as before. It restores the imports only: the CPU unswizzle stays threaded from 4 MB, so it is not an off switch for the whole PR (revert the commit for that) off Off RTX 5070 (below): no unswizzle import at Market entry - AMD, Intel; CPUs with fewer cores

Measurements

The original author's report (from Hookshot PR 2785): on Windows 11 with an NVIDIA RTX card, turning the camera toward the Deku Tree hitched for about 60 ms, three of those imports in one frame. The hitch was gone with both changes, measured as part of a combined build.

This PR alone, on Longshot's current main, 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/335 @ ddbc5fa7, the same plus FIocker's commit (meas/335). Its code is this branch's 17a3ef23; the tests and docs came after.
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; main's lazy heap on.
Method: one warm-up run of each build, then main, PR 334 and this PR 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 @ ddbc5fa7
vk: slow upload lines per run 3 [3-3] 1 [1-1]
their time in all 102 [79-105] ms 37 [36-40] ms
the slowest upload frame (a frame's uploads took) 137 [135-146] ms 120 [112-121] ms
a second such frame 56 and 55 ms (in 2 of 3 runs) none
textures unswizzled on the GPU in those frames 2 (40.6 MB), and 1 (8.4 MB) in the second 0: all on the CPU
Worst frame 445 [253-458] ms 447 [214-456] ms
Frames over 50 ms 7 [6-9] 8 [5-10]
Frames over 100 ms 4 [2-4] 3 [1-5]
99.9th percentile frame 119 [69-123] ms 73 [55-130] ms
Biggest slow submit 172 [166-174] ms 179 [164-197] ms
Median fps, Market windows 60.0 60.0

What it shows: in every main run, two slow uploads came besides the 12288x12288 R8 one: a 128x128 array texture (28-36 ms) and an ASTC texture (16-33 ms). With this PR neither appears in any run. The one left is the 12288x12288 R8 texture (294 MB) on the CPU path, which this PR doesn't touch. At the Market entry nothing is unswizzled on the GPU any more: no imported pool held those textures, so they all took the CPU path. I haven't traced why the ASTC upload goes too; on main it comes in the frame after the GPU unswizzles. The worst frames are within the run-to-run spread: the load's own frame (445 ms on main) and others this PR doesn't change. So these runs show fewer and shorter upload stalls, but no frame-time gain I can claim.

M1_main  vk: slow upload 35.8 ms: #510 0x114682a000 12288x12288x1 x1 levels 1 NVN 0x32, 294912 of 294912 KB
M1_main  vk: slow upload 35.6 ms: #525 0x10a9230e00 128x128x1 x396 levels 8 NVN 0x4b, 9306 of 9306 KB
M1_main  vk: slow upload 33.1 ms: #1230 0x10c8a3fa00 1024x1024x1 x1 levels 11 NVN 0x8c ASTC (BC3), 561 of 561 KB
M1_main  vk: a frame's uploads took 135 ms: ASTC on the GPU 101 (106.4 MB, 27 ms); unswizzled on the GPU 2 (40.6 MB, 3 ms); on the CPU 119 (502.5 MB, 106 ms)
M1_main  vk: a frame's uploads took 56 ms: ASTC on the GPU 414 (143.0 MB, 47 ms); unswizzled on the GPU 1 (8.4 MB, 0 ms); on the CPU 313 (74.3 MB, 9 ms)
M1_335   vk: slow upload 37.1 ms: #510 0x114682a000 12288x12288x1 x1 levels 1 NVN 0x32, 294912 of 294912 KB
M1_335   vk: a frame's uploads took 112 ms: ASTC on the GPU 65 (88.3 MB, 9 ms); on the CPU 121 (543.1 MB, 103 ms)
M2_335   vk: a frame's uploads took 120 ms: ASTC on the GPU 65 (88.3 MB, 9 ms); on the CPU 121 (543.1 MB, 112 ms)
M3_335   vk: a frame's uploads took 121 ms: ASTC on the GPU 65 (88.3 MB, 8 ms); on the CPU 121 (543.1 MB, 113 ms)

Per run (worst frame / frames over 100 ms / slow uploads): main 445/4/3, 253/2/3, 458/4/3; this PR 456/3/1, 447/5/1, 214/1/1.

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

Testing

Tested:

  • tests/test_unswizzle_upload.py (new). It cuts upload_unswizzle_gpu's choice (with memory_guest_contained) and upload_inner's threaded unswizzle out of vk_texture.cpp and runs them against a model of guest memory, with suyu's real UnswizzleTexture:
    • The choice, each case in its own process: a pool holding the texture, one exactly its range, and a virtual address above 1 TB looked up at its guest address give the GPU unswizzle with no import. No pool, half in one, split between two, a virtual mapping that breaks halfway, and an unmapped one take the CPU path and never reach gpu_buffer. RX_VK_UNSWIZZLE_IMPORT=1 imports as before.
    • The rows: 19 cases compared byte for byte with one call over the whole level, and the threads counted. Just under 4 MB (one thread) and at 4 MB (several); the 2779x1318 scene target (no GOB-multiple width, partial last block row); partial uploads from the middle and to the end; sparse storage gathered by gpu_read; storage ending inside the span; 2 and 3 threads; 1, 8 and 16-byte texels.
    • Against main's code (--source) it fails as it should: six of the choice's cases reach for a pool (five import one), and 15 levels of 4-16 MB run on one thread.
  • 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. No texture artifacts were looked for on screen: the runs are scripted.

Not tested:

  • AMD and Intel GPUs, and CPUs with fewer cores: the CPU path's extra cost where hardware_concurrency()/2 gives 2-3 threads is unmeasured.
  • Linux.
  • A visual comparison of the textures that now take the CPU path.

Needs other testers:

  • AMD or Intel GPU on Windows, or a 4-6 core CPU: Market entry and camera turns, main against this PR, expecting fewer vk: slow upload lines, no texture artifacts and no worse hitch on fewer cores (T-EEPM; it waits for a maintainer's "testing wanted" label).

Screenshots

Nothing on screen.

Risk and rollback

Textures that used to take the GPU unswizzle now take the CPU path when no imported pool holds them. That costs CPU time on worker threads, but no import. On a CPU with few cores the threaded path has fewer threads to spread over (unmeasured, see Testing). RX_VK_UNSWIZZLE_IMPORT=1 restores the imports; reverting the commit restores the 16 MB threshold too.

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).
  • 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.
  • 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 @FIocker.** This is their NotRexed/Hookshot PR 2785 (Hookshot commit 5192732a). 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; it applies to Longshot's main without changes. The tests, docs and runs on top are mine, from the review. FIocker, if you'd rather open it yourself, say so and I'll close this one. ## Why Turning the camera toward something new, or entering an area, could hitch for tens of milliseconds while textures uploaded. Two things on the replay thread caused it: - a GPU unswizzle imported the guest memory under one texture just for that upload, and each such import took 22-25 ms; - screen-size render targets were unswizzled on one CPU thread, about 20 ms each. ## What changed From the original PR (FIocker's commit, 17a3ef23 here): - `gpu/vk/vk_texture.cpp` `upload_unswizzle_gpu` and `gpu/vk/vk_memory.cpp` `memory_guest_contained`: the GPU path is taken only where one already-imported pool holds the whole range. Everything else takes the CPU path, so an upload no longer imports a pool for itself. - `gpu/vk/vk_texture.cpp` `upload_inner`: the CPU unswizzle is threaded from 4 MB, not 16 MB. A 15 MB scene-size level takes a few ms on several threads instead of ~20 ms on one. - `gpu/vk/vk_internal.h`: the declaration of `memory_guest_contained`. Added for the review (46cbcc94, 3344ade7): - `tests/test_unswizzle_upload.py` and `tests/unswizzle_upload_test.cpp`: the choice and the threaded rows cut out of `vk_texture.cpp` and run against a model of guest memory (see Testing). - `docs/development.md`: `RX_VK_UNSWIZZLE_IMPORT` in the switch reference. - `docs/tested/2026-11-14-market-entry-duduyg-{main,pr335}-run0{1,2,3}.md`: the six runs below. The three main files are the same files in PR 334, 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_UNSWIZZLE_IMPORT=1` | Lets a GPU unswizzle import a pool for itself, as before. It restores the imports only: the CPU unswizzle stays threaded from 4 MB, so it is not an off switch for the whole PR (revert the commit for that) | off | Off | RTX 5070 (below): no unswizzle import at Market entry | - | AMD, Intel; CPUs with fewer cores | ## Measurements The original author's report (from Hookshot PR 2785): on Windows 11 with an NVIDIA RTX card, turning the camera toward the Deku Tree hitched for about 60 ms, three of those imports in one frame. The hitch was gone with both changes, measured as part of a combined build. **This PR alone, on Longshot's current main**, 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/335 @ ddbc5fa7, the same plus FIocker's commit ([meas/335](https://git.magicbean.dev/DUDUYG/Longshot/src/branch/meas/335)). Its code is this branch's 17a3ef23; the tests and docs came after. 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`; main's lazy heap on. Method: one warm-up run of each build, then main, PR 334 and this PR 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 @ ddbc5fa7 | |---|---|---| | `vk: slow upload` lines per run | 3 [3-3] | 1 [1-1] | | their time in all | 102 [79-105] ms | 37 [36-40] ms | | the slowest upload frame (`a frame's uploads took`) | 137 [135-146] ms | 120 [112-121] ms | | a second such frame | 56 and 55 ms (in 2 of 3 runs) | none | | textures unswizzled on the GPU in those frames | 2 (40.6 MB), and 1 (8.4 MB) in the second | 0: all on the CPU | | Worst frame | 445 [253-458] ms | 447 [214-456] ms | | Frames over 50 ms | 7 [6-9] | 8 [5-10] | | Frames over 100 ms | 4 [2-4] | 3 [1-5] | | 99.9th percentile frame | 119 [69-123] ms | 73 [55-130] ms | | Biggest `slow submit` | 172 [166-174] ms | 179 [164-197] ms | | Median fps, Market windows | 60.0 | 60.0 | What it shows: in every main run, two slow uploads came besides the 12288x12288 R8 one: a 128x128 array texture (28-36 ms) and an ASTC texture (16-33 ms). With this PR neither appears in any run. The one left is the 12288x12288 R8 texture (294 MB) on the CPU path, which this PR doesn't touch. At the Market entry nothing is unswizzled on the GPU any more: no imported pool held those textures, so they all took the CPU path. I haven't traced why the ASTC upload goes too; on main it comes in the frame after the GPU unswizzles. The worst frames are within the run-to-run spread: the load's own frame (445 ms on main) and others this PR doesn't change. So these runs show fewer and shorter upload stalls, but no frame-time gain I can claim. ``` M1_main vk: slow upload 35.8 ms: #510 0x114682a000 12288x12288x1 x1 levels 1 NVN 0x32, 294912 of 294912 KB M1_main vk: slow upload 35.6 ms: #525 0x10a9230e00 128x128x1 x396 levels 8 NVN 0x4b, 9306 of 9306 KB M1_main vk: slow upload 33.1 ms: #1230 0x10c8a3fa00 1024x1024x1 x1 levels 11 NVN 0x8c ASTC (BC3), 561 of 561 KB M1_main vk: a frame's uploads took 135 ms: ASTC on the GPU 101 (106.4 MB, 27 ms); unswizzled on the GPU 2 (40.6 MB, 3 ms); on the CPU 119 (502.5 MB, 106 ms) M1_main vk: a frame's uploads took 56 ms: ASTC on the GPU 414 (143.0 MB, 47 ms); unswizzled on the GPU 1 (8.4 MB, 0 ms); on the CPU 313 (74.3 MB, 9 ms) M1_335 vk: slow upload 37.1 ms: #510 0x114682a000 12288x12288x1 x1 levels 1 NVN 0x32, 294912 of 294912 KB M1_335 vk: a frame's uploads took 112 ms: ASTC on the GPU 65 (88.3 MB, 9 ms); on the CPU 121 (543.1 MB, 103 ms) M2_335 vk: a frame's uploads took 120 ms: ASTC on the GPU 65 (88.3 MB, 9 ms); on the CPU 121 (543.1 MB, 112 ms) M3_335 vk: a frame's uploads took 121 ms: ASTC on the GPU 65 (88.3 MB, 8 ms); on the CPU 121 (543.1 MB, 113 ms) ``` Per run (worst frame / frames over 100 ms / slow uploads): main 445/4/3, 253/2/3, 458/4/3; this PR 456/3/1, 447/5/1, 214/1/1. The earlier combined runs (this PR together with Hookshot PR 3043, on 514591a3 before the lazy heap) are in the PR's history; they measured both changes, not this one. ## Testing **Tested:** - `tests/test_unswizzle_upload.py` (new). It cuts `upload_unswizzle_gpu`'s choice (with `memory_guest_contained`) and `upload_inner`'s threaded unswizzle out of `vk_texture.cpp` and runs them against a model of guest memory, with suyu's real `UnswizzleTexture`: - The choice, each case in its own process: a pool holding the texture, one exactly its range, and a virtual address above 1 TB looked up at its guest address give the GPU unswizzle with no import. No pool, half in one, split between two, a virtual mapping that breaks halfway, and an unmapped one take the CPU path and never reach `gpu_buffer`. `RX_VK_UNSWIZZLE_IMPORT=1` imports as before. - The rows: 19 cases compared byte for byte with one call over the whole level, and the threads counted. Just under 4 MB (one thread) and at 4 MB (several); the 2779x1318 scene target (no GOB-multiple width, partial last block row); partial uploads from the middle and to the end; sparse storage gathered by `gpu_read`; storage ending inside the span; 2 and 3 threads; 1, 8 and 16-byte texels. - Against main's code (`--source`) it fails as it should: six of the choice's cases reach for a pool (five import one), and 15 levels of 4-16 MB run on one thread. - 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. No texture artifacts were looked for on screen: the runs are scripted. **Not tested:** - AMD and Intel GPUs, and CPUs with fewer cores: the CPU path's extra cost where `hardware_concurrency()/2` gives 2-3 threads is unmeasured. - Linux. - A visual comparison of the textures that now take the CPU path. **Needs other testers:** - AMD or Intel GPU on Windows, or a 4-6 core CPU: Market entry and camera turns, main against this PR, expecting fewer `vk: slow upload` lines, no texture artifacts and no worse hitch on fewer cores (T-EEPM; it waits for a maintainer's "testing wanted" label). ## Screenshots Nothing on screen. ## Risk and rollback Textures that used to take the GPU unswizzle now take the CPU path when no imported pool holds them. That costs CPU time on worker threads, but no import. On a CPU with few cores the threaded path has fewer threads to spread over (unmeasured, see Testing). `RX_VK_UNSWIZZLE_IMPORT=1` restores the imports; reverting the commit restores the 16 MB threshold too. ## 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. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [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] 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): no pool import for one GPU unswizzle, and the CPU unswizzle threaded from 4 MB
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
c9ead212a9
Turning the camera to the Deku Tree hitched for about 60 ms. Both causes
are in texture uploads, and this PR fixes both. It was split out of
#1834, as Joey-Magnus suggested, and builds on its own on main.

## What it does

- **No pool import for one upload** (`vk_texture.cpp
upload_unswizzle_gpu`, `vk_memory.cpp memory_guest_contained`):
- **Before**: when no imported pool held the guest memory under a
texture, the GPU unswizzle imported it for that one upload. That took
22-25 ms on the replay thread each time, since the driver locks the
pages, and the Deku Tree turn did three in one frame.
- **Now**: the GPU path is taken only where an imported pool already
holds the whole range, and the rest goes to the CPU path.
- `RX_VK_UNSWIZZLE_IMPORT=1` imports as before (read with `env_on`, as
`test_env_on.py` wants).
- **The CPU unswizzle threaded from 4 MB** (`upload_inner`), not 16 MB.
The scene-size targets uploaded at that moment (2779 × 1318 RGBA8, 15
MB) took about 20 ms each on one thread; on several threads, a 15 MB
level takes a few ms.

## Testing

- **Builds**: `vk_texture.cpp` and `vk_memory.cpp` compile with clang
using `nx2/build.py`'s renderer flags.
- **Tests**: `tests/run_all.py`, the whole suite, locally (clang
targeting MSVC, the CPM Vulkan headers): 433 passed. The failures are
exactly the ones main at 51da0464 has on this machine, all down to the
local toolchain.
- **In game**: the 60 ms hitch was measured on Windows 11 with an NVIDIA
RTX card (OoT v131072) and gone with both changes, as part of the
combined build. This head on its own hasn't had a separate run yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: FIocker <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>

Originally NotRexed/Hookshot PR 2785 by FIocker, merged there on 2026-11-13 and not carried into Longshot when it moved
(Hookshot commit 5192732a). Cherry-picked onto Longshot main as it was; author kept.
DUDUYG force-pushed perf/unswizzle-no-pool-import from c9ead212a9
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to c4dd2d01ae
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
2026-11-14 02:16:53 +00:00
Compare
mooki left a comment

Read head c4dd2d01 against main cf605c85: the full diff and description, the empty comments and reviews, pool lookup, virtual-address translation, CPU fallback and upload publication; ran test_import_buffer_binding, test_image_view_retry, test_texture_staging_publication, test_import_span, test_mirror_trim and test_env_on with GCC and Clang: all passed. No game or renderer build was run.

What's good

  • The cost exists in main: upload_unswizzle_gpu calls gpu_buffer, which can reach import_pool synchronously through pool_of and pool_lookup. Checking for an existing import before that call is a small change, not a second upload implementation.
  • The new rejection is handled: upload_inner continues into the CPU path. The contiguity check also stays ahead of virtual-address translation. The threaded conversion still divides work at GOB-block rows and joins its workers before flushing staging memory and queuing the copy.
  • The description clearly says the game measurements include another change and leaves the isolated comparison unchecked.

Must change

No patch-introduced correctness defect found in the paths reviewed. This is a comment on the WIP, not confirmation of its performance claim.

Measurement and smaller points

  • Add coverage for the changed decisions. gpu/vk/vk_texture.cpp, lines 1203-1208 and 1645-1646, and gpu/vk/vk_memory.cpp, lines 3859-3865, have no new behavioral test. The passing view check extracts only view creation; the staging check extracts publication and checks that a join precedes it. Neither executes the new containment decision or the threaded conversion. Cover an existing containing import, a missing or partial import, a noncontiguous virtual mapping, and the override. Compare threaded output with serial output around the new threshold, including a partial final block row and a partial upload. The no-import case should reject main for actually attempting an import, not for missing a source string.
  • Keep the combined result separate from this PR's gain. The Market table reports lower costs for the combined build, but cannot attribute them to this patch rather than https://github.com/NotRexed/Hookshot/pull/3043. Before making this ready, compare named baseline and candidate builds differing only by this patch, with the same instrumentation and settings, warm-up, at least two interleaved runs each, medians, spread and the original log lines. Keep the excluded failed-load run identified. The current disclosure is appropriate; there is no isolated speed-up established here.
  • Refresh the evidence details. This head's parent is cf605c85, not the eca9b12a named as the branch base in the description. Identify the three logging commits too, document RX_VK_UNSWIZZLE_IMPORT in the switch reference, and put the measured runs in separate files under docs/tested/. No frozen-table rows were added by this patch. The override restores import behavior, not the old CPU threading threshold, so it is not an off switch for the whole patch. Replace “request to open” with the hardware test requests before completing that checklist item.

Not checked

No Windows execution, AMD, Intel, Steam Deck, visual comparison, full suite or independent reading of the raw game runs here. In particular, the new default's CPU cost on a lower-core-count machine remains unmeasured. The focused checks do not settle that or the changed output paths. The original discussion at https://github.com/NotRexed/Hookshot/pull/2785 was unavailable, so its earlier reviews could not be checked. The current source-check jobs are blocked, not passing: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/3798.

Read head `c4dd2d01` against main `cf605c85`: the full diff and description, the empty comments and reviews, pool lookup, virtual-address translation, CPU fallback and upload publication; ran `test_import_buffer_binding`, `test_image_view_retry`, `test_texture_staging_publication`, `test_import_span`, `test_mirror_trim` and `test_env_on` with GCC and Clang: all passed. No game or renderer build was run. ### What's good - The cost exists in main: `upload_unswizzle_gpu` calls `gpu_buffer`, which can reach `import_pool` synchronously through `pool_of` and `pool_lookup`. Checking for an existing import before that call is a small change, not a second upload implementation. - The new rejection is handled: `upload_inner` continues into the CPU path. The contiguity check also stays ahead of virtual-address translation. The threaded conversion still divides work at GOB-block rows and joins its workers before flushing staging memory and queuing the copy. - The description clearly says the game measurements include another change and leaves the isolated comparison unchecked. ### Must change No patch-introduced correctness defect found in the paths reviewed. This is a comment on the WIP, not confirmation of its performance claim. ### Measurement and smaller points - **Add coverage for the changed decisions.** `gpu/vk/vk_texture.cpp`, lines 1203-1208 and 1645-1646, and `gpu/vk/vk_memory.cpp`, lines 3859-3865, have no new behavioral test. The passing view check extracts only view creation; the staging check extracts publication and checks that a join precedes it. Neither executes the new containment decision or the threaded conversion. Cover an existing containing import, a missing or partial import, a noncontiguous virtual mapping, and the override. Compare threaded output with serial output around the new threshold, including a partial final block row and a partial upload. The no-import case should reject main for actually attempting an import, not for missing a source string. - **Keep the combined result separate from this PR's gain.** The Market table reports lower costs for the combined build, but cannot attribute them to this patch rather than https://github.com/NotRexed/Hookshot/pull/3043. Before making this ready, compare named baseline and candidate builds differing only by this patch, with the same instrumentation and settings, warm-up, at least two interleaved runs each, medians, spread and the original log lines. Keep the excluded failed-load run identified. The current disclosure is appropriate; there is no isolated speed-up established here. - **Refresh the evidence details.** This head's parent is `cf605c85`, not the `eca9b12a` named as the branch base in the description. Identify the three logging commits too, document `RX_VK_UNSWIZZLE_IMPORT` in the switch reference, and put the measured runs in separate files under `docs/tested/`. No frozen-table rows were added by this patch. The override restores import behavior, not the old CPU threading threshold, so it is not an off switch for the whole patch. Replace “request to open” with the hardware test requests before completing that checklist item. ### Not checked No Windows execution, AMD, Intel, Steam Deck, visual comparison, full suite or independent reading of the raw game runs here. In particular, the new default's CPU cost on a lower-core-count machine remains unmeasured. The focused checks do not settle that or the changed output paths. The original discussion at https://github.com/NotRexed/Hookshot/pull/2785 was unavailable, so its earlier reviews could not be checked. The current source-check jobs are blocked, not passing: https://git.magicbean.dev/MagicBeanDev/Longshot/actions/runs/3798.
DUDUYG force-pushed perf/unswizzle-no-pool-import from c4dd2d01ae
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 4s
Website / Website on Cloudflare (pull_request_target) Has been skipped
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
to 3344ade7a0
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 5s
Website / Website on Cloudflare (pull_request_target) Has been skipped
2026-11-14 12:46:06 +00:00
Compare
Author
First-time contributor

@mooki thanks. Pushed 46cbcc94 and 3344ade7, rebased onto 5e072fc8 (FIocker's commit is 17a3ef23 now).

  • Coverage: tests/test_unswizzle_upload.py cuts the choice (with memory_guest_contained) and the threaded rows out of vk_texture.cpp, and runs them against a model of guest memory.
    • Choice cases that get the GPU path with no import: a pool holds the texture, a pool is exactly its range, and a virtual address above 1 TB is looked up at its guest address.
    • Cases that get the CPU path and never call gpu_buffer: no pool, half in one, split across two pools, a noncontiguous virtual mapping, and an unmapped texture.
    • RX_VK_UNSWIZZLE_IMPORT=1, in its own process, imports as before.
    • Rows: 19 cases, each compared byte for byte with one UnswizzleTexture over the whole level, with the threads counted. They cover just under and at 4 MB, a partial last block row, partial uploads from the middle and to the end, sparse storage, and storage that ends inside the span.
    • With --source on main's code it fails on behavior: six choice cases reach gpu_buffer (five import a pool), and the 4-16 MB levels run on one thread.
  • Evidence details:
    • The base is 5e072fc8.
    • The three logging commits are named in the description: b2698fae, a8988ce3 and 0e78b98f.
    • The measured builds are on my fork: meas/main and meas/335.
  • Docs: RX_VK_UNSWIZZLE_IMPORT is documented as restoring the imports only, not the 16 MB threshold.
  • Isolated runs: three interleaved runs of each build on 5e072fc8. The six runs are in docs/tested/.
    • Slow uploads: from 3 to 1 in every run (102 ms to 37 ms in all).
    • The slowest upload frame: from 137 ms to 120 ms.
    • Nothing is unswizzled on the GPU at the Market entry any more.
    • The frame times stay within the run-to-run spread, so I make no frame-time claim.
  • Test request: T-EEPM, for AMD/Intel and 4-6 core CPUs. It waits for a "testing wanted" label. The cost on CPUs with fewer cores is still unmeasured; it is named under Not tested.
  • Full suite: tests/run_all.py on Windows has 8 failures, all of which fail the same way on main; they are named in the description. I didn't build for Linux here.
@mooki thanks. Pushed 46cbcc94 and 3344ade7, rebased onto 5e072fc8 (FIocker's commit is 17a3ef23 now). - **Coverage:** `tests/test_unswizzle_upload.py` cuts the choice (with `memory_guest_contained`) and the threaded rows out of `vk_texture.cpp`, and runs them against a model of guest memory. - Choice cases that get the GPU path with no import: a pool holds the texture, a pool is exactly its range, and a virtual address above 1 TB is looked up at its guest address. - Cases that get the CPU path and never call `gpu_buffer`: no pool, half in one, split across two pools, a noncontiguous virtual mapping, and an unmapped texture. - `RX_VK_UNSWIZZLE_IMPORT=1`, in its own process, imports as before. - Rows: 19 cases, each compared byte for byte with one `UnswizzleTexture` over the whole level, with the threads counted. They cover just under and at 4 MB, a partial last block row, partial uploads from the middle and to the end, sparse storage, and storage that ends inside the span. - With `--source` on main's code it fails on behavior: six choice cases reach `gpu_buffer` (five import a pool), and the 4-16 MB levels run on one thread. - **Evidence details:** - The base is 5e072fc8. - The three logging commits are named in the description: b2698fae, a8988ce3 and 0e78b98f. - The measured builds are on my fork: `meas/main` and `meas/335`. - **Docs:** `RX_VK_UNSWIZZLE_IMPORT` is documented as restoring the imports only, not the 16 MB threshold. - **Isolated runs:** three interleaved runs of each build on 5e072fc8. The six runs are in `docs/tested/`. - Slow uploads: from 3 to 1 in every run (102 ms to 37 ms in all). - The slowest upload frame: from 137 ms to 120 ms. - Nothing is unswizzled on the GPU at the Market entry any more. - The frame times stay within the run-to-run spread, so I make no frame-time claim. - **Test request:** T-EEPM, for AMD/Intel and 4-6 core CPUs. It waits for a "testing wanted" label. The cost on CPUs with fewer cores is still unmeasured; it is named under Not tested. - **Full suite:** `tests/run_all.py` on Windows has 8 failures, all of which fail the same way on main; they are named in the description. I didn't build for Linux here.
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 5s
Website / Website on Cloudflare (pull_request_target) Has been skipped
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/unswizzle-no-pool-import:DUDUYG-perf/unswizzle-no-pool-import'
git switch 'DUDUYG-perf/unswizzle-no-pool-import'
Sign in to join this conversation.
No reviewers
No milestone
No project
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!335
No description provided.