perf(vk): reduce renderer CPU work and add bounded diagnostics #1541

Open

mikaba-eu wants to merge 19 commits from mikaba-eu/pr/renderer-performance into main

mikaba-eu commented

2026-11-10 07:31:08 +00:00

(Migrated from github.com)

Preserve the Linux process CPU allowance, reserve the presentation core, and keep background workers off it while respecting explicit affinity settings.
Reuse descriptor writers and validated resolved handles, and cache stage lowering by full content with bounded storage.
Correct ordering for overlapping deferred uniform writes and preserve capture geometry, readback dimensions, and capture resource limits.
Add opt-in stage/present traces and offline analysis, including both legacy three-column and current seven-column native-present CSVs.

Merged upstream ddafc1f9, retaining its topology/color-type helpers and shader-answer validation changes. The earlier local page-copy experiments remain disabled; this PR does not contain the experimental material/packet/query redesign or external neural processing.

Validation: the refreshed branch passes the full Linux source suite: 377 passed, 0 failed, 14 skipped, using the bundled Clang toolchain. The subsequent HDR/FSR update also passed all 17 affected menu/upscaler checks. The final builder-only upstream update passed 11 affected checks. The skipped checks require unavailable platform/hardware/tool dependencies. Earlier combined-branch gameplay evidence is recorded in docs/tested-configurations.md; it is not an isolated speedup measurement for this refreshed PR. Windows and AMD gameplay were not tested locally.

The final source includes upstream 2e62a977; its release-only changes passed both affected release/update checks.

Repeated renderer preparation and avoidable CPU contention add work to frame submission, while existing traces make it difficult to identify where time is spent. This PR reduces that repeated work and adds bounded diagnostics without promising a particular gameplay frame rate. Implemented with GPT-6 Astra medium & xhigh. - Preserve the Linux process CPU allowance, reserve the presentation core, and keep background workers off it while respecting explicit affinity settings. - Reuse descriptor writers and validated resolved handles, and cache stage lowering by full content with bounded storage. - Correct ordering for overlapping deferred uniform writes and preserve capture geometry, readback dimensions, and capture resource limits. - Add opt-in stage/present traces and offline analysis, including both legacy three-column and current seven-column native-present CSVs. Merged upstream `ddafc1f9`, retaining its topology/color-type helpers and shader-answer validation changes. The earlier local page-copy experiments remain disabled; this PR does not contain the experimental material/packet/query redesign or external neural processing. Validation: the refreshed branch passes the full Linux source suite: **377 passed, 0 failed, 14 skipped**, using the bundled Clang toolchain. The subsequent HDR/FSR update also passed all 17 affected menu/upscaler checks. The final builder-only upstream update passed 11 affected checks. The skipped checks require unavailable platform/hardware/tool dependencies. Earlier combined-branch gameplay evidence is recorded in `docs/tested-configurations.md`; it is not an isolated speedup measurement for this refreshed PR. Windows and AMD gameplay were not tested locally. The final source includes upstream `2e62a977`; its release-only changes passed both affected release/update checks.

hckrman101 commented

2026-11-10 17:49:32 +00:00

(Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

mikaba-eu commented

2026-11-10 19:14:29 +00:00

(Migrated from github.com)

Merged current main (1392ea3) and resolved the conflicts, preserving upstream replacements where they supersede our changes. Migrated local tests to the new per-test runner declarations. Independent source suites and the combined suite pass; fresh Linux gameplay results and limitations are recorded in the updated description. Hyrule Field and a warm Forest run completed the short movement tests, but camera hitches remain; earlier memory-guard stops and the incomplete Market event are not reported as passes. AI-assisted development: GPT-6-Astra (medium and xhigh).

Merged current main (`1392ea3`) and resolved the conflicts, preserving upstream replacements where they supersede our changes. Migrated local tests to the new per-test runner declarations. Independent source suites and the combined suite pass; fresh Linux gameplay results and limitations are recorded in the updated description. Hyrule Field and a warm Forest run completed the short movement tests, but camera hitches remain; earlier memory-guard stops and the incomplete Market event are not reported as passes. AI-assisted development: GPT-6-Astra (medium and xhigh).

mikaba-eu commented

2026-11-10 19:39:27 +00:00

(Migrated from github.com)

Refreshed again to main 117eae7 and resolved the new Anti-Lag source-fixture conflict. Complete Linux source suite re-run: 320 passed, 0 failed, 18 skipped. Existing gameplay evidence remains explicitly tied to its tested build.

Refreshed again to main `117eae7` and resolved the new Anti-Lag source-fixture conflict. Complete Linux source suite re-run: 320 passed, 0 failed, 18 skipped. Existing gameplay evidence remains explicitly tied to its tested build.

hckrman101 commented

2026-11-11 15:51:04 +00:00

(Migrated from github.com)

Heads-up: this conflicts with main now. Could you rebase onto main and resolve the conflicts? Thanks!

Joey-Magnus (Migrated from github.com) requested changes

2026-11-11 20:06:08 +00:00

Joey-Magnus (Migrated from github.com) left a comment

There's useful work in here, and the tests you changed get stronger rather than weaker. test_program_answer_domains now checks that resolved_answers_valid agrees with still_valid in every case, which is a nice touch. But in its current form I don't think this can be reviewed or merged as one unit. I covered the riskiest parts: the Linux affinity changes, the stage-lowering cache, the pick validation, the switches and the test edits. I didn't review the capture, trace or analysis scripts in depth.

blocking: merge conflicts. Against current origin/main it conflicts in gpu/vk/vk_draw.cpp, vk_memory.cpp, vk_replay.cpp, vk_shader.cpp, vk_texture.cpp, tests/test_antilag.py and tests/test_present_semaphores.py. hckrman101 asked for a rebase again today. With this much of the replay touched, a re-resolution is effectively a new change and needs its own test pass.

blocking: scope. CONTRIBUTING asks for one topic per PR, and that a bug fix not carry refactors. This PR has at least six independent changes with different risk profiles:

Linux thread placement (host/hos_thread.c, host/posix/win32_posix.c, gpu/vk/vk_thread_affinity.h, vk_jobs.cpp).
The stage-lowering cache (vk_stage_cache.h, vk_shader.cpp read_code).
Pick revalidation from the resolved handles (vk_shader.cpp resolved_answers_valid).
The ordering fix for overlapping deferred uniform writes. That one is a correctness fix and should land on its own, with a before/after.
Capture and readback size limits.
The stage/present traces and three new scripts.

Each could merge (or be reverted) independently. Part of the Linux affinity work is already on main: process_affinity taken before main exists there now. Please split along those lines, ideally starting with the uniform ordering fix.

should fix: new switches default to on, and none are documented where switches live. HOS_PIN_RENDER (host/hos_thread.c:774, on by default) changes where the Presentation Thread runs on every Linux machine. RX_VK_STAGE_CACHE (gpu/vk/vk_shader.cpp:165) is on by default. RX_VK_PICK_REREAD, RX_VK_STAGE_TRACE, RX_VK_STAGE_TRACE_ROWS, RX_VK_SLOW_STAGE_US, RX_VK_PRESENT_STAGES, RX_VK_CAPTURE_CPU_UNCACHED and RX_VK_SHADOW_COMPARE_STATS are also new. docs/development.md's switch table has none of them; two appear only in the new docs/renderer-performance.md. They are also read with atoi/presence rather than env_on (#2042), so HOS_PIN_RENDER=off would leave pinning on. CONTRIBUTING also says new switches are "off, or neutral, unless asked for". A default-on placement change on Linux needs measurements on the Deck and on a desktop with few cores, given #2263's findings about how sensitive this is.

should fix gpu/vk/vk_shader.cpp:167: static thread_local StageCache<Code, kControlSize> cache(64u << 20) gives each thread its own 64 MB budget (up to 4096 entries). read_code runs on every thread that makes programs: the recording threads, the parallel-replay workers, and the precompile pool at :2352, which starts hardware_concurrency() threads. So the worst case is 64 MB times the thread count, and a stage lowered on one thread is lowered again on every other. A single shared cache under a lock (lowering is far costlier than the lookup) with one process-wide budget would bound memory and hit more often. If per-thread is deliberate, please give the worst-case total for a 16-thread CPU and for the Deck.

should fix host/posix/win32_posix.c:312: main's GetProcessAffinityMask falls back to reading the mask again when the constructor didn't run or its call failed. This version returns FALSE instead. Callers such as hos_thread_pin and #2263's hos_short_sleep_timer then see an unknown processor count. Please keep main's fallback when you resolve this file.

question gpu/vk/vk_shader.cpp:2218-2223: resolved_answers_valid matches still_valid given the same handles, which the test now proves. But answer_handles now stores the full handle, sampler bits included, instead of & 0xFFFFF. So a sampler-only change now counts as "differ" and goes to revalidation, where before it was a direct hit. That's correct, but was the hit rate in Kokiri measured before and after? Otherwise this part may cost more than it saves.

There's useful work in here, and the tests you changed get stronger rather than weaker. `test_program_answer_domains` now checks that `resolved_answers_valid` agrees with `still_valid` in every case, which is a nice touch. But in its current form I don't think this can be reviewed or merged as one unit. I covered the riskiest parts: the Linux affinity changes, the stage-lowering cache, the pick validation, the switches and the test edits. I didn't review the capture, trace or analysis scripts in depth. **blocking**: merge conflicts. Against current `origin/main` it conflicts in `gpu/vk/vk_draw.cpp`, `vk_memory.cpp`, `vk_replay.cpp`, `vk_shader.cpp`, `vk_texture.cpp`, `tests/test_antilag.py` and `tests/test_present_semaphores.py`. hckrman101 asked for a rebase again today. With this much of the replay touched, a re-resolution is effectively a new change and needs its own test pass. **blocking**: scope. CONTRIBUTING asks for one topic per PR, and that a bug fix not carry refactors. This PR has at least six independent changes with different risk profiles: 1. Linux thread placement (`host/hos_thread.c`, `host/posix/win32_posix.c`, `gpu/vk/vk_thread_affinity.h`, `vk_jobs.cpp`). 2. The stage-lowering cache (`vk_stage_cache.h`, `vk_shader.cpp` `read_code`). 3. Pick revalidation from the resolved handles (`vk_shader.cpp` `resolved_answers_valid`). 4. The ordering fix for overlapping deferred uniform writes. That one is a correctness fix and should land on its own, with a before/after. 5. Capture and readback size limits. 6. The stage/present traces and three new scripts. Each could merge (or be reverted) independently. Part of the Linux affinity work is already on main: `process_affinity` taken before main exists there now. Please split along those lines, ideally starting with the uniform ordering fix. **should fix**: new switches default to on, and none are documented where switches live. `HOS_PIN_RENDER` (`host/hos_thread.c:774`, on by default) changes where the Presentation Thread runs on every Linux machine. `RX_VK_STAGE_CACHE` (`gpu/vk/vk_shader.cpp:165`) is on by default. `RX_VK_PICK_REREAD`, `RX_VK_STAGE_TRACE`, `RX_VK_STAGE_TRACE_ROWS`, `RX_VK_SLOW_STAGE_US`, `RX_VK_PRESENT_STAGES`, `RX_VK_CAPTURE_CPU_UNCACHED` and `RX_VK_SHADOW_COMPARE_STATS` are also new. `docs/development.md`'s switch table has none of them; two appear only in the new `docs/renderer-performance.md`. They are also read with `atoi`/presence rather than `env_on` (#2042), so `HOS_PIN_RENDER=off` would leave pinning on. CONTRIBUTING also says new switches are "off, or neutral, unless asked for". A default-on placement change on Linux needs measurements on the Deck and on a desktop with few cores, given #2263's findings about how sensitive this is. **should fix** `gpu/vk/vk_shader.cpp:167`: `static thread_local StageCache<Code, kControlSize> cache(64u << 20)` gives each thread its own 64 MB budget (up to 4096 entries). `read_code` runs on every thread that makes programs: the recording threads, the parallel-replay workers, and the precompile pool at `:2352`, which starts `hardware_concurrency()` threads. So the worst case is 64 MB times the thread count, and a stage lowered on one thread is lowered again on every other. A single shared cache under a lock (lowering is far costlier than the lookup) with one process-wide budget would bound memory and hit more often. If per-thread is deliberate, please give the worst-case total for a 16-thread CPU and for the Deck. **should fix** `host/posix/win32_posix.c:312`: main's `GetProcessAffinityMask` falls back to reading the mask again when the constructor didn't run or its call failed. This version returns `FALSE` instead. Callers such as `hos_thread_pin` and #2263's `hos_short_sleep_timer` then see an unknown processor count. Please keep main's fallback when you resolve this file. **question** `gpu/vk/vk_shader.cpp:2218-2223`: `resolved_answers_valid` matches `still_valid` given the same handles, which the test now proves. But `answer_handles` now stores the full handle, sampler bits included, instead of `& 0xFFFFF`. So a sampler-only change now counts as "differ" and goes to revalidation, where before it was a direct hit. That's correct, but was the hit rate in Kokiri measured before and after? Otherwise this part may cost more than it saves.

Joey-Magnus (Migrated from github.com) reviewed

2026-11-11 21:45:56 +00:00

Joey-Magnus (Migrated from github.com) left a comment

No new commits since my review at 131b0a3, so every point there still stands. The branch conflicts with current main in the same seven files: vk_draw.cpp, vk_memory.cpp, vk_replay.cpp, vk_shader.cpp, vk_texture.cpp, tests/test_antilag.py and tests/test_present_semaphores.py. None of its pieces has landed separately: main has no vk_stage_cache.h, resolved_answers_valid, HOS_PIN_RENDER or RX_VK_STAGE_CACHE.

The still-open points are:

the split into separate PRs, starting with the deferred-uniform ordering fix;
the switches that default to on and aren't read with env_on;
the 64 MB stage cache per thread at vk_shader.cpp:167;
the dropped GetProcessAffinityMask fallback at win32_posix.c:312.

One addition: #2172 is stacked on this branch and contains all of its commits. Splitting this PR means #2172 has to be rebased onto whichever pieces land, so it's worth planning those two together.

No new commits since my review at 131b0a3, so every point there still stands. The branch conflicts with current main in the same seven files: `vk_draw.cpp`, `vk_memory.cpp`, `vk_replay.cpp`, `vk_shader.cpp`, `vk_texture.cpp`, `tests/test_antilag.py` and `tests/test_present_semaphores.py`. None of its pieces has landed separately: main has no `vk_stage_cache.h`, `resolved_answers_valid`, `HOS_PIN_RENDER` or `RX_VK_STAGE_CACHE`. The still-open points are: - the split into separate PRs, starting with the deferred-uniform ordering fix; - the switches that default to on and aren't read with `env_on`; - the 64 MB stage cache per thread at `vk_shader.cpp:167`; - the dropped `GetProcessAffinityMask` fallback at `win32_posix.c:312`. One addition: #2172 is stacked on this branch and contains all of its commits. Splitting this PR means #2172 has to be rebased onto whichever pieces land, so it's worth planning those two together.

Commenting is not possible because the repository is archived.

No reviewers

Joey-Magnus