Skip to content

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

Open
mikaba-eu wants to merge 19 commits into
NotRexed:mainfrom
mikaba-eu:pr/renderer-performance
Open

mikaba-eu wants to merge 19 commits into
NotRexed:mainfrom
mikaba-eu:pr/renderer-performance

Conversation

@mikaba-eu

@mikaba-eu mikaba-eu commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

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.

Preserve submitted-memory and texture-generation validation while reusing stage lowering, descriptor state and resolved shader handles. Restore Linux synchronous presentation affinity, order overlapping uniform writes, and preserve capture geometry. Include authored regression tests and opt-in timing tools without changing upstream compute preloading or presentation recovery.

Assisted-by: GPT-6-Astra (medium and xhigh)
Co-Authored-By: OpenAI Codex <codex@openai.com>
Assisted-by: GPT-6-Astra (medium and xhigh)
Co-Authored-By: OpenAI Codex <codex@openai.com>
@mikaba-eu

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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.

@mikaba-eu mikaba-eu changed the title Reduce renderer CPU work and add bounded stage diagnostics perf(vk): reduce renderer CPU work and add bounded diagnostics Nov 11, 2026
@mikaba-eu
mikaba-eu marked this pull request as ready for review November 11, 2026 06:00

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

DevL0rd added a commit to DevL0rd/Hookshot that referenced this pull request Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants