perf(vk): keep four pages for the replay's reads as submitted, not one (Kokiri village view 50 -> 57 fps) #1762

github-sync · 2026-11-10T16:06:14Z

Joey-Magnus commented

2026-11-10 16:06:14 +00:00

(Migrated from github.com)

Kokiri Forest drops to ~50 fps whenever the camera faces the village, the "46 fps minimum, dips to 38" camera stutter reported on Reddit. The replay spends about a fifth of its time re-copying the same 4 KB captured pages, because its read cache holds only one page.

Cause. memory_read_as_submitted keeps the last page it read so the next uniform words skip pools_mu. A draw's handle reads (stage_handle → ub_read) take turns between its stages' uniform buffers on different pages. So with one page kept, each read evicted the other: lock, cap-table lookup, and a 4,096-byte memcpy of the captured page, every time. In a profile of that view (HOS_SAMPLE_ALL, scripts/crit.py), that memcpy (the 0x1000 copy into submitted_page.bytes) was ~22% of the Presentation thread's samples.

Change. Keep four pages per thread, replacing the oldest first. The reuse rule is unchanged: a page is reused only while its write generation and the capture view are the ones it was copied at (submitted_page_reuse), so a stamp or a new capture still drops all four. RX_VK_SUBMITTED_PAGES=1 restores the single page. The every-20,000-reads log line now also counts the whole pages copied.

Measured. Linux, RTX 4070 Ti (595.99.02), Ryzen 7 9800X3D. Integration build (main + open PRs), Kokiri village view standing still (2,224 draws), HOS_FPS_MAX=120, 25 s per run with HOS_PRESENT_TRACE, two passes in A-B…B-A order:

	fps	p50 frame	1% low	replay submit / frame	of it: programs
one page (`RX_VK_SUBMITTED_PAGES=1`)	50.0, 50.2	20.4 ms	39.0, 35.5	18.5 ms	5.7 ms
four pages	56.8, 56.9	17.6 ms	43.4, 42.5	16.0 ms	3.3 ms

+13.5% fps, and the slow-path reads fell ~3x. Turning off #1619's non-temporal capture copies (RX_VK_CAPTURE_NT=0) made no difference (49.7 fps), so that isn't a factor here.

Tests. tests/test_submitted_page.py now also builds submitted_find / submitted_slot from the source: two pages read in turn stay kept, a fifth replaces the oldest, a stamp or a new capture is not reused, and RX_VK_SUBMITTED_PAGES=1 keeps one. It still checks that the lookup comes before pools_mu. It fails on main and passes here. run_all.py: 241 passed, 0 failed. (With the build's tools on PATH, test_finish_output.py and test_linux_update.py fail on main too, unrelated.) The renderer builds standalone from this branch.

Not on screen: no visual change. Same frames, faster.

🤖 Generated with Claude Code

Kokiri Forest drops to ~50 fps whenever the camera faces the village, the "46 fps minimum, dips to 38" camera stutter reported on Reddit. The replay spends about a fifth of its time re-copying the same 4 KB captured pages, because its read cache holds only one page. **Cause.** `memory_read_as_submitted` keeps the last page it read so the next uniform words skip `pools_mu`. A draw's handle reads (`stage_handle → ub_read`) take turns between its stages' uniform buffers on different pages. So with one page kept, each read evicted the other: lock, cap-table lookup, and a 4,096-byte `memcpy` of the captured page, every time. In a profile of that view (HOS_SAMPLE_ALL, `scripts/crit.py`), that `memcpy` (the 0x1000 copy into `submitted_page.bytes`) was ~22% of the Presentation thread's samples. **Change.** Keep four pages per thread, replacing the oldest first. The reuse rule is unchanged: a page is reused only while its write generation and the capture view are the ones it was copied at (`submitted_page_reuse`), so a stamp or a new capture still drops all four. `RX_VK_SUBMITTED_PAGES=1` restores the single page. The every-20,000-reads log line now also counts the whole pages copied. **Measured.** Linux, RTX 4070 Ti (595.99.02), Ryzen 7 9800X3D. Integration build (main + open PRs), Kokiri village view standing still (2,224 draws), `HOS_FPS_MAX=120`, 25 s per run with `HOS_PRESENT_TRACE`, two passes in A-B…B-A order: | | fps | p50 frame | 1% low | replay submit / frame | of it: programs | |---|---|---|---|---|---| | one page (`RX_VK_SUBMITTED_PAGES=1`) | 50.0, 50.2 | 20.4 ms | 39.0, 35.5 | 18.5 ms | 5.7 ms | | four pages | 56.8, 56.9 | 17.6 ms | 43.4, 42.5 | 16.0 ms | 3.3 ms | +13.5% fps, and the slow-path reads fell ~3x. Turning off #1619's non-temporal capture copies (`RX_VK_CAPTURE_NT=0`) made no difference (49.7 fps), so that isn't a factor here. **Tests.** `tests/test_submitted_page.py` now also builds `submitted_find` / `submitted_slot` from the source: two pages read in turn stay kept, a fifth replaces the oldest, a stamp or a new capture is not reused, and `RX_VK_SUBMITTED_PAGES=1` keeps one. It still checks that the lookup comes before `pools_mu`. It fails on `main` and passes here. `run_all.py`: 241 passed, 0 failed. (With the build's tools on PATH, `test_finish_output.py` and `test_linux_update.py` fail on `main` too, unrelated.) The renderer builds standalone from this branch. Not on screen: no visual change. Same frames, faster. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) approved these changes

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

hckrman101 (Migrated from github.com) left a comment

Looks right to me. The key hasn't changed, it's still page + page_gen + cap_view_gen per entry, so a stamp or a newly published capture invalidates all four slots the same way it did the single one. The slow path only fills a slot after submitted_find misses for the current (page, gen, view), so you can't end up with two live entries for the same key. An entry with overlay == false still drops through to gpu_read and is accepted by the second find, same as before. The cache is thread_local, so parallel replay workers each get their own four slots and there's nothing to race on; st_page_copies is atomic. Reads that span two pages still skip the cache and go through the locked per-page path, unchanged. RX_VK_SUBMITTED_PAGES=1 gives back the old behaviour exactly (next % 1).

The fps number seems plausible from the code. With uniform reads alternating between two pages, every read on main paid for pools_mu, the cap-table lookup and a 4 KB memcpy. With four slots that becomes a scan of four entries. The test is auto-discovered by run_all.py, and it does test the find/slot code from the source and both env settings. CI was still pending when I looked. main has moved 21 commits but hasn't touched this function.

Heads-up: #1375 (32 hashed slots, and it drops the 4 KB page copy completely) and #1541 (16 hashed slots plus per-block validity) both rewrite this same SubmittedPage cache in different ways. Whichever one lands first will conflict with the others, so it's probably worth picking one approach.

A couple of nits inline, none of them blocking.

Looks right to me. The key hasn't changed, it's still page + page_gen + cap_view_gen per entry, so a stamp or a newly published capture invalidates all four slots the same way it did the single one. The slow path only fills a slot after `submitted_find` misses for the current (page, gen, view), so you can't end up with two live entries for the same key. An entry with `overlay == false` still drops through to `gpu_read` and is accepted by the second find, same as before. The cache is thread_local, so parallel replay workers each get their own four slots and there's nothing to race on; `st_page_copies` is atomic. Reads that span two pages still skip the cache and go through the locked per-page path, unchanged. `RX_VK_SUBMITTED_PAGES=1` gives back the old behaviour exactly (`next % 1`). The fps number seems plausible from the code. With uniform reads alternating between two pages, every read on main paid for pools_mu, the cap-table lookup and a 4 KB memcpy. With four slots that becomes a scan of four entries. The test is auto-discovered by run_all.py, and it does test the find/slot code from the source and both env settings. CI was still pending when I looked. main has moved 21 commits but hasn't touched this function. Heads-up: #1375 (32 hashed slots, and it drops the 4 KB page copy completely) and #1541 (16 hashed slots plus per-block validity) both rewrite this same `SubmittedPage` cache in different ways. Whichever one lands first will conflict with the others, so it's probably worth picking one approach. A couple of nits inline, none of them blocking.

gpu/vk/vk_memory.cpp

					
				@ -1071,0 +1075,4 @@

				thread_local unsigned submitted_next;

				int submitted_ways() {

				    static const int v = getenv("RX_VK_SUBMITTED_PAGES") ? std::clamp(atoi(getenv("RX_VK_SUBMITTED_PAGES")), 1, kSubmittedPages)

				                                                         : kSubmittedPages;

Nit: a non-numeric value (e.g. RX_VK_SUBMITTED_PAGES=off) goes through atoi -> 0 -> clamp to 1, so it silently means single-page. That's probably what you'd want anyway, just mentioning it. Calling getenv once into a local would also be a bit tidier.

Nit: a non-numeric value (e.g. `RX_VK_SUBMITTED_PAGES=off`) goes through `atoi` -> 0 -> clamp to 1, so it silently means single-page. That's probably what you'd want anyway, just mentioning it. Calling `getenv` once into a local would also be a bit tidier.

gpu/vk/vk_memory.cpp

					
				@ -1071,0 +1087,4 @@

				}

				/// the entry a new page goes into: the oldest, in turn

				SubmittedPage& submitted_slot() { return submitted_pages[submitted_next++ % (unsigned)submitted_ways()]; }

				std::atomic<int64_t> st_page_copies{0};

Nit: this is FIFO by insertion, not LRU, because a hit in the fast path doesn't refresh the slot. That's fine for the two-page ping-pong. But when one hot page is mixed with 4+ others, the hot page still gets evicted every 4 misses. Also, a refill of page X after a stamp takes a fresh slot while X's stale entry stays in another one until it rotates out. Reusing a slot whose .page matches before falling back to next++ would avoid that. Neither of these is a correctness problem.

Nit: this is FIFO by insertion, not LRU, because a hit in the fast path doesn't refresh the slot. That's fine for the two-page ping-pong. But when one hot page is mixed with 4+ others, the hot page still gets evicted every 4 misses. Also, a refill of page X after a stamp takes a fresh slot while X's stale entry stays in another one until it rotates out. Reusing a slot whose `.page` matches before falling back to `next++` would avoid that. Neither of these is a correctness problem.