MagicBeanDev/Hookshot
Archived
Watch
6
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

fix(vk): copy the late pages on the GPU instead of waiting for their write-back #2789

Open
github-sync wants to merge 2 commits from github/pr/2789 into main
pull from: github/pr/2789
merge into: MagicBeanDev:main
Conversation 2 Commits 2 Files changed 6 +469 -29
github-sync commented 2026-11-12 13:38:16 +00:00
Owner

@Revens2 opened this pull request on GitHub · 2026-11-12 13:20 UTC · original

Why

On an RTX 3080 in heavy areas (#2655), one capture every frame made the replay wait for the GPU before copying its late pages - guest pages the CPU wrote that a submission still on the GPU writes back. The log showed a capture taken (ms): all 1.973; GPU wait for late pages 1.957 (100 of 600 takes) at 600 submits a frame, and heavy areas ran at 31-34 fps while the GPU was not the limit. With RX_VK_CAPTURE_WAIT=0 the wait went away and it ran at 57-60 fps, but black flicker appeared. This records those copies on the GPU instead, in the submission that uses the capture, so the game's thread no longer waits for the write-back.

Issue: #2655 - RX_VK_CAPTURE_WAIT=0 takes OoT from ~31 to 60 fps on an RTX 3080 (late-page wait every frame) (#2655)

The issue comment from blushOFF (#2655) is accounted for. Their simpler GPU-side attempt copied the pages live from guest memory into the mirror in the upload command buffer, and the Market flashed white in 1 of 4 runs: the CPU had already written a later frame's exposure page, so the GPU added into the wrong sums. This change does not copy into the mirror: it copies the late pages into the capture's own slots, from the imported pool's buffer, in the recording's upload command buffer after the write-back's submission in queue order - the same bytes the old CPU copy took once the wait ended. A CPU read of a page whose copy has not run yet falls back to guest memory (late_page_pending), and the CPU wait stays wherever the GPU copy cannot be ordered: a page no imported pool holds, a page the texture shadow reads at this take, a burst recording (its uploads would run before its own write-backs), or a parallel replay. On blushOFF's RTX 3070 the wait was rare (6-30 of 600 takes) and RX_VK_CAPTURE_WAIT=0 gave no Market gain, so the expected win is on setups like the reporter's (100 of 600); the A/B below will check both, Market included.

What changed

  • gpu/vk/vk_memory.cpp:
    • late_copy_runs maps each late page, in capture order, to the imported pool that holds it and its 4 KB slot; a page no pool holds, or one whose run passes the pool's end, returns false and the caller keeps the CPU wait.
    • late_copy_plan resolves the pages under pools_mu, keeps each pool's import and mirror alive until the copy's submission is done (last_value), skips a plan that the texture shadow reads at this take, caps the pending pages at 4096, and refuses a burst, a parallel replay (RX_VK_PARALLEL > 1), or RX_VK_CAPTURE_GPU_LATE=0.
    • late_copy_flush (first in memory_begin_submission) records the copies with vkCmdCopyBuffer in the recording's upload command buffer, before its first barrier and before the mirror syncs that may read the same capture buffer, then a full barrier; the pages stay listed until that submission's timeline value completes, and a CPU read of one meanwhile reads guest memory (late_page_pending).
    • Capture buffers gain TRANSFER_DST; the 600-take log line now reports the copies done on the GPU and the GPU waits left separately.
  • gpu/vk/vk_internal.h, gpu/vk/vk_replay.cpp: replay_burst_open() is declared and defined at namespace scope (the fork's first build failed to link it from the anonymous namespace), so a burst's recording, whose uploads run before its own write-backs, uses the CPU wait.
  • tests/test_capture_late_copy.py (new): the runs in capture order, each pool's offsets, the no-pool and past-the-end fallbacks, the plan before the wait, the upload-command-buffer recording, the pending-page read, replay_burst_open at namespace scope, the parallel guard and the cap. tests/test_capture_wait_log.py: updated for the split log line and the fallback counters.

Testing

Game-free, Windows, llvm-mingw, in the worktree:

  • python -I -B tests/run_all.py: 462 passed, 1 failed, 33 skipped.
  • Failed: tests/test_input_background.py, a declared known environment failure on this PC (no MSVC SDK; gcc -Wunused-but-set-global on the fixture). It fails the same way on an untouched upstream main.
  • The two other declared environment failures, tests/test_precompile_compute_guard.py and tests/test_recorder_sync.py, passed in this run.
  • Focused: test_capture_late_copy and test_capture_wait_log pass on HEAD. On upstream 51da0464 they fail for the missing wiring (test_capture_late_copy fails at once; 3 of 6 test_capture_wait_log cases fail).
  • Link: the Windows llvm-mingw link check (the one the anonymous-namespace error broke) succeeds, and every check on the fork's PR #11 was green.
  • Not tested: a real game run (A/B below).

GPU/driver/area: NVIDIA, Windows. Issue reporter: RTX 3080 10 GB, driver 0x9b404000, Windows 10 Pro 22H2; comment: RTX 3070 8 GB, Windows 11; development: RTX 5070 12 GB, driver 32.0.16.2114, Windows 11 build 26200, i5-14600KF. Zone: Vulkan capture and late-page copy (gpu/vk/vk_memory.cpp) on the Windows back end.

in-game A/B: pending, results in a comment

AI assistance

Assisted-by: oc-impl-5 (unknown/unknown), oc-upstream-1 (opencode/deepseek-v4.1-flash-free)

> [@Revens2](https://github.com/Revens2) opened this pull request on GitHub · 2026-11-12 13:20 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2789) ## Why On an RTX 3080 in heavy areas (#2655), one capture every frame made the replay wait for the GPU before copying its late pages - guest pages the CPU wrote that a submission still on the GPU writes back. The log showed `a capture taken (ms): all 1.973; GPU wait for late pages 1.957 (100 of 600 takes)` at 600 submits a frame, and heavy areas ran at 31-34 fps while the GPU was not the limit. With `RX_VK_CAPTURE_WAIT=0` the wait went away and it ran at 57-60 fps, but black flicker appeared. This records those copies on the GPU instead, in the submission that uses the capture, so the game's thread no longer waits for the write-back. Issue: #2655 - RX_VK_CAPTURE_WAIT=0 takes OoT from ~31 to 60 fps on an RTX 3080 (late-page wait every frame) (https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/2655) The issue comment from blushOFF (https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/2655) is accounted for. Their simpler GPU-side attempt copied the pages live from guest memory into the mirror in the upload command buffer, and the Market flashed white in 1 of 4 runs: the CPU had already written a later frame's exposure page, so the GPU added into the wrong sums. This change does not copy into the mirror: it copies the late pages into the capture's own slots, from the imported pool's buffer, in the recording's upload command buffer after the write-back's submission in queue order - the same bytes the old CPU copy took once the wait ended. A CPU read of a page whose copy has not run yet falls back to guest memory (`late_page_pending`), and the CPU wait stays wherever the GPU copy cannot be ordered: a page no imported pool holds, a page the texture shadow reads at this take, a burst recording (its uploads would run before its own write-backs), or a parallel replay. On blushOFF's RTX 3070 the wait was rare (6-30 of 600 takes) and `RX_VK_CAPTURE_WAIT=0` gave no Market gain, so the expected win is on setups like the reporter's (100 of 600); the A/B below will check both, Market included. ## What changed - `gpu/vk/vk_memory.cpp`: - `late_copy_runs` maps each late page, in capture order, to the imported pool that holds it and its 4 KB slot; a page no pool holds, or one whose run passes the pool's end, returns false and the caller keeps the CPU wait. - `late_copy_plan` resolves the pages under `pools_mu`, keeps each pool's import and mirror alive until the copy's submission is done (`last_value`), skips a plan that the texture shadow reads at this take, caps the pending pages at 4096, and refuses a burst, a parallel replay (`RX_VK_PARALLEL` > 1), or `RX_VK_CAPTURE_GPU_LATE=0`. - `late_copy_flush` (first in `memory_begin_submission`) records the copies with `vkCmdCopyBuffer` in the recording's upload command buffer, before its first barrier and before the mirror syncs that may read the same capture buffer, then a full barrier; the pages stay listed until that submission's timeline value completes, and a CPU read of one meanwhile reads guest memory (`late_page_pending`). - Capture buffers gain `TRANSFER_DST`; the 600-take log line now reports the copies done on the GPU and the GPU waits left separately. - `gpu/vk/vk_internal.h`, `gpu/vk/vk_replay.cpp`: `replay_burst_open()` is declared and defined at namespace scope (the fork's first build failed to link it from the anonymous namespace), so a burst's recording, whose uploads run before its own write-backs, uses the CPU wait. - `tests/test_capture_late_copy.py` (new): the runs in capture order, each pool's offsets, the no-pool and past-the-end fallbacks, the plan before the wait, the upload-command-buffer recording, the pending-page read, `replay_burst_open` at namespace scope, the parallel guard and the cap. `tests/test_capture_wait_log.py`: updated for the split log line and the fallback counters. ## Testing Game-free, Windows, llvm-mingw, in the worktree: - `python -I -B tests/run_all.py`: 462 passed, 1 failed, 33 skipped. - Failed: `tests/test_input_background.py`, a declared known environment failure on this PC (no MSVC SDK; gcc `-Wunused-but-set-global` on the fixture). It fails the same way on an untouched upstream main. - The two other declared environment failures, `tests/test_precompile_compute_guard.py` and `tests/test_recorder_sync.py`, passed in this run. - Focused: `test_capture_late_copy` and `test_capture_wait_log` pass on HEAD. On upstream `51da0464` they fail for the missing wiring (`test_capture_late_copy` fails at once; 3 of 6 `test_capture_wait_log` cases fail). - Link: the Windows llvm-mingw link check (the one the anonymous-namespace error broke) succeeds, and every check on the fork's PR #11 was green. - Not tested: a real game run (A/B below). GPU/driver/area: NVIDIA, Windows. Issue reporter: RTX 3080 10 GB, driver `0x9b404000`, Windows 10 Pro 22H2; comment: RTX 3070 8 GB, Windows 11; development: RTX 5070 12 GB, driver 32.0.16.2114, Windows 11 build 26200, i5-14600KF. Zone: Vulkan capture and late-page copy (`gpu/vk/vk_memory.cpp`) on the Windows back end. in-game A/B: pending, results in a comment ## AI assistance Assisted-by: oc-impl-5 (unknown/unknown), oc-upstream-1 (opencode/deepseek-v4.1-flash-free) <!-- github-sync:item:2789 -->
Assisted-by: oc-upstream-1 (opencode/deepseek-v4.1-flash-free)
Trace-Issue: #2655
github-sync commented 2026-11-12 14:48:25 +00:00
Author
Owner

@Revens2 commented on GitHub · 2026-11-12 13:54 UTC · original

In-game A/B (late pages copied on the GPU)

Default replay (RX_VK_PARALLEL=0, event waits/bursts on):

build avg FPS 1% low p99 ms >33 ms late pages on GPU N/600 GPU wait M/600 capture all ms
control 97.31 81.57 12.26 0 - 100 0.270
patch 98.44 83.46 12.00 0 0 98 0.240

The GPU path never engages on the default replay: take_capture runs inside a burst (replay_burst_open() true), so late_copy_plan always takes the CPU wait; RX_VK_PARALLEL>1 disables it too (gpu_late_on()). N=0, wait counters unchanged; the mean difference is build noise (the change is inert), not a win.

Bursts off (RX_VK_PARALLEL=0; RX_VK_EVENT_WAITS=0):

build avg FPS 1% low p99 ms >33 ms late pages on GPU N/600 GPU wait M/600 capture all ms
control 83.55 58.51 17.09 0 - 120 0.470
patch 76.66 64.31 15.55 0 56-69 0 0.004
patch + RX_VK_CAPTURE_GPU_LATE=0 80.88 58.31 17.15 0 0 80 0.313

With bursts off the copy path works: the late-page copy moves to the GPU, the CPU wait drops to 0 and the capture cost falls 0.47 -> 0.004 ms; GPU_LATE=0 restores the CPU wait (0.313 ms) as expected. But that replay mode costs ~14-20% avg FPS (83.6 vs 97.3 default), so no usable win here.

Diagnostic (no numbers claimed): RX_VK_CAPTURE_WAIT=0, default bursts: 118.26 avg / 94.34 1% low / 10.60 p99, capture 0.003 ms, no waits; stale-data mode unchanged, visual flicker not asserted.

Setup: RTX 5070 12 GB, i5-14600KF, Windows 11 26200, 1440p, frame generation on, 240 cap, same save/route, 2 runs per default config, CPU idle checks. DLL-only on the installed v2.1.0 host (MKWVK API 14): the vk change was ported onto the v2.1.0 tree (upstream-main builds are API 19 and cannot load into that host).

> [@Revens2](https://github.com/Revens2) commented on GitHub · 2026-11-12 13:54 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2789#issuecomment-6825534313) In-game A/B (late pages copied on the GPU) Default replay (RX_VK_PARALLEL=0, event waits/bursts on): | build | avg FPS | 1% low | p99 ms | >33 ms | late pages on GPU N/600 | GPU wait M/600 | capture all ms | |---|---|---|---|---|---|---|---| | control | 97.31 | 81.57 | 12.26 | 0 | - | 100 | 0.270 | | patch | 98.44 | 83.46 | 12.00 | 0 | 0 | 98 | 0.240 | The GPU path never engages on the default replay: take_capture runs inside a burst (replay_burst_open() true), so late_copy_plan always takes the CPU wait; RX_VK_PARALLEL>1 disables it too (gpu_late_on()). N=0, wait counters unchanged; the mean difference is build noise (the change is inert), not a win. Bursts off (RX_VK_PARALLEL=0; RX_VK_EVENT_WAITS=0): | build | avg FPS | 1% low | p99 ms | >33 ms | late pages on GPU N/600 | GPU wait M/600 | capture all ms | |---|---|---|---|---|---|---|---| | control | 83.55 | 58.51 | 17.09 | 0 | - | 120 | 0.470 | | patch | 76.66 | 64.31 | 15.55 | 0 | 56-69 | 0 | 0.004 | | patch + RX_VK_CAPTURE_GPU_LATE=0 | 80.88 | 58.31 | 17.15 | 0 | 0 | 80 | 0.313 | With bursts off the copy path works: the late-page copy moves to the GPU, the CPU wait drops to 0 and the capture cost falls 0.47 -> 0.004 ms; GPU_LATE=0 restores the CPU wait (0.313 ms) as expected. But that replay mode costs ~14-20% avg FPS (83.6 vs 97.3 default), so no usable win here. Diagnostic (no numbers claimed): RX_VK_CAPTURE_WAIT=0, default bursts: 118.26 avg / 94.34 1% low / 10.60 p99, capture 0.003 ms, no waits; stale-data mode unchanged, visual flicker not asserted. Setup: RTX 5070 12 GB, i5-14600KF, Windows 11 26200, 1440p, frame generation on, 240 cap, same save/route, 2 runs per default config, CPU idle checks. DLL-only on the installed v2.1.0 host (MKWVK API 14): the vk change was ported onto the v2.1.0 tree (upstream-main builds are API 19 and cannot load into that host). <!-- github-sync:c:6825534313 -->
github-sync commented 2026-11-12 14:48:25 +00:00
Author
Owner

@Joey-Magnus requested changes on GitHub · 2026-11-12 14:35 UTC · original

I merged this into current main and ran tests/run_all.py. Everything passes except test_lift_cache, which fails on main here too. The tests aren't the problem. Two things are:

  • Your own A/B comment shows the GPU path never runs on the default replay. take_capture runs inside a burst, so late_copy_plan always falls back to the CPU wait. It only runs with RX_VK_EVENT_WAITS=0, and that mode is 14-20% slower.
  • The copy reads the pool's imported buffer (vk_memory.cpp:1302, p->buffer), which is live guest memory, when the GPU reaches the upload command buffer. That's the race blushOFF hit on #2655: the game thread had already written a later frame's exposure page, and the Market flashed white. Copying into the capture slot instead of the mirror doesn't change which bytes are read. So "the same bytes the old CPU copy took" isn't true: the old copy ran right after the wait, and this one runs later.

The A/B was also a port onto v2.1.0, not this head. I'd suggest doing the word-by-word merge against the write-back diff's shadow that blushOFF described, or closing this.

> [@Joey-Magnus](https://github.com/Joey-Magnus) requested changes on GitHub · 2026-11-12 14:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2789#pullrequestreview-5876257549) I merged this into current main and ran tests/run_all.py. Everything passes except test_lift_cache, which fails on main here too. The tests aren't the problem. Two things are: - Your own A/B comment shows the GPU path never runs on the default replay. take_capture runs inside a burst, so late_copy_plan always falls back to the CPU wait. It only runs with RX_VK_EVENT_WAITS=0, and that mode is 14-20% slower. - The copy reads the pool's imported buffer (vk_memory.cpp:1302, `p->buffer`), which is live guest memory, when the GPU reaches the upload command buffer. That's the race blushOFF hit on #2655: the game thread had already written a later frame's exposure page, and the Market flashed white. Copying into the capture slot instead of the mirror doesn't change which bytes are read. So "the same bytes the old CPU copy took" isn't true: the old copy ran right after the wait, and this one runs later. The A/B was also a port onto v2.1.0, not this head. I'd suggest doing the word-by-word merge against the write-back diff's shadow that blushOFF described, or closing this. <!-- github-sync:r:5876257549 -->
Assisted-by: impl-2655 (unknown/unknown)
Trace-Issue: #2655
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
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/Hookshot!2789
No description provided.