perf(vk): less copying of captured memory on the game and replay threads #1375

github-sync · 2026-11-09T23:43:56Z

valonsodev commented

2026-11-09 23:43:56 +00:00

(Migrated from github.com)

This branch is on main at 0828863. The 3+3 table below was measured on v2.0.11 (7dbdcdc), and the single run on 581afaf.

What changed

Two changes to how the renderer reads and keeps its captures of guest memory, both in gpu/vk/vk_memory.cpp:

Keep 32 submitted pages a thread, not one (memory_read_as_submitted).
- #315 added a one-page cache here, so the next read of a page skips pools_mu and the capture lookup. The answer is now kept per page, 32 a thread, picked by page number (submitted_page_for).
- The reuse rule is unchanged: same page, same page generation, same capture view.
The texture shadow's pages kept in place and copied by the capture helpers (take_capture).
- The pages copied now keep their shadow slot and are overwritten in place. Only pages written but not copied are erased.
- A full shadow drops what it can't replace, as before.
- The 4 KB copies run on the capture's copy helpers (copy_pairs), with shadow_mu still held.
- copy_pages and copy_pairs share the helpers, and each call now waits for its own parts, not every caller's.

Why

On a profile of v2.0.11 in Hyrule Castle Town, on a test route (Tracy zones on the host and renderer, perf for the rest), the game thread (sead::MainThread) is the frame's critical path:

Its own CPU: about 21 ms of a 45 ms frame (under capture).
Then its job batches, about 16 ms, run on the six BaseProcExecutors.
Then about 6 ms waiting for the replay.

The replay thread waits about 16 ms a frame for the game.

Change 2: the game thread's copies. About half of the game thread's CPU was libc memcpy, in the capture work of its own submits:

take_capture's shadow pass erased the shadow copy of every page written since the last capture (a hash erase and a 4 KB delete[] each),
then inserted the copied pages again (a hash insert and a 4 KB new[] each),
then copied their 4 KB one after another.

OoT's game thread submits twice a frame and rewrites mostly the same pages. On a profiling build (v2.0.10), Tracy zones around the pass measured 6.8 ms a frame before this change and 1.4 ms after.

Change 1: the replay's per-draw reads. A handle program reads a uniform word on one page and a storage-buffer word on another (stage_handle → stage_cbuf_word → ub_read → memory_read_as_submitted). With one page kept, those reads take turns missing: each takes pools_mu and copies the whole 4 KB page to serve 4 bytes.

