perf(vk): 6 background workers on machines with 16 or more hardware threads #2440

Merged
lucasmazzoni45-del merged 1 commit from perf-workers-16-threads into main 2026-11-12 03:04:51 +00:00
lucasmazzoni45-del commented 2026-11-11 19:16:02 +00:00 (Migrated from github.com)

The workers (shader translation, pipelines) were a third of the hardware threads, clamped to 2-4. With 16 or more hardware threads this uses 6, which leaves ten threads for the game's six virtual cores, the replay and the presentation. RX_VK_WORKERS=<n> sets the count (1 to 16). Only the count changes. Not measured alone; a Ryzen 7 5800X3D (8c/16t) is the only machine I ran it on.

Tested: the Vulkan back end builds (llvm-mingw, RX_VK_ONLY) on top of v2.1.0, and a full build of that tree boots to the title at 60 fps on an RX 7800 XT / Ryzen 7 5800X3D (Windows 11, driver 0x8001a0) with the new log lines present. The repository test suite was not run, and I have not yet played a long session on this build, so the effect in real play is not shown. Related data: #995.

🤖 Generated with Claude Code

The workers (shader translation, pipelines) were a third of the hardware threads, clamped to 2-4. With 16 or more hardware threads this uses 6, which leaves ten threads for the game's six virtual cores, the replay and the presentation. `RX_VK_WORKERS=<n>` sets the count (1 to 16). Only the count changes. Not measured alone; a Ryzen 7 5800X3D (8c/16t) is the only machine I ran it on. **Tested:** the Vulkan back end builds (llvm-mingw, `RX_VK_ONLY`) on top of v2.1.0, and a full build of that tree boots to the title at 60 fps on an RX 7800 XT / Ryzen 7 5800X3D (Windows 11, driver 0x8001a0) with the new log lines present. The repository test suite was **not** run, and I have not yet played a long session on this build, so the effect in real play is not shown. Related data: #995. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 20:08:53 +00:00
Joey-Magnus (Migrated from github.com) left a comment

The change is small and doesn't break anything I could check. tests/test_jobs_failure.py, tests/test_jobs_wait_runs_queued.py and tests/test_ub_threads.py pass at this head. The Intel freeze path (#496) goes through jobs_push_serial and isn't affected. CI hasn't run on this PR yet (only the title check), so this is the first suite run it has had. It isn't ready to merge as a perf change yet, though:

  • should fix No measurement. docs/workflow.md asks for a measurement on any performance claim ("Measure, don't guess", and numbers from hookshot_log.txt for performance). The description says the change wasn't measured on its own and no long session was played. Please compare 4 against 6 workers on the 5800X3D: first-start shader preparation time, and the stutter or 1% low in an area that compiles pipelines while you play (Kokiri Forest → Lost Woods, or Hyrule Field). Add the rows to docs/tested-configurations.md. More workers mean more pipelines compiling at once in the driver, so peak memory and contention with the game's six virtual cores matter as well as throughput. A low-RAM 16-thread machine is a case to keep in mind.
  • should fix gpu/vk/vk_jobs.cpp:77 counts std::thread::hardware_concurrency(). Main now has usable_cpus() (gpu/vk/vk_internal.h, from #2323), which counts the CPUs the process may actually run on: its affinity mask, and groups beyond 64. After a rebase, size the pool from that, e.g. const unsigned hw = usable_cpus();. Otherwise a process limited to fewer CPUs still gets 6 workers.
  • should fix RX_VK_WORKERS (line 79) is a new switch with no documentation. CONTRIBUTING asks for new switches to be documented where the others are, so add a row to the table in docs/development.md. Also note that a value that isn't a number (atoi → 0) quietly becomes 1 worker.
  • minor The file's header comment (line 1, "As many as a third of the hardware threads (2 to 4)") is now out of date.
The change is small and doesn't break anything I could check. `tests/test_jobs_failure.py`, `tests/test_jobs_wait_runs_queued.py` and `tests/test_ub_threads.py` pass at this head. The Intel freeze path (#496) goes through `jobs_push_serial` and isn't affected. CI hasn't run on this PR yet (only the title check), so this is the first suite run it has had. It isn't ready to merge as a `perf` change yet, though: - **should fix** No measurement. `docs/workflow.md` asks for a measurement on any performance claim ("Measure, don't guess", and numbers from `hookshot_log.txt` for performance). The description says the change wasn't measured on its own and no long session was played. Please compare 4 against 6 workers on the 5800X3D: first-start shader preparation time, and the stutter or 1% low in an area that compiles pipelines while you play (Kokiri Forest → Lost Woods, or Hyrule Field). Add the rows to `docs/tested-configurations.md`. More workers mean more pipelines compiling at once in the driver, so peak memory and contention with the game's six virtual cores matter as well as throughput. A low-RAM 16-thread machine is a case to keep in mind. - **should fix** `gpu/vk/vk_jobs.cpp:77` counts `std::thread::hardware_concurrency()`. Main now has `usable_cpus()` (`gpu/vk/vk_internal.h`, from #2323), which counts the CPUs the process may actually run on: its affinity mask, and groups beyond 64. After a rebase, size the pool from that, e.g. `const unsigned hw = usable_cpus();`. Otherwise a process limited to fewer CPUs still gets 6 workers. - **should fix** `RX_VK_WORKERS` (line 79) is a new switch with no documentation. CONTRIBUTING asks for new switches to be documented where the others are, so add a row to the table in `docs/development.md`. Also note that a value that isn't a number (`atoi` → 0) quietly becomes 1 worker. - **minor** The file's header comment (line 1, "As many as a third of the hardware threads (2 to 4)") is now out of date.
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 21:26:37 +00:00
Joey-Magnus (Migrated from github.com) left a comment
  • vk_jobs.cpp:77 counts hardware_concurrency(). Main now has usable_cpus() (vk_internal.h:317), so an affinity-limited process still gets 6 workers.
  • It changes the default on every 16+ thread PC without a measurement. A 4-vs-6 comparison would settle it.
  • RX_VK_WORKERS isn't documented, a non-numeric value silently becomes 1, and the header comment on line 1 is out of date.
- `vk_jobs.cpp:77` counts `hardware_concurrency()`. Main now has `usable_cpus()` (`vk_internal.h:317`), so an affinity-limited process still gets 6 workers. - It changes the default on every 16+ thread PC without a measurement. A 4-vs-6 comparison would settle it. - `RX_VK_WORKERS` isn't documented, a non-numeric value silently becomes 1, and the header comment on line 1 is out of date.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 22:19:24 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks. The code itself is fine: a 3-line change to the pool size, workers still run below normal priority, and the Intel serial path (#496) is untouched. I agree with all of @Joey-Magnus's points and won't repeat them in detail; they are all still open at this head:

  • size from usable_cpus() (on main since #2323, already reachable through vk_internal.h) instead of hardware_concurrency(), so an affinity-limited process doesn't get 6 workers. The branch is 65 commits behind main, so a rebase is needed for that anyway;
  • a 4-vs-6 measurement on the 5800X3D (first-start preparation time and pipeline-compile stutter in play), since this changes the default on every 16+ thread PC;
  • document RX_VK_WORKERS in docs/development.md, and the header comment on line 1 still says "2 to 4".

On the sibling PRs (#2437, #2438): no code overlap, they touch other files and merge independently. They share the same gap though: none is measured alone on v2.1.0 and the test suite wasn't run on any of them. One interaction worth keeping in mind when you measure: #2437 starts up to 3 extra detached threads for its capture-buffer spares, beside these workers, at exactly the moments (scene loads) when pipelines compile. If you run the 4-vs-6 comparison, doing it without #2437 applied keeps the numbers attributable to this change.

CI has only run the title check so far.

Thanks. The code itself is fine: a 3-line change to the pool size, workers still run below normal priority, and the Intel serial path (#496) is untouched. I agree with all of @Joey-Magnus's points and won't repeat them in detail; they are all still open at this head: - size from `usable_cpus()` (on main since #2323, already reachable through `vk_internal.h`) instead of `hardware_concurrency()`, so an affinity-limited process doesn't get 6 workers. The branch is 65 commits behind main, so a rebase is needed for that anyway; - a 4-vs-6 measurement on the 5800X3D (first-start preparation time and pipeline-compile stutter in play), since this changes the default on every 16+ thread PC; - document `RX_VK_WORKERS` in `docs/development.md`, and the header comment on line 1 still says "2 to 4". On the sibling PRs (#2437, #2438): no code overlap, they touch other files and merge independently. They share the same gap though: none is measured alone on v2.1.0 and the test suite wasn't run on any of them. One interaction worth keeping in mind when you measure: #2437 starts up to 3 extra detached threads for its capture-buffer spares, beside these workers, at exactly the moments (scene loads) when pipelines compile. If you run the 4-vs-6 comparison, doing it without #2437 applied keeps the numbers attributable to this change. CI has only run the title check so far.
Commenting is not possible because the repository is archived.
No description provided.