perf(vk): 6 background workers on machines with 16 or more hardware threads #2440
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!2440
Loading…
Reference in a new issue
No description provided.
Delete branch "perf-workers-16-threads"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 change is small and doesn't break anything I could check.
tests/test_jobs_failure.py,tests/test_jobs_wait_runs_queued.pyandtests/test_ub_threads.pypass at this head. The Intel freeze path (#496) goes throughjobs_push_serialand 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 aperfchange yet, though:docs/workflow.mdasks for a measurement on any performance claim ("Measure, don't guess", and numbers fromhookshot_log.txtfor 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 todocs/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.gpu/vk/vk_jobs.cpp:77countsstd::thread::hardware_concurrency(). Main now hasusable_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.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 indocs/development.md. Also note that a value that isn't a number (atoi→ 0) quietly becomes 1 worker.vk_jobs.cpp:77countshardware_concurrency(). Main now hasusable_cpus()(vk_internal.h:317), so an affinity-limited process still gets 6 workers.RX_VK_WORKERSisn't documented, a non-numeric value silently becomes 1, and the header comment on line 1 is out of date.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:
usable_cpus()(on main since #2323, already reachable throughvk_internal.h) instead ofhardware_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;RX_VK_WORKERSindocs/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.
github-sync referenced this pull request2026-11-12 06:22:54 +00:00