Profile: on the v2.0.10 profiling build, before the change, program_for was 41% of the replay thread, and ub_read 35%, mostly that copy.
Slow-path reads: on v2.0.11, 3.2× fewer with the change (the vk: replay reads of guest memory as submitted counter: 2.68 M → 0.84 M).
Replay thread CPU: 63–67% → 48–56% in each A/B pair (the log's per-thread times).

It matters less for fps here, because the replay thread already waits for the game. It matters where the replay is the limit.

How it was tested

Hardware: NVIDIA GeForce RTX 3060 Ti, driver 615.78.08; AMD Ryzen 9 5900X; Arch Linux, kernel 7.2.14. Windows is not tested.

Game area: Hyrule Castle Town (young Link, from a save at noon), on a scripted 30 s test route through the plaza: the same stick and camera inputs every run, with camera sweeps over the crowd.

Settings: the same options file for every run: quality preset 5, frame generation 3× (frame_generation=3); built without the DLSS SDK. The fps counts the game's own presents (nvnQueuePresentTexture), not the interpolated frames.

Builds: normal build settings (make_game.py, RX_MARCH=native), from the same tree. A local debugging commit that drives the route over a socket is in every build (it isn't part of this PR).

build	run 1	run 2	run 3	mean
`main` (v2.0.11)	23.99	23.86	25.42	24.42
change 1 only	25.53	23.72	26.99	25.41
this PR (1 + 2)	32.35	32.62	30.99	31.99
this PR on `581afaf` (`main` when opened)	33.22			one run

The first two rows were run alternately, main then change 1. The third was run right after, on the same PC and route. The last row is a single run of this branch rebased on current main, with no baseline run on that main beside it. It shows the gain holds there, not a new percentage.

With both changes: +7.6 fps over main (+31%). The slowest run with them (30.99) is faster than every run without them.

The game thread's CPU (the log's per-thread times) fell from about 44% to 31%, and the replay thread's rose to about 61%, now that it gets more frames. All ten runs ended at the same point on the route. Their end-of-route screenshots render as before, with no device losses.

Checks:

tests/test_submitted_page.py, extended: the reuse rule, the per-page lookup in both places, and no slot shared by a page and its neighbour or by pages 32 apart.
tests/test_capture_copy_helpers.py, new:
- copy_pairs and copy_pages run on the shared helpers from two threads at once;
- every call returns with all of its own copies made, for sizes around the helpers' threshold;
- take_capture's shadow pass keeps the shape described above.
With copy_pairs' wait removed, it fails.
python -I -B tests/run_all.py (on 581afaf): 182 passed, 19 skipped, 1 failed. The failure is tests/test_make_game_cache.py, and it fails on main too on this PC: it runs the builder, which finds this machine's real build-cache profile.

Cost: 32 × 4 KB = 128 KB of thread-local memory in hookshot_vk per thread that touches the renderer's thread-locals, up from 4 KB.

Assisted-by: Claude Opus 5.5

🤖 Generated with Claude Code

This branch is on `main` at 0828863. The 3+3 table below was measured on v2.0.11 (7dbdcdc), and the single run on 581afaf. ## What changed Two changes to how the renderer reads and keeps its captures of guest memory, both in `gpu/vk/vk_memory.cpp`: 1. **Keep 32 submitted pages a thread, not one** (`memory_read_as_submitted`). - #315 added a one-page cache here, so the next read of a page skips `pools_mu` and the capture lookup. The answer is now kept per page, 32 a thread, picked by page number (`submitted_page_for`). - The reuse rule is unchanged: same page, same page generation, same capture view. 2. **The texture shadow's pages kept in place and copied by the capture helpers** (`take_capture`). - The pages copied now keep their shadow slot and are overwritten in place. Only pages written but not copied are erased. - A full shadow drops what it can't replace, as before. - The 4 KB copies run on the capture's copy helpers (`copy_pairs`), with `shadow_mu` still held. - `copy_pages` and `copy_pairs` share the helpers, and each call now waits for its own parts, not every caller's. ## Why On a profile of v2.0.11 in Hyrule Castle Town, on a test route (Tracy zones on the host and renderer, `perf` for the rest), the game thread (`sead::MainThread`) is the frame's critical path: - **Its own CPU:** about 21 ms of a 45 ms frame (under capture). - **Then its job batches,** about 16 ms, run on the six `BaseProcExecutor`s. - **Then about 6 ms waiting for the replay.** The replay thread waits about 16 ms a frame for the game. **Change 2: the game thread's copies.** About half of the game thread's CPU was libc `memcpy`, in the capture work of its own submits: - `take_capture`'s shadow pass erased the shadow copy of every page written since the last capture (a hash erase and a 4 KB `delete[]` each), - then inserted the copied pages again (a hash insert and a 4 KB `new[]` each), - then copied their 4 KB one after another. OoT's game thread submits twice a frame and rewrites mostly the same pages. On a profiling build (v2.0.10), Tracy zones around the pass measured 6.8 ms a frame before this change and 1.4 ms after. **Change 1: the replay's per-draw reads.** A handle program reads a uniform word on one page and a storage-buffer word on another (`stage_handle` → `stage_cbuf_word` → `ub_read` → `memory_read_as_submitted`). With one page kept, those reads take turns missing: each takes `pools_mu` and copies the whole 4 KB page to serve 4 bytes. - **Profile:** on the v2.0.10 profiling build, before the change, `program_for` was 41% of the replay thread, and `ub_read` 35%, mostly that copy. - **Slow-path reads:** on v2.0.11, 3.2× fewer with the change (the `vk: replay reads of guest memory as submitted` counter: 2.68 M → 0.84 M). - **Replay thread CPU:** 63–67% → 48–56% in each A/B pair (the log's per-thread times). It matters less for fps here, because the replay thread already waits for the game. It matters where the replay is the limit. ## How it was tested **Hardware:** NVIDIA GeForce RTX 3060 Ti, driver 615.78.08; AMD Ryzen 9 5900X; Arch Linux, kernel 7.2.14. Windows is not tested. **Game area:** Hyrule Castle Town (young Link, from a save at noon), on a scripted 30 s test route through the plaza: the same stick and camera inputs every run, with camera sweeps over the crowd. **Settings:** the same options file for every run: quality preset 5, frame generation 3× (`frame_generation=3`); built without the DLSS SDK. The fps counts the game's own presents (`nvnQueuePresentTexture`), not the interpolated frames. **Builds:** normal build settings (`make_game.py`, `RX_MARCH=native`), from the same tree. A local debugging commit that drives the route over a socket is in every build (it isn't part of this PR). | build | run 1 | run 2 | run 3 | mean | |---|---|---|---|---| | `main` (v2.0.11) | 23.99 | 23.86 | 25.42 | **24.42** | | change 1 only | 25.53 | 23.72 | 26.99 | **25.41** | | this PR (1 + 2) | 32.35 | 32.62 | 30.99 | **31.99** | | this PR on 581afaf (`main` when opened) | 33.22 | | | one run | The first two rows were run alternately, `main` then change 1. The third was run right after, on the same PC and route. The last row is a single run of this branch rebased on current `main`, with no baseline run on that `main` beside it. It shows the gain holds there, not a new percentage. With both changes: +7.6 fps over `main` (+31%). The slowest run with them (30.99) is faster than every run without them. The game thread's CPU (the log's per-thread times) fell from about 44% to 31%, and the replay thread's rose to about 61%, now that it gets more frames. All ten runs ended at the same point on the route. Their end-of-route screenshots render as before, with no device losses. **Checks:** - `tests/test_submitted_page.py`, extended: the reuse rule, the per-page lookup in both places, and no slot shared by a page and its neighbour or by pages 32 apart. - `tests/test_capture_copy_helpers.py`, new: - `copy_pairs` and `copy_pages` run on the shared helpers from two threads at once; - every call returns with all of its own copies made, for sizes around the helpers' threshold; - `take_capture`'s shadow pass keeps the shape described above. With `copy_pairs`' wait removed, it fails. - `python -I -B tests/run_all.py` (on 581afaf): 182 passed, 19 skipped, 1 failed. The failure is `tests/test_make_game_cache.py`, and it fails on `main` too on this PC: it runs the builder, which finds this machine's real build-cache profile. **Cost:** 32 × 4 KB = 128 KB of thread-local memory in `hookshot_vk` per thread that touches the renderer's thread-locals, up from 4 KB. Assisted-by: Claude Opus 5.5 🤖 Generated with [Claude Code](https://claude.com/claude-code)

klimovich008 commented

2026-11-10 00:29:49 +00:00

(Migrated from github.com)

Review from reading the diff against v2.0.11 (581e83f), running the two tests, and checking logs of our own Windows runs. I have not measured the PR's fps effect yet; a 3×3 A/B on Windows/AMD is queued and I'll add the numbers here.

Setup for the numbers below: Windows 11, Ryzen 7 5700X3D, RX 9070 XT, v2.0.11, 32:9, warm caches, replay inline on the game's threads (the default).

Correctness

I found no correctness problem.

Change 1: each of the 32 slots keeps the old rule (same page, same page generation, same capture view), so it is as safe as the single slot.
Change 2: the shadow ends in the same state as before, including the full-shadow path (a page already there is overwritten even when full; a page that can't be placed loses its stale copy in the trailing loop). Slots taken in the pass carry the current shadow_seq, so the sweep can't drop one whose copy is still pending.
Helpers: each caller waits on its own left, changed only under copy_helpers.mu, so copy_pages and copy_pairs can overlap without one returning early or waiting forever.

Both commits cherry-pick cleanly onto v2.0.11.

Findings

tests/test_capture_copy_helpers.py does not build on Windows. It compiles its fixture with -Werror, and the MSVC CRT marks getenv deprecated (-Wdeprecated-declarations, twice, in copy_pages and copy_pairs). With -D_CRT_SECURE_NO_WARNINGS added to the compile line (the game build passes it too) the test passes here. tests/test_submitted_page.py passes as it is.
The helper threshold in copy_pairs looks low. From 64 pages (256 KB) the copies go to the helpers. Copying 64 pages inline is on the order of 10–15 µs; waking two to four threads through a condition variable and waiting for them usually costs more. The caller now also waits with pools_mu and shadow_mu held, behind whatever copy_pages has queued from another thread, so on a loaded or low-core PC a short copy can become a wait under two locks. This is from reading, not a measurement. copy_pages uses 128 pages for copies that are slower (guarded reads of guest memory).
The gain of change 2 is not split. The 6.8 → 1.4 ms figure covers keeping the pages in place and moving the copies to helpers. A row for change 2 without the helpers would show whether they are needed at all; my guess is that the in-place part (no free, allocation and two hash operations a page) carries most of it.
Memory, minor. thread_local SubmittedPage submitted_pages[32] is about 132 KB of TLS. On Windows every thread of the process gets a module's TLS block when it starts, used or not; it was 4 KB. A lazily allocated table would avoid that.

Does it matter outside Castle Town?

Change 1: yes, by our logs. In Hyrule Field memory_read_as_submitted takes its slow path 3,528 times a frame and copies 769 pages from captures (Kokiri Forest: 1,250 and 162), from the vk: replay reads of guest memory as submitted counter over 720 frames.
Change 2: probably little in those two scenes. The texture shadow holds about 840 pages (3 MB) there.

A possible follow-up

The reuse rule compares the capture view, which moves at every capture and every stamp anywhere, six or more times a frame, so all 32 slots are emptied each time even for pages nothing touched. What a kept answer depends on is narrower: the page's own generation, and, when the answer is a capture's copy, that capture still being kept. Captures are retired oldest first, so one published "oldest kept generation" is enough to check that. I have this as a small patch on top of this PR with a test, behind a switch for the old rule; I'll measure it with the A/B and open it separately only if it shows a gain.

🤖 Reviewed with Claude Code

Review from reading the diff against v2.0.11 (581e83f), running the two tests, and checking logs of our own Windows runs. I have **not** measured the PR's fps effect yet; a 3×3 A/B on Windows/AMD is queued and I'll add the numbers here. **Setup for the numbers below:** Windows 11, Ryzen 7 5700X3D, RX 9070 XT, v2.0.11, 32:9, warm caches, replay inline on the game's threads (the default). ### Correctness I found no correctness problem. - **Change 1:** each of the 32 slots keeps the old rule (same page, same page generation, same capture view), so it is as safe as the single slot. - **Change 2:** the shadow ends in the same state as before, including the full-shadow path (a page already there is overwritten even when full; a page that can't be placed loses its stale copy in the trailing loop). Slots taken in the pass carry the current `shadow_seq`, so the sweep can't drop one whose copy is still pending. - **Helpers:** each caller waits on its own `left`, changed only under `copy_helpers.mu`, so `copy_pages` and `copy_pairs` can overlap without one returning early or waiting forever. Both commits cherry-pick cleanly onto v2.0.11. ### Findings 1. **`tests/test_capture_copy_helpers.py` does not build on Windows.** It compiles its fixture with `-Werror`, and the MSVC CRT marks `getenv` deprecated (`-Wdeprecated-declarations`, twice, in `copy_pages` and `copy_pairs`). With `-D_CRT_SECURE_NO_WARNINGS` added to the compile line (the game build passes it too) the test passes here. `tests/test_submitted_page.py` passes as it is. 2. **The helper threshold in `copy_pairs` looks low.** From 64 pages (256 KB) the copies go to the helpers. Copying 64 pages inline is on the order of 10–15 µs; waking two to four threads through a condition variable and waiting for them usually costs more. The caller now also waits with `pools_mu` and `shadow_mu` held, behind whatever `copy_pages` has queued from another thread, so on a loaded or low-core PC a short copy can become a wait under two locks. This is from reading, not a measurement. `copy_pages` uses 128 pages for copies that are slower (guarded reads of guest memory). 3. **The gain of change 2 is not split.** The 6.8 → 1.4 ms figure covers keeping the pages in place and moving the copies to helpers. A row for change 2 without the helpers would show whether they are needed at all; my guess is that the in-place part (no free, allocation and two hash operations a page) carries most of it. 4. **Memory, minor.** `thread_local SubmittedPage submitted_pages[32]` is about 132 KB of TLS. On Windows every thread of the process gets a module's TLS block when it starts, used or not; it was 4 KB. A lazily allocated table would avoid that. ### Does it matter outside Castle Town? - **Change 1: yes, by our logs.** In Hyrule Field `memory_read_as_submitted` takes its slow path 3,528 times a frame and copies 769 pages from captures (Kokiri Forest: 1,250 and 162), from the `vk: replay reads of guest memory as submitted` counter over 720 frames. - **Change 2: probably little in those two scenes.** The texture shadow holds about 840 pages (3 MB) there. ### A possible follow-up The reuse rule compares the capture view, which moves at every capture and every stamp anywhere, six or more times a frame, so all 32 slots are emptied each time even for pages nothing touched. What a kept answer depends on is narrower: the page's own generation, and, when the answer is a capture's copy, that capture still being kept. Captures are retired oldest first, so one published "oldest kept generation" is enough to check that. I have this as a small patch on top of this PR with a test, behind a switch for the old rule; I'll measure it with the A/B and open it separately only if it shows a gain. 🤖 Reviewed with [Claude Code](https://claude.com/claude-code)

klimovich008 commented

2026-11-10 01:15:38 +00:00

(Migrated from github.com)

The measurement I promised above, and a correction to my own comment.

Setup: Windows 11, Ryzen 7 5700X3D, RX 9070 XT, v2.0.11 (581e83f) exe, 32:9, warm caches, replay inline. Hyrule Field, camera turning, 20 s windows anchored on the save load, 3 runs an arm, alternating. Both renderer DLLs are built with the same recipe. The variant is v2.0.11 plus this PR's two commits, cherry-picked, plus my follow-up patch from above set to this PR's own reuse rule by its environment switch (so it behaves as this PR, with one extra counter in the log line).

per frame, median of 3	v2.0.11	this PR
fps	58.7 (57.2 / 58.7 / 58.8)	58.5 (57.4 / 58.5 / 58.8)
replay "submit"	13.42 ms	13.50 ms
of it "programs"	1.23 ms	1.18 ms
`memory_read_as_submitted` slow-path reads	3,556	833
pages copied from captures	773	214
process CPU	4.00 cores	4.01 cores

The PR does what it says: slow-path reads fall by 77 % and page copies by 72 %.
On this scene it changes nothing measurable. The reads were not a real cost here: the function is asked about 4,850 times a frame in all, and removing 2,700 slow reads moved "programs" by 0.05 ms. fps and replay time are within run-to-run noise. This scene is replay-bound at about 58 fps, so it is the case the description says the change should help; here it doesn't, and it doesn't hurt either.
Correction: I wrote above that change 1 "matters, by our logs". The counter was high, but its cost was small. I inferred a cost from a count; the A/B says otherwise for this scene. Castle Town with a large texture shadow may well be different, as your numbers show.
The follow-up I floated (keeping pages across submits by the oldest kept capture's generation) took slow reads from 833 to 639 a frame with no change in any phase or in fps (3 runs against 2). Not worth a PR; dropped.

Not tested here: Castle Town, frame generation, a game-thread-bound scene, change 2's helpers on a low-core PC.

🤖 Measured with Claude Code

The measurement I promised above, and a correction to my own comment. **Setup:** Windows 11, Ryzen 7 5700X3D, RX 9070 XT, v2.0.11 (581e83f) exe, 32:9, warm caches, replay inline. Hyrule Field, camera turning, 20 s windows anchored on the save load, 3 runs an arm, alternating. Both renderer DLLs are built with the same recipe. The variant is v2.0.11 plus this PR's two commits, cherry-picked, plus my follow-up patch from above set to this PR's own reuse rule by its environment switch (so it behaves as this PR, with one extra counter in the log line). | per frame, median of 3 | v2.0.11 | this PR | |---|---|---| | fps | 58.7 (57.2 / 58.7 / 58.8) | 58.5 (57.4 / 58.5 / 58.8) | | replay "submit" | 13.42 ms | 13.50 ms | | of it "programs" | 1.23 ms | 1.18 ms | | `memory_read_as_submitted` slow-path reads | 3,556 | 833 | | pages copied from captures | 773 | 214 | | process CPU | 4.00 cores | 4.01 cores | - **The PR does what it says:** slow-path reads fall by 77 % and page copies by 72 %. - **On this scene it changes nothing measurable.** The reads were not a real cost here: the function is asked about 4,850 times a frame in all, and removing 2,700 slow reads moved "programs" by 0.05 ms. fps and replay time are within run-to-run noise. This scene is replay-bound at about 58 fps, so it is the case the description says the change should help; here it doesn't, and it doesn't hurt either. - **Correction:** I wrote above that change 1 "matters, by our logs". The counter was high, but its cost was small. I inferred a cost from a count; the A/B says otherwise for this scene. Castle Town with a large texture shadow may well be different, as your numbers show. - **The follow-up I floated** (keeping pages across submits by the oldest kept capture's generation) took slow reads from 833 to 639 a frame with no change in any phase or in fps (3 runs against 2). Not worth a PR; dropped. Not tested here: Castle Town, frame generation, a game-thread-bound scene, change 2's helpers on a low-core PC. 🤖 Measured with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) reviewed

2026-11-10 04:16:19 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, this is a nice find, and the write-up makes it easy to check. I read both changes against main (5609af4, which only touches two unrelated bounds checks in vk_memory.cpp, so it merges cleanly) and looked mostly at thread safety. I didn't find a correctness bug.

Per-page cache: the slots are thread-local and each one still uses the old reuse rule (page, page_gen, cap_view_gen). An overlay answer is the thread's own copy of the bytes, so retiring a capture can't leave it pointing at freed memory. Keeping answers for 32 pages instead of 1 holds them longer, but nothing new can make one stale: anything that changes an answer already bumps the page gen or the view. The slot hash keeps neighbours and pages 32 apart in different slots, as the test says. hookshot_vk is LoadLibrary'd/dlopen'd, so on Linux the 132 KB of TLS is allocated lazily per thread. On Windows every thread gets it once the DLL is loaded (klimovich's point 4). Fine as is, just worth knowing.
Shadow in place: I traced the three cases and they end in the same state as before. A copied page that's already in the shadow is overwritten. A new page goes in while there's room. When the shadow is full and the sweep can't make room, the trailing loop drops the stale copies. The skip loop relies on c->slots being ascending indices into c->pages, which capture_make guarantees. Slots taken in this pass carry the current shadow_seq, so the in-loop sweep (seq + 64 < shadow_seq) can't free a buffer that's still waiting in pairs. slot.b.get() stays valid across rehashes because the 4 KB buffer lives outside the map node.
Helpers / memory model: a helper's writes into the shadow buffers happen-before its --*w.left under copy_helpers.mu. The caller sees left == 0 under the same mutex, then releases shadow_mu, so memory_as_submitted and memory_drop_shadow only ever see finished pages. Helpers take none of our locks, so waiting with pools_mu + shadow_mu held can't deadlock. The per-caller left is also a real fix: with the shared pending, a memory_capture_now on the replay thread and a game-thread copy_pages could each wait on the other's parts.

Two things before merge, both small, plus nits inline:

The new test passes -Werror, and against the MSVC CRT (clang targeting msvc) getenv raises -Wdeprecated-declarations, so it fails to build there, as klimovich found. Adding -D_CRT_SECURE_NO_WARNINGS to the compile line fixes it. I'm not sure whether the windows-2022 job's g++ (mingw) warns, but the flag costs nothing.
CI hasn't run on this branch yet (no checks reported), so the windows-2022 run_all.py --strict job needs to go green once a maintainer approves the workflow run.

On the perf side, klimovich's Hyrule Field A/B (fewer slow reads, no fps change where the replay is the limit) and your Castle Town numbers fit together: change 2 is where the gain comes from. Splitting it into in-place vs helpers, as he suggested, would still be good to know but shouldn't block this.

Thanks, this is a nice find, and the write-up makes it easy to check. I read both changes against main (5609af4, which only touches two unrelated bounds checks in vk_memory.cpp, so it merges cleanly) and looked mostly at thread safety. I didn't find a correctness bug. - **Per-page cache:** the slots are thread-local and each one still uses the old reuse rule (page, page_gen, cap_view_gen). An overlay answer is the thread's own copy of the bytes, so retiring a capture can't leave it pointing at freed memory. Keeping answers for 32 pages instead of 1 holds them longer, but nothing new can make one stale: anything that changes an answer already bumps the page gen or the view. The slot hash keeps neighbours and pages 32 apart in different slots, as the test says. hookshot_vk is LoadLibrary'd/dlopen'd, so on Linux the 132 KB of TLS is allocated lazily per thread. On Windows every thread gets it once the DLL is loaded (klimovich's point 4). Fine as is, just worth knowing. - **Shadow in place:** I traced the three cases and they end in the same state as before. A copied page that's already in the shadow is overwritten. A new page goes in while there's room. When the shadow is full and the sweep can't make room, the trailing loop drops the stale copies. The skip loop relies on `c->slots` being ascending indices into `c->pages`, which `capture_make` guarantees. Slots taken in this pass carry the current `shadow_seq`, so the in-loop sweep (`seq + 64 < shadow_seq`) can't free a buffer that's still waiting in `pairs`. `slot.b.get()` stays valid across rehashes because the 4 KB buffer lives outside the map node. - **Helpers / memory model:** a helper's writes into the shadow buffers happen-before its `--*w.left` under `copy_helpers.mu`. The caller sees `left == 0` under the same mutex, then releases `shadow_mu`, so `memory_as_submitted` and `memory_drop_shadow` only ever see finished pages. Helpers take none of our locks, so waiting with pools_mu + shadow_mu held can't deadlock. The per-caller `left` is also a real fix: with the shared `pending`, a `memory_capture_now` on the replay thread and a game-thread `copy_pages` could each wait on the other's parts. Two things before merge, both small, plus nits inline: 1. The new test passes `-Werror`, and against the MSVC CRT (clang targeting msvc) `getenv` raises `-Wdeprecated-declarations`, so it fails to build there, as klimovich found. Adding `-D_CRT_SECURE_NO_WARNINGS` to the compile line fixes it. I'm not sure whether the windows-2022 job's g++ (mingw) warns, but the flag costs nothing. 2. CI hasn't run on this branch yet (no checks reported), so the windows-2022 `run_all.py --strict` job needs to go green once a maintainer approves the workflow run. On the perf side, klimovich's Hyrule Field A/B (fewer slow reads, no fps change where the replay is the limit) and your Castle Town numbers fit together: change 2 is where the gain comes from. Splitting it into in-place vs helpers, as he suggested, would still be good to know but shouldn't block this.

gpu/vk/vk_memory.cpp

					
				@ -1371,1 +1345,4 @@

				    int* left = nullptr;   // (the caller's parts not done yet (copy_helpers.mu): a caller waits for its own, not another's too)

				};

				void copy_part(const CopyPart& w) {

				    if (w.pairs) {

Nit: nothing reads copy_helpers.pending anymore (both waits use left now), so it's just a counter. Drop it, or keep it with a comment saying it's only for debugging. Also nit: the helpers expression is now copied in copy_pages and copy_pairs. A small capture_copy_helpers() would keep the two from drifting apart, since they share one pool of threads that only the first caller starts.

Nit: nothing reads `copy_helpers.pending` anymore (both waits use `left` now), so it's just a counter. Drop it, or keep it with a comment saying it's only for debugging. Also nit: the `helpers` expression is now copied in `copy_pages` and `copy_pairs`. A small `capture_copy_helpers()` would keep the two from drifting apart, since they share one pool of threads that only the first caller starts.

gpu/vk/vk_memory.cpp

Not a bug, but this caller waits with pools_mu and shadow_mu held, and the queue is FIFO. If the game thread's copy_pages already queued its parts (guarded copies of ~9 MB, which go through ReadProcessMemory on Windows), the replay sits behind them under both locks while its own parts haven't started. Two cheap options: raise the threshold (klimovich's point; 64 pages inline is ~10-15 us), or have the waiting caller pop and run its own unstarted parts (those with left == &left) instead of only waiting. Either way the wait under the locks stays bounded by its own work.

Not a bug, but this caller waits with pools_mu and shadow_mu held, and the queue is FIFO. If the game thread's `copy_pages` already queued its parts (guarded copies of ~9 MB, which go through ReadProcessMemory on Windows), the replay sits behind them under both locks while its own parts haven't started. Two cheap options: raise the threshold (klimovich's point; 64 pages inline is ~10-15 us), or have the waiting caller pop and run its own unstarted parts (those with `left == &left`) instead of only waiting. Either way the wait under the locks stays bounded by its own work.

tests/test_capture_copy_helpers.py

					
				@ -0,0 +1,152 @@

				"""The capture's copy helpers: copy_pages and copy_pairs share them, and each call waits for its own parts only.

With -Werror, the two getenv calls in the extracted copy_pages/copy_pairs raise -Wdeprecated-declarations against the MSVC CRT, so the fixture doesn't build there. Adding "-D_CRT_SECURE_NO_WARNINGS" here (the game build passes it too) fixes it.

With `-Werror`, the two `getenv` calls in the extracted `copy_pages`/`copy_pairs` raise `-Wdeprecated-declarations` against the MSVC CRT, so the fixture doesn't build there. Adding `"-D_CRT_SECURE_NO_WARNINGS"` here (the game build passes it too) fixes it.

djs5008 commented

2026-11-10 05:40:58 +00:00

(Migrated from github.com)

Some data from Linux + NVIDIA that might help with change 1.

I'm on an RTX 5080 + Ryzen 9 9950X3D, Arch, driver 615.78, same as the author. On my machine that 4 KB copy is really expensive. In Kokiri it was about 35% of the Presentation Thread, and the thread was the bottleneck at 75 to 85% of a core. That's roughly 120 page copies a frame costing about 9 ms, so around 75 µs per copy. klimovich's Windows numbers show the same copies being basically free, so I think something about how that memory is mapped on NVIDIA Linux makes CPU reads from it very slow.

I tried a different approach: don't snapshot captured pages at all. A read of a captured page just copies the n bytes it needs under pools_mu, which the slow path already does. The one-page cache stays, but only for pages no capture holds. Retiring happens under pools_mu in take_capture, so nothing keeps a pointer into a buffer that could go away.

         if (page0 == page1) {
-            submitted_page.page = page0;
-            ...
-            std::memcpy(submitted_page.bytes, (const uint8_t*)b->second.map + (uint64_t)e.slot * 4096, 4096);
-            submitted_page.overlay = true;
+            submitted_page.view = 0;
+            if (from == 0) {
+                submitted_page.page = page0;
+                submitted_page.pgen = page_gen(page0);
+                submitted_page.view = cap_view_gen.load(std::memory_order_relaxed);
+            }
         }

(plus removing the overlay fast path at the top and the bytes/overlay fields)

Kokiri gameplay, same save and settings, DLSS on:

	before	after
median fps	38.7	55.9
lowest 5 s window	32.1	50.7
Presentation Thread	75 to 85%	60 to 65%

test_submitted_page.py and test_ub_memo.py pass. I haven't tested Windows, and from klimovich's numbers I wouldn't expect much there. This should stack with change 2, since that one is on the game thread. Happy to open it as a separate PR or leave it for you to fold in, whichever is easier.

Some data from Linux + NVIDIA that might help with change 1. I'm on an RTX 5080 + Ryzen 9 9950X3D, Arch, driver 615.78, same as the author. On my machine that 4 KB copy is really expensive. In Kokiri it was about 35% of the Presentation Thread, and the thread was the bottleneck at 75 to 85% of a core. That's roughly 120 page copies a frame costing about 9 ms, so around 75 µs per copy. klimovich's Windows numbers show the same copies being basically free, so I think something about how that memory is mapped on NVIDIA Linux makes CPU reads from it very slow. I tried a different approach: don't snapshot captured pages at all. A read of a captured page just copies the n bytes it needs under pools_mu, which the slow path already does. The one-page cache stays, but only for pages no capture holds. Retiring happens under pools_mu in take_capture, so nothing keeps a pointer into a buffer that could go away. ```diff if (page0 == page1) { - submitted_page.page = page0; - ... - std::memcpy(submitted_page.bytes, (const uint8_t*)b->second.map + (uint64_t)e.slot * 4096, 4096); - submitted_page.overlay = true; + submitted_page.view = 0; + if (from == 0) { + submitted_page.page = page0; + submitted_page.pgen = page_gen(page0); + submitted_page.view = cap_view_gen.load(std::memory_order_relaxed); + } } ``` (plus removing the overlay fast path at the top and the `bytes`/`overlay` fields) Kokiri gameplay, same save and settings, DLSS on: | | before | after | |---|---|---| | median fps | 38.7 | 55.9 | | lowest 5 s window | 32.1 | 50.7 | | Presentation Thread | 75 to 85% | 60 to 65% | test_submitted_page.py and test_ub_memo.py pass. I haven't tested Windows, and from klimovich's numbers I wouldn't expect much there. This should stack with change 2, since that one is on the game thread. Happy to open it as a separate PR or leave it for you to fold in, whichever is easier.

valonsodev commented

2026-11-10 09:50:21 +00:00

(Migrated from github.com)

Thanks all for the careful reviews and the measurements. Rebased on main (779bdcf), two commits added:

1. djs5008's change, folded in (perf(vk): a captured page's words read from the capture, not a 4 KB copy kept a thread, with djs5008 as co-author). A read of a captured page now copies only the bytes it asks for from the capture under pools_mu. The 32 slots keep only "this page is in no capture", so each thread's TLS for them is 768 bytes, not ~132 KB (klimovich's point 4).

I measured it where the 4 KB copy should be cheap: Linux, RTX 3060 Ti, capture buffers in host-cached memory (about 0.3 µs per 4 KB read here). OoT's Market on a scripted route; the only difference between the two builds was hookshot_vk.so; 4 rounds alternating after a warm-up:

	fps (median)	Presentation Thread CPU
this PR before	32.96 (32.0 / 33.0 / 33.3)	66.8 %
with djs5008's change	35.48 (34.1 / 35.2 / 35.8 / 35.8)	60.9 %

Every run with the change was faster than every run without it. Reads that go to the captures rose from ~0.3 M to ~1.0 M per run, so the lock is taken more often, and it's still cheaper than the 4 KB copy. (One "before" run hung at boot waiting on a GPU fence. That hang happened in runs before this PR too and isn't in this code.)

On the 75 µs per copy: I timed 4 KB CPU reads from each host-visible memory type on this card. Host-cached took 0.28 µs, host-visible uncached 12 µs, and device-local host-visible (ReBAR) 89 µs. So djs5008's 75 µs matches uncached reads, and on cached memory the change still wins, as the table shows.

2. Review fixes (perf(vk): the capture's copies split by one function; ...):

The test builds with -D_CRT_SECURE_NO_WARNINGS, like the other -Werror fixtures and the game's own Windows build.
copy_pages and copy_pairs are now one copy_split, with the helper count in one capture_copy_helpers(). copy_helpers.pending is gone.
Threshold 128 copies for both (was 64 for copy_pairs).
hckrman101's second option: once a caller has done its first part, it takes back and runs its own parts no helper has started yet, so the wait under pools_mu/shadow_mu is bounded by its own work and not by another caller's queued guest copies. The test checks it: with both helpers and another copy_pages caller held mid-copy, copy_pairs still returns (without the new loop it times out).
Market, 3 rounds alternating: 33.9 / 35.2 / 35.1 fps before and 34.2 / 35.3 / 33.9 after, within run-to-run noise, no hangs.

Not done: splitting change 2 into in-place vs helpers (klimovich's point 3).

CI still needs a maintainer to approve the workflow run.

Thanks all for the careful reviews and the measurements. Rebased on main (779bdcf), two commits added: **1. djs5008's change, folded in** (`perf(vk): a captured page's words read from the capture, not a 4 KB copy kept a thread`, with djs5008 as co-author). A read of a captured page now copies only the bytes it asks for from the capture under `pools_mu`. The 32 slots keep only "this page is in no capture", so each thread's TLS for them is 768 bytes, not ~132 KB (klimovich's point 4). I measured it where the 4 KB copy should be cheap: Linux, RTX 3060 Ti, capture buffers in host-cached memory (about 0.3 µs per 4 KB read here). OoT's Market on a scripted route; the only difference between the two builds was `hookshot_vk.so`; 4 rounds alternating after a warm-up: | | fps (median) | Presentation Thread CPU | |---|---|---| | this PR before | 32.96 (32.0 / 33.0 / 33.3) | 66.8 % | | with djs5008's change | **35.48** (34.1 / 35.2 / 35.8 / 35.8) | **60.9 %** | Every run with the change was faster than every run without it. Reads that go to the captures rose from ~0.3 M to ~1.0 M per run, so the lock is taken more often, and it's still cheaper than the 4 KB copy. (One "before" run hung at boot waiting on a GPU fence. That hang happened in runs before this PR too and isn't in this code.) On the 75 µs per copy: I timed 4 KB CPU reads from each host-visible memory type on this card. Host-cached took 0.28 µs, host-visible uncached 12 µs, and device-local host-visible (ReBAR) 89 µs. So djs5008's 75 µs matches uncached reads, and on cached memory the change still wins, as the table shows. **2. Review fixes** (`perf(vk): the capture's copies split by one function; ...`): - The test builds with `-D_CRT_SECURE_NO_WARNINGS`, like the other `-Werror` fixtures and the game's own Windows build. - `copy_pages` and `copy_pairs` are now one `copy_split`, with the helper count in one `capture_copy_helpers()`. `copy_helpers.pending` is gone. - Threshold 128 copies for both (was 64 for `copy_pairs`). - hckrman101's second option: once a caller has done its first part, it takes back and runs its own parts no helper has started yet, so the wait under `pools_mu`/`shadow_mu` is bounded by its own work and not by another caller's queued guest copies. The test checks it: with both helpers and another `copy_pages` caller held mid-copy, `copy_pairs` still returns (without the new loop it times out). - Market, 3 rounds alternating: 33.9 / 35.2 / 35.1 fps before and 34.2 / 35.3 / 33.9 after, within run-to-run noise, no hangs. Not done: splitting change 2 into in-place vs helpers (klimovich's point 3). CI still needs a maintainer to approve the workflow run.

👍 1

itaskaev2 commented

2026-11-10 12:12:56 +00:00

(Migrated from github.com)

Tested on Windows + NVIDIA, since the numbers so far are from Linux and from AMD.

Setup: Windows 11, Ryzen 7 5800X3D, RTX 4080 SUPER (driver 616.92), 4K output, DLSS, Auto resolution, replay inline. Kokiri Forest from the spawn, 16 camera swings, 130 s runs (the summary is of the camera part, from 30 s). Only hookshot_vk.dll differs: main at 779bdcf, this PR at 448bd27, and this PR with #1619 (non-temporal capture copies) merged on top. Same exe, the three arms alternating, two runs each.

	median frame	fps	p99	capture a submit
main `779bdcf`	19.1 / 19.8 ms	51.5 / 49	28.9 / 31.2 ms	0.69 / 0.73 ms
this PR	20.0 / 19.4 ms	49 / 50.5	30.5 / 31.3 ms	0.74 / 0.72 ms
this PR + #1619	19.5 / 19.1 ms	50 / 52	31.1 / 28.4 ms	0.57 / 0.57 ms

No measurable change on this machine: the spread is this scene's run-to-run noise (about ±0.4 ms). No flash lines in any run's log.
That fits the profile: on main, memory_read_as_submitted was 1.5 % of the Presentation Thread's samples (2.2 % of its running ones; all-thread sampler, 50 s of the same scene), with the capture buffers in host-cached memory. The 4 KB copy that cost ~35 % of the thread on Linux (djs5008's numbers) doesn't show up on Windows/NVIDIA, so here the change neither helps nor hurts.
Kokiri puts no texture storage into the capture ("0 texture pages copied" in the take's log line), so these runs don't exercise the shadow-pass part of the PR.
tests/run_all.py on Windows (llvm-mingw, clang 23): the capture tests pass (test_capture_copy_helpers, test_submitted_page, test_capture_slot_ownership, test_capture_copy_runs, ...). The only failures are the two that fail on main the same way on this PC: nx2/test_nup_paths.py (no symlink rights here) and test_precompile_compute_guard.py, which passes from a VS developer prompt.
It merges cleanly with #1619, and the two together ran fine: this PR's reads from the capture buffers come after a streaming copy there (from memory, not the cache), and that cost nothing measurable either.

Tested on Windows + NVIDIA, since the numbers so far are from Linux and from AMD. **Setup:** Windows 11, Ryzen 7 5800X3D, RTX 4080 SUPER (driver 616.92), 4K output, DLSS, Auto resolution, replay inline. Kokiri Forest from the spawn, 16 camera swings, 130 s runs (the summary is of the camera part, from 30 s). Only `hookshot_vk.dll` differs: main at 779bdcf, this PR at 448bd27, and this PR with #1619 (non-temporal capture copies) merged on top. Same exe, the three arms alternating, two runs each. | | median frame | fps | p99 | capture a submit | |---|---|---|---|---| | main 779bdcf | 19.1 / 19.8 ms | 51.5 / 49 | 28.9 / 31.2 ms | 0.69 / 0.73 ms | | this PR | 20.0 / 19.4 ms | 49 / 50.5 | 30.5 / 31.3 ms | 0.74 / 0.72 ms | | this PR + #1619 | 19.5 / 19.1 ms | 50 / 52 | 31.1 / 28.4 ms | 0.57 / 0.57 ms | - No measurable change on this machine: the spread is this scene's run-to-run noise (about ±0.4 ms). No flash lines in any run's log. - That fits the profile: on main, `memory_read_as_submitted` was 1.5 % of the Presentation Thread's samples (2.2 % of its running ones; all-thread sampler, 50 s of the same scene), with the capture buffers in host-cached memory. The 4 KB copy that cost ~35 % of the thread on Linux (djs5008's numbers) doesn't show up on Windows/NVIDIA, so here the change neither helps nor hurts. - Kokiri puts no texture storage into the capture ("0 texture pages copied" in the take's log line), so these runs don't exercise the shadow-pass part of the PR. - `tests/run_all.py` on Windows (llvm-mingw, clang 23): the capture tests pass (`test_capture_copy_helpers`, `test_submitted_page`, `test_capture_slot_ownership`, `test_capture_copy_runs`, ...). The only failures are the two that fail on main the same way on this PC: `nx2/test_nup_paths.py` (no symlink rights here) and `test_precompile_compute_guard.py`, which passes from a VS developer prompt. - It merges cleanly with #1619, and the two together ran fine: this PR's reads from the capture buffers come after a streaming copy there (from memory, not the cache), and that cost nothing measurable either.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 16:21:10 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks for the follow-up, and for folding in djs5008's change. Re-read at 448bd27.

Fixed since my last review:

The new test builds with -D_CRT_SECURE_NO_WARNINGS.
copy_pages/copy_pairs are one copy_split, the helper count lives in capture_copy_helpers(), and pending is gone.
The wait under pools_mu/shadow_mu: the threshold is 128 for both now, and the caller takes back its own unstarted parts (w.left == &left) after its first one, so it never waits behind another caller's queued guest copies. I checked the accounting: left only changes under copy_helpers.mu, a stolen part is erased from the queue before it runs, and the caller doesn't return before left == 0, so no helper can touch the stack left after the caller returns. The held-helpers case in the test covers it.
djs5008's change: only "this page is in no capture" is kept now (from == 0), so a kept answer is just guest memory, and a captured page's bytes are copied from the capture under pools_mu on every read. Nothing outlives its capture, and the TLS goes down to 768 bytes. The slot hash still keeps neighbours and pages 32 apart in different slots.

The shadow pass is unchanged from what I traced last time.

Two things before this can land, neither a code problem:

It conflicts with main. #1762 merged today and rewrote the same cache in memory_read_as_submitted: four round-robin entries that each still hold a 4 KB copy of the captured page, plus an RX_VK_SUBMITTED_PAGES switch, a whole pages kept counter, and a rewritten tests/test_submitted_page.py. All three conflicts in gpu/vk/vk_memory.cpp are in that function, and the test conflicts too. Main's other changes to the file since your base (779bdcf) merge cleanly. Your version covers what #1762 was after (two pages read in turn stay kept) and also drops the whole-page copy, so I'd resolve toward this PR's side: drop submitted_find/submitted_slot/the 4 KB bytes, and adapt the test. Keeping RX_VK_SUBMITTED_PAGES=1 as a way back to one slot would be a nice touch but isn't needed. Please merge main. A quick A/B in #1762's scene (Kokiri village view, where it went 50 -> 57 fps) against post-#1762 main would confirm that reading captured pages under the lock is at least as good as main's four kept copies there. For context, draft #1541 rewrites this same function again (16 hashed slots that still keep page copies), so whichever lands second will need a rebase.
CI still hasn't run on this branch. It needs a maintainer to approve the workflow so the windows-2022 run_all.py --strict job can go green.

Not blocking: splitting change 2 into in-place vs helpers (klimovich's point 3) is still open. It would be good to know, but it shouldn't hold this up.

Thanks for the follow-up, and for folding in djs5008's change. Re-read at 448bd27. Fixed since my last review: - The new test builds with `-D_CRT_SECURE_NO_WARNINGS`. - `copy_pages`/`copy_pairs` are one `copy_split`, the helper count lives in `capture_copy_helpers()`, and `pending` is gone. - The wait under `pools_mu`/`shadow_mu`: the threshold is 128 for both now, and the caller takes back its own unstarted parts (`w.left == &left`) after its first one, so it never waits behind another caller's queued guest copies. I checked the accounting: `left` only changes under `copy_helpers.mu`, a stolen part is erased from the queue before it runs, and the caller doesn't return before `left == 0`, so no helper can touch the stack `left` after the caller returns. The held-helpers case in the test covers it. - djs5008's change: only "this page is in no capture" is kept now (`from == 0`), so a kept answer is just guest memory, and a captured page's bytes are copied from the capture under `pools_mu` on every read. Nothing outlives its capture, and the TLS goes down to 768 bytes. The slot hash still keeps neighbours and pages 32 apart in different slots. The shadow pass is unchanged from what I traced last time. Two things before this can land, neither a code problem: 1. **It conflicts with main.** #1762 merged today and rewrote the same cache in `memory_read_as_submitted`: four round-robin entries that each still hold a 4 KB copy of the captured page, plus an `RX_VK_SUBMITTED_PAGES` switch, a `whole pages kept` counter, and a rewritten `tests/test_submitted_page.py`. All three conflicts in `gpu/vk/vk_memory.cpp` are in that function, and the test conflicts too. Main's other changes to the file since your base (779bdcf) merge cleanly. Your version covers what #1762 was after (two pages read in turn stay kept) and also drops the whole-page copy, so I'd resolve toward this PR's side: drop `submitted_find`/`submitted_slot`/the 4 KB `bytes`, and adapt the test. Keeping `RX_VK_SUBMITTED_PAGES=1` as a way back to one slot would be a nice touch but isn't needed. Please merge main. A quick A/B in #1762's scene (Kokiri village view, where it went 50 -> 57 fps) against post-#1762 main would confirm that reading captured pages under the lock is at least as good as main's four kept copies there. For context, draft #1541 rewrites this same function again (16 hashed slots that still keep page copies), so whichever lands second will need a rebase. 2. CI still hasn't run on this branch. It needs a maintainer to approve the workflow so the windows-2022 `run_all.py --strict` job can go green. Not blocking: splitting change 2 into in-place vs helpers (klimovich's point 3) is still open. It would be good to know, but it shouldn't hold this up.

gpu/vk/vk_memory.cpp

					
				@ -1080,1 +1076,4 @@

				// capture holds is not kept: its words are copied from the capture under pools_mu at each read, not the whole 4 KB page

				// into a copy of the thread's own (that copy was a third of the replay's thread in Kokiri Forest on NVIDIA Linux)

				constexpr int kSubmittedPages = 32;

				thread_local SubmittedPage submitted_pages[kSubmittedPages];

This is where main conflicts now: #1762 replaced the single submitted_page with submitted_pages[4] + submitted_find/submitted_slot (round-robin, still with the 4 KB bytes overlay) and RX_VK_SUBMITTED_PAGES. When you merge main, I'd keep this hashed, overlay-free table and drop main's version, along with its whole-page counter in the log line. tests/test_submitted_page.py needs the same resolution, since main's version extracts submitted_find(/submitted_slot(/submitted_ways(.

This is where main conflicts now: #1762 replaced the single `submitted_page` with `submitted_pages[4]` + `submitted_find`/`submitted_slot` (round-robin, still with the 4 KB `bytes` overlay) and `RX_VK_SUBMITTED_PAGES`. When you merge main, I'd keep this hashed, overlay-free table and drop main's version, along with its whole-page counter in the log line. `tests/test_submitted_page.py` needs the same resolution, since main's version extracts `submitted_find(`/`submitted_slot(`/`submitted_ways(`.

valonsodev commented

2026-11-10 17:53:29 +00:00

(Migrated from github.com)

Rebased on main (428818b, after #1762 and #1619). The conflicts with #1762 are resolved toward this branch: submitted_find/submitted_slot, the 4 KB bytes copy and the "whole pages kept" counter are gone. The new last commit keeps RX_VK_SUBMITTED_PAGES=1 as one slot for every page. tests/test_submitted_page.py checks both modes and builds with -D_CRT_SECURE_NO_WARNINGS. test_capture_copy_helpers stubs #1619's copy_fence. run_all.py test_submitted_page test_capture_copy_helpers test_capture_copy_nt test_capture_copy_runs: 4 passed (Linux, clang and gcc).

A/B in #1762's scene: Linux, RTX 3060 Ti (615.78), Ryzen 9 5900X. Kokiri village view from the Lost Woods bridge, standing still 25 s (2,950–3,570 draws). Main at f758d9f (with #1762) against this branch merged with it (the same changes as this rebase), both built the same way, one warm-up each, then 4 rounds alternating in both orders:

	fps	median
main (four kept copies)	32.80, 32.12, 33.24, 33.00	32.9
this PR	33.96, 34.12, 33.44, 33.56	33.8

This PR was ahead in every round (+0.2 to +2.0 fps). Main read from the captures ~370k times per 3M reads, copying a whole 4 KB page each time. This branch read from them ~1.1M times, each copying only the bytes asked for, under pools_mu.

Rebased on main (`428818b`, after #1762 and #1619). The conflicts with #1762 are resolved toward this branch: `submitted_find`/`submitted_slot`, the 4 KB `bytes` copy and the "whole pages kept" counter are gone. The new last commit keeps `RX_VK_SUBMITTED_PAGES=1` as one slot for every page. `tests/test_submitted_page.py` checks both modes and builds with `-D_CRT_SECURE_NO_WARNINGS`. `test_capture_copy_helpers` stubs #1619's `copy_fence`. `run_all.py test_submitted_page test_capture_copy_helpers test_capture_copy_nt test_capture_copy_runs`: 4 passed (Linux, clang and gcc). A/B in #1762's scene: Linux, RTX 3060 Ti (615.78), Ryzen 9 5900X. Kokiri village view from the Lost Woods bridge, standing still 25 s (2,950–3,570 draws). Main at `f758d9f` (with #1762) against this branch merged with it (the same changes as this rebase), both built the same way, one warm-up each, then 4 rounds alternating in both orders: | | fps | median | |---|---|---| | main (four kept copies) | 32.80, 32.12, 33.24, 33.00 | 32.9 | | this PR | 33.96, 34.12, 33.44, 33.56 | 33.8 | This PR was ahead in every round (+0.2 to +2.0 fps). Main read from the captures ~370k times per 3M reads, copying a whole 4 KB page each time. This branch read from them ~1.1M times, each copying only the bytes asked for, under `pools_mu`.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 18:06:35 +00:00

hckrman101 (Migrated from github.com) left a comment

Re-review at cfefd0e (rebased on 428818b, after #1762, #1619 and #1822).

#1762 conflict: resolved the way we suggested. submitted_find/submitted_slot, the 4 KB bytes copy and the "whole pages kept" counter are gone, and the hashed, copy-free 32-slot table stays. Against main the vk_memory.cpp diff deletes only #1762's submitted-page code. Nothing else of main's is lost, and #1619's copy_fence/NT path is untouched (the pairs path is plain memcpy, so it needs no fence). RX_VK_SUBMITTED_PAGES=1 is kept as one slot. It was never in docs/, so there's nothing to update there. The test now checks both modes.

A/B in #1762's scene: done, thanks. Kokiri village view, 4 alternating rounds against f758d9f: 32.9 → 33.8 fps median, ahead in every round. So dropping main's four kept copies costs nothing there.

New code: I re-read copy_split and the read path. left lives on the caller's stack and is only changed under copy_helpers.mu. Stolen parts are erased from the deque before they run, and the caller can't return before the last helper's decrement. The captured bytes are read from cap_bufs[...].map under pools_mu, and retire happens under the same lock, so nothing outlives its buffer. I found no problems.

Still open / small:

#1822 (merged before your rebase): the new tests/test_capture_copy_helpers.py doesn't declare RUN_ALL_ARGS = ["--cxx", "{cxx}"], unlike its siblings (test_capture_copy_nt, test_capture_copy_runs, test_capture_slot_ownership). So run_all.py runs it with its default g++ instead of the compiler it found, and doesn't skip it when there is none. test_submitted_page.py lacks it on main too, so you could add it to both while you're there.
Not blocking, as before: change 2 isn't split into in-place vs helpers (klimovich's point 3).
The PR description is out of date: it still says 128 KB of TLS, copy_pages/copy_pairs as two functions, and "on main at 0828863". Worth refreshing before merge.
CI: still hasn't run. All three workflows are action_required, so a maintainer has to approve them.

Approving. A maintainer still needs to approve and run the CI workflows before this merges.

Re-review at cfefd0e (rebased on 428818b, after #1762, #1619 and #1822). **#1762 conflict:** resolved the way we suggested. `submitted_find`/`submitted_slot`, the 4 KB `bytes` copy and the "whole pages kept" counter are gone, and the hashed, copy-free 32-slot table stays. Against main the vk_memory.cpp diff deletes only #1762's submitted-page code. Nothing else of main's is lost, and #1619's `copy_fence`/NT path is untouched (the pairs path is plain memcpy, so it needs no fence). `RX_VK_SUBMITTED_PAGES=1` is kept as one slot. It was never in docs/, so there's nothing to update there. The test now checks both modes. **A/B in #1762's scene:** done, thanks. Kokiri village view, 4 alternating rounds against f758d9f: 32.9 → 33.8 fps median, ahead in every round. So dropping main's four kept copies costs nothing there. **New code:** I re-read `copy_split` and the read path. `left` lives on the caller's stack and is only changed under `copy_helpers.mu`. Stolen parts are erased from the deque before they run, and the caller can't return before the last helper's decrement. The captured bytes are read from `cap_bufs[...].map` under `pools_mu`, and retire happens under the same lock, so nothing outlives its buffer. I found no problems. **Still open / small:** - **#1822 (merged before your rebase):** the new `tests/test_capture_copy_helpers.py` doesn't declare `RUN_ALL_ARGS = ["--cxx", "{cxx}"]`, unlike its siblings (`test_capture_copy_nt`, `test_capture_copy_runs`, `test_capture_slot_ownership`). So `run_all.py` runs it with its default `g++` instead of the compiler it found, and doesn't skip it when there is none. `test_submitted_page.py` lacks it on main too, so you could add it to both while you're there. - Not blocking, as before: change 2 isn't split into in-place vs helpers (klimovich's point 3). - The PR description is out of date: it still says 128 KB of TLS, `copy_pages`/`copy_pairs` as two functions, and "on main at 0828863". Worth refreshing before merge. - **CI:** still hasn't run. All three workflows are `action_required`, so a maintainer has to approve them. Approving. A maintainer still needs to approve and run the CI workflows before this merges.

gpu/vk/vk_memory.cpp

					
				@ -1120,1 +1096,4 @@

				    if (page0 == page1 && submitted_page_reuse(page0, page_gen(page0), view, submitted_page.page, submitted_page.pgen,

				                                               submitted_page.view))

				        return true;

				    static std::atomic<int64_t> reads{0}, pages_from{0};

Nit: "as before #1762" isn't quite the old behaviour. Before #1762 the one slot also kept a 4 KB copy of a captured page, and now it only keeps "in no capture". It's fine as an A/B switch, but maybe say "one slot" without the "as before".

tests/test_capture_copy_helpers.py

					
				@ -0,0 +14,4 @@

				import subprocess

				import tempfile

				ROOT = Path(__file__).resolve().parents[1]

Since #1822, siblings declare what run_all passes them. This one should have RUN_ALL_ARGS = ["--cxx", "{cxx}"] after the imports, as in test_capture_copy_nt.py / test_capture_copy_runs.py. Without it run_all calls it with no arguments, so it uses g++ even where only clang++ was found, and fails rather than skips with no compiler.

Since #1822, siblings declare what run_all passes them. This one should have `RUN_ALL_ARGS = ["--cxx", "{cxx}"]` after the imports, as in `test_capture_copy_nt.py` / `test_capture_copy_runs.py`. Without it run_all calls it with no arguments, so it uses `g++` even where only clang++ was found, and fails rather than skips with no compiler.

tests/test_submitted_page.py

Nit: main's version dropped RX_VK_SUBMITTED_PAGES from the env for this default run. With os.environ passed through, a shell that has it set to anything (say 4 from the #1762 days) takes the fixture's one-slot branch, while submitted_page_for only treats == 1 as one slot, so the test fails for a reason that isn't a bug. Stripping it like before (or checking atoi(...) == 1 in the fixture) avoids that.

Nit: main's version dropped `RX_VK_SUBMITTED_PAGES` from the env for this default run. With `os.environ` passed through, a shell that has it set to anything (say `4` from the #1762 days) takes the fixture's one-slot branch, while `submitted_page_for` only treats `== 1` as one slot, so the test fails for a reason that isn't a bug. Stripping it like before (or checking `atoi(...) == 1` in the fixture) avoids that.

Nitnek commented

2026-11-10 22:40:52 +00:00

(Migrated from github.com)

Windows/NVIDIA smoke test of the actual merged commit e63d30dbea2bc8b660a890dd1620489d50e61229: Ryzen 7 5800X, RTX 3070 8GB, driver 610.88, Windows 11 26300.9818.

Two independent launches loaded isolated copies of the same custom save/profile at the stationary castle approach, 1280×720 output, frame generation off. Each had at least 60 seconds of warm-up followed by approximately 60 seconds of measurement. The position/camera was checked before measurement; all eight measurement PNGs were inspected. No gross rendering corruption was visible in those samples, and no unexpected exit, device-loss or fatal-error marker was observed. The test processes were stopped deliberately at the measurement limit.

Observed throughput was 44.24 / 43.32 FPS, computed over equal 120-present intervals, retaining the slow intervals. This is not a measured speedup: I have no first-parent runtime control, and background desktop activity/instrumentation were not eliminated.

Caveat: the first cold preparation reported 14 failed programs out of 38,809 prepared (164.8 seconds total startup). The cached second launch started in 3.0 seconds and reported zero runtime failures, but it did not repeat the same preparation population; it does not erase those first 14 failures. I cannot attribute them to this change without a parent control.

This is limited compatibility evidence for this scene/hardware, not a general no-regression guarantee. Sparse still images cannot exclude single-frame flashes, and this test did not cover repeated scene transitions or a long session.

Windows/NVIDIA smoke test of the actual merged commit `e63d30dbea2bc8b660a890dd1620489d50e61229`: Ryzen 7 5800X, RTX 3070 8GB, driver 610.88, Windows 11 26300.9818. Two independent launches loaded isolated copies of the same custom save/profile at the stationary castle approach, 1280×720 output, frame generation off. Each had at least 60 seconds of warm-up followed by approximately 60 seconds of measurement. The position/camera was checked before measurement; all eight measurement PNGs were inspected. No gross rendering corruption was visible in those samples, and no unexpected exit, device-loss or fatal-error marker was observed. The test processes were stopped deliberately at the measurement limit. Observed throughput was **44.24 / 43.32 FPS**, computed over equal 120-present intervals, retaining the slow intervals. **This is not a measured speedup:** I have no first-parent runtime control, and background desktop activity/instrumentation were not eliminated. **Caveat:** the first cold preparation reported **14 failed programs out of 38,809 prepared** (164.8 seconds total startup). The cached second launch started in 3.0 seconds and reported zero runtime failures, but it did not repeat the same preparation population; it does not erase those first 14 failures. I cannot attribute them to this change without a parent control. This is limited compatibility evidence for this scene/hardware, not a general no-regression guarantee. Sparse still images cannot exclude single-frame flashes, and this test did not cover repeated scene transitions or a long session.

github-sync referenced this pull request

2026-11-12 14:41:17 +00:00

[Tracking] Linux and Steam Deck: GPU hangs, crashes, low FPS and build failures #1000

Commenting is not possible because the repository is archived.

No reviewers

hckrman101