feat(vk): cap compiler pools at physical cores #2414

Open
polaco1782 wants to merge 3 commits from polaco1782/fix/physical-core-shader-workers into main
polaco1782 commented 2026-11-11 18:02:10 +00:00 (Migrated from github.com)

Concurrent driver pipeline compiles can consume several GiB more memory when every SMT thread gets a worker. This proposes a physical-core upper bound to reduce that concurrent work, accepting longer preparation; the available measurements do not establish an OOM fix or a speedup.

Originally rebuilt from clean upstream main (3e95ca68), including merged #2323; now refreshed through upstream 463eab1f in merge commit 7689539d. This remains a compiler-pool policy proposal awaiting the maintainer's direction and additional hardware measurements.

Behavior

  • compile_worker_count(previous_budget) caps the existing pool budget by the detected physical cores. It preserves #2323's usable_cpus() / half_cpus() budgets and every smaller stage ceiling.
  • The main difference on an unrestricted SMT machine is predict_run and precompile_run: they previously used up to 64 usable logical CPUs. The half-CPU translation, state-list and library pools usually already equal or fall below the physical count. They can change under restricted affinity or an unusual topology. The shared background pool is capped too.
  • Linux counts cores within the calling thread's affinity mask, with dynamically sized masks and global sibling CPU IDs. A missing/unreadable CPU record counts separately when other records are readable; both current and older sysfs names are tried.
  • Windows counts RelationProcessorCore records across groups, including hybrid cores, and honors process affinity on single-group systems. Windows 10 multi-group process restrictions and cgroup CPU quotas remain limitations, as on main.
  • Unknown topology returns 0: the caller retains its previous budget, including the background minimum of two. vk: CPU topology not readable, using N workers is logged once. A genuine single physical core still gives one worker.
  • RX_VK_COMPILE_LOGICAL=1 bypasses the cap for comparison and keeps the existing affinity-aware budgets. This limits application worker counts; it does not pin workers or constrain threads inside the driver.
  • Cache readers are unchanged. Neither vk_shader.cpp nor vk_progcache.cpp/.h is modified relative to main. The translator stamp and cache format are unchanged by this PR, so it adds no full-game retranslation on top of main.

Historical measurements

These are the original pipeline-only budget experiment, not a benchmark of this revised branch or a complete cold first start. They predate #2323 and the newer quarantine/watchdog changes. Shader translation and gameplay were not timed. The old experiment also changed the cache-reader budget; this revision deliberately leaves cache readers unchanged.

Ryzen 7 5700X (8C/16T), 32 GB; RX 7800 XT; Fedora 44, Linux 7.1.13, RADV/Mesa 26.1.8. Host log: v2.1.0-rc.3-53-g49f75dc-dirty (source 52a1c006). Identical installed renderer SHA-256: 0fec25a6e7ac5a965bb38e6e02585b64f278abfd6ff4bb7ff1cd21c4db97a71b.

Four independent cold --prepare-shaders trials, each followed by a warm run, in 8/16/16/8 order. Cold trials had empty application pipeline and private Mesa caches, an already translated bank of 46,607 program keys and warmed filesystem inputs. Seed/state lists were omitted. Every trial selected 12,752 variants and built 16,037 graphics plus 1,218 compute pipelines. No game files, caches or translated programs are shared here.

A local LD_PRELOAD shim changed the renderer's get_nprocs result (its hardware_concurrency query) to 8 or 16, preserving 16 for the driver and other modules. SMT stayed enabled; affinity was unchanged. Wall time is monotonic; process CPU time and peak RSS use wait4. Background desktop load and CPU boost were uncontrolled.

Trial Workers Cold seconds Warm seconds Cold peak RSS, GiB
1 8 266.485 2.336 6.35
2 16 256.360 2.978 9.88
3 16 255.016 2.590 9.41
4 8 266.638 2.339 6.77

Cold means: 266.561 vs 255.688 s (+4.25% with eight workers). Mean peak RSS: 6.56 vs 9.65 GiB (32% lower). Mean CPU use: 793.5% vs 1483.0% (100% is one logical CPU). All eight processes succeeded with equal workload counts and no reported compiler fault or device loss. Original input hashes were unchanged. An incomplete full-translation pilot was excluded.

#2323 also measured NVIDIA/Linux prediction at 302 s on six workers vs 271 s on twelve, about 11% longer at the physical-core count. The resource/time tradeoff varies by driver. A physical-core count is not a memory budget: a 16-core PC with 16 GB can still run out of memory.

Current branch validation

  • Refreshed with upstream 463eab1f in 7689539deeb6b5e750a53ea519ab336de2a152ac. The only merge conflict was adjacent rows in docs/tested-configurations.md; both were retained. The compiler-pool feature diff remains unchanged: 12 files, +479/-17, with no cache-reader feature changes.
  • The Windows scanf fixture correction is inherited from upstream rather than added as a separate patch.
  • Cloud checks on this merged revision: 6 focused checks passed; aggregate 361 passed, 5 failed, 58 skipped. All five failures also reproduced in a fresh worktree at the same upstream commit: test_seed_reader.py, test_program_cache_reader.py, test_control.py, test_hookshot_ctl.py, and test_mem_gate_children.py. This is not a full local test pass. Skips include missing compiler/header/tool dependencies and platform/GPU/game requirements; no native Windows or game test was performed in this cloud run.
  • Started new fork CI for this exact head: Windows/Linux source regressions, Windows builder, and Linux AppImage. These runs have not yet been reported as passing.

Validation of the earlier revision

  • Full game-free suite, sequential: 389 passed, 1 pre-existing fixture failure, 21 skipped in 377 s on the initial clean base d8f4a173. The failure is test_make_game_cache.py: its blocked subprocess encounters the local PGO-profile compiler probe; reproduced against unmodified upstream, and nx2/make_game.py is identical on the final base.
  • After rebasing onto 3e95ca68 (the intervening VRAM-compaction fix): all eight focused topology, affinity, queue, driver-stall, cache-stamp, startup and VRAM checks passed. Queue regressions also passed with native affinity restricted to one CPU.
  • Final topology fixtures passed with GCC and Clang: Windows/Linux stand-ins, unknown topology, partial/missing records, SMT/hybrid/no-SMT, groups, sparse IDs, growing masks, fallback budgets, warning once and logical override. The native Linux detector reports eight cores on this 16-logical-CPU host.
  • Linux Vulkan renderer rebuild passed with one build worker: 68 s, zero compile failures, linked hookshot_vk.so. No new gameplay or current-branch performance run was performed.
  • Fork Windows validation passed for 44443d99: Windows Builder package/setup and the native llvm-mingw Vulkan back end compiled and linked. Release publication was skipped.
  • Clean 12-file source/test/docs diff; no translator-stamped/cache-source changes or game-derived artifacts.

Still needed before merge

  • A current-branch cold first-start comparison on a 16 GB machine, ideally one that OOMed on main, and on Windows with NVIDIA or AMD. No such hardware run has been performed here.
  • Maintainer direction on this default cap versus #2323's logical-CPU policy, #1610's explicit job limits, or memory-aware admission. No MemoryGate rewrite is included while that decision is pending. Related reports: #995 and #1593; this PR does not claim to resolve them.
  • Upstream CI approval and successful source-regression/Windows/Linux checks for the current merged head. The current fork runs are linked above; earlier build successes do not validate this merged revision or supply the missing Windows driver benchmark.

The tested-configurations table now has one historical summary row linking here. The separate measurement document and PR self-entry in the worklist have been removed from the proposed diff.

Concurrent driver pipeline compiles can consume several GiB more memory when every SMT thread gets a worker. This proposes a physical-core upper bound to reduce that concurrent work, accepting longer preparation; the available measurements do not establish an OOM fix or a speedup. Originally rebuilt from clean upstream `main` (`3e95ca68`), including merged #2323; now refreshed through upstream `463eab1f` in merge commit `7689539d`. This remains a compiler-pool policy proposal awaiting the maintainer's direction and additional hardware measurements. ### Behavior - `compile_worker_count(previous_budget)` caps the **existing** pool budget by the detected physical cores. It preserves #2323's `usable_cpus()` / `half_cpus()` budgets and every smaller stage ceiling. - The main difference on an unrestricted SMT machine is `predict_run` and `precompile_run`: they previously used up to 64 usable logical CPUs. The half-CPU translation, state-list and library pools usually already equal or fall below the physical count. They can change under restricted affinity or an unusual topology. The shared background pool is capped too. - Linux counts cores within the calling thread's affinity mask, with dynamically sized masks and global sibling CPU IDs. A missing/unreadable CPU record counts separately when other records are readable; both current and older sysfs names are tried. - Windows counts `RelationProcessorCore` records across groups, including hybrid cores, and honors process affinity on single-group systems. Windows 10 multi-group process restrictions and cgroup CPU quotas remain limitations, as on main. - **Unknown topology returns 0:** the caller retains its previous budget, including the background minimum of two. `vk: CPU topology not readable, using N workers` is logged once. A genuine single physical core still gives one worker. - `RX_VK_COMPILE_LOGICAL=1` bypasses the cap for comparison and keeps the existing affinity-aware budgets. This limits application worker counts; it does not pin workers or constrain threads inside the driver. - **Cache readers are unchanged.** Neither `vk_shader.cpp` nor `vk_progcache.cpp/.h` is modified relative to main. The translator stamp and cache format are unchanged by this PR, so it adds no full-game retranslation on top of main. ### Historical measurements These are the original **pipeline-only budget experiment**, not a benchmark of this revised branch or a complete cold first start. They predate #2323 and the newer quarantine/watchdog changes. Shader translation and gameplay were not timed. The old experiment also changed the cache-reader budget; this revision deliberately leaves cache readers unchanged. Ryzen 7 5700X (8C/16T), 32 GB; RX 7800 XT; Fedora 44, Linux 7.1.13, RADV/Mesa 26.1.8. Host log: `v2.1.0-rc.3-53-g49f75dc-dirty (source 52a1c006)`. Identical installed renderer SHA-256: `0fec25a6e7ac5a965bb38e6e02585b64f278abfd6ff4bb7ff1cd21c4db97a71b`. Four independent cold `--prepare-shaders` trials, each followed by a warm run, in 8/16/16/8 order. Cold trials had empty application pipeline and private Mesa caches, an already translated bank of 46,607 program keys and warmed filesystem inputs. Seed/state lists were omitted. Every trial selected 12,752 variants and built 16,037 graphics plus 1,218 compute pipelines. No game files, caches or translated programs are shared here. A local `LD_PRELOAD` shim changed the renderer's `get_nprocs` result (its `hardware_concurrency` query) to 8 or 16, preserving 16 for the driver and other modules. SMT stayed enabled; affinity was unchanged. Wall time is monotonic; process CPU time and peak RSS use `wait4`. Background desktop load and CPU boost were uncontrolled. | Trial | Workers | Cold seconds | Warm seconds | Cold peak RSS, GiB | |---|---:|---:|---:|---:| | 1 | 8 | 266.485 | 2.336 | 6.35 | | 2 | 16 | 256.360 | 2.978 | 9.88 | | 3 | 16 | 255.016 | 2.590 | 9.41 | | 4 | 8 | 266.638 | 2.339 | 6.77 | Cold means: **266.561 vs 255.688 s (+4.25% with eight workers)**. Mean peak RSS: **6.56 vs 9.65 GiB (32% lower)**. Mean CPU use: 793.5% vs 1483.0% (100% is one logical CPU). All eight processes succeeded with equal workload counts and no reported compiler fault or device loss. Original input hashes were unchanged. An incomplete full-translation pilot was excluded. #2323 also measured NVIDIA/Linux prediction at **302 s on six workers vs 271 s on twelve**, about 11% longer at the physical-core count. The resource/time tradeoff varies by driver. A physical-core count is not a memory budget: a 16-core PC with 16 GB can still run out of memory. ### Current branch validation - Refreshed with upstream `463eab1f` in `7689539deeb6b5e750a53ea519ab336de2a152ac`. The only merge conflict was adjacent rows in `docs/tested-configurations.md`; both were retained. The compiler-pool feature diff remains unchanged: 12 files, +479/-17, with no cache-reader feature changes. - The Windows `scanf` fixture correction is inherited from upstream rather than added as a separate patch. - Cloud checks on this merged revision: **6 focused checks passed**; aggregate **361 passed, 5 failed, 58 skipped**. All five failures also reproduced in a fresh worktree at the same upstream commit: `test_seed_reader.py`, `test_program_cache_reader.py`, `test_control.py`, `test_hookshot_ctl.py`, and `test_mem_gate_children.py`. This is **not a full local test pass**. Skips include missing compiler/header/tool dependencies and platform/GPU/game requirements; no native Windows or game test was performed in this cloud run. - Started new fork CI for this exact head: [Windows/Linux source regressions](https://github.com/polaco1782/Hookshot/actions/runs/43100959869), [Windows builder](https://github.com/polaco1782/Hookshot/actions/runs/43100969637), and [Linux AppImage](https://github.com/polaco1782/Hookshot/actions/runs/43100979068). These runs have not yet been reported as passing. ### Validation of the earlier revision - Full game-free suite, sequential: **389 passed, 1 pre-existing fixture failure, 21 skipped** in 377 s on the initial clean base `d8f4a173`. The failure is `test_make_game_cache.py`: its blocked subprocess encounters the local PGO-profile compiler probe; reproduced against unmodified upstream, and `nx2/make_game.py` is identical on the final base. - After rebasing onto `3e95ca68` (the intervening VRAM-compaction fix): all eight focused topology, affinity, queue, driver-stall, cache-stamp, startup and VRAM checks passed. Queue regressions also passed with native affinity restricted to one CPU. - Final topology fixtures passed with GCC and Clang: Windows/Linux stand-ins, unknown topology, partial/missing records, SMT/hybrid/no-SMT, groups, sparse IDs, growing masks, fallback budgets, warning once and logical override. The native Linux detector reports eight cores on this 16-logical-CPU host. - Linux Vulkan renderer rebuild passed with one build worker: 68 s, zero compile failures, linked `hookshot_vk.so`. No new gameplay or current-branch performance run was performed. - [Fork Windows validation](https://github.com/polaco1782/Hookshot/actions/runs/43086470564) passed for `44443d99`: Windows Builder package/setup and the native llvm-mingw Vulkan back end compiled and linked. Release publication was skipped. - Clean 12-file source/test/docs diff; no translator-stamped/cache-source changes or game-derived artifacts. ### Still needed before merge - A current-branch cold first-start comparison on a 16 GB machine, ideally one that OOMed on main, and on Windows with NVIDIA or AMD. No such hardware run has been performed here. - Maintainer direction on this default cap versus #2323's logical-CPU policy, #1610's explicit job limits, or memory-aware admission. No MemoryGate rewrite is included while that decision is pending. Related reports: #995 and #1593; this PR does not claim to resolve them. - Upstream CI approval and successful source-regression/Windows/Linux checks for the current merged head. The current fork runs are linked above; earlier build successes do not validate this merged revision or supply the missing Windows driver benchmark. The tested-configurations table now has one historical summary row linking here. The separate measurement document and PR self-entry in the worklist have been removed from the proposed diff.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 18:25:50 +00:00
hckrman101 (Migrated from github.com) left a comment

Thanks for the careful measurements, and for keeping an A/B switch. The detector itself reads right to me: on Windows it counts RelationProcessorCore records across groups (so hybrid E-cores count once each and >64-LP machines aren't truncated), applies the affinity mask on single-group systems, and the record walk is bounds-checked; on Linux sched_getaffinity covers taskset/cpusets/offline CPUs and the first CPU in core_cpus_list is a global id, so multi-package boxes are fine. A CPU quota (docker --cpus, cgroup cpu.max) isn't seen, but main doesn't see it either, so that's not a regression.

What I'd like settled before this goes in:

  1. The failure fallback is a cliff. Any detection failure gives one worker for everything: predict and precompile then build ~17,000 pipelines on a single thread (on your 5700X that's roughly 8x the 266 s), and the in-play job pool drops below its old minimum of 2. On Linux one allowed CPU without a readable topology file is enough (sandboxes that mask /sys, some container runtimes). Nothing logs that detection failed either: the worker-count lines just say 1. I'd fall back to the old budget (hardware_concurrency(), or half of it) and log vk: CPU topology not readable, using N workers once.

  2. What this actually changes, and what it costs. On SMT CPUs state_list_prepare, warm_libraries and prepare_run already ran cpus / 2, which is the physical count, and on non-SMT CPUs cpus / 2 is still below the core count. So in practice the behaviour change is predict_run + precompile_run (plus the cache reader and the job pool at the edges). It'd help to say that in the description. On those two, both measurements we have say the cap makes first start slower: your RADV run is +4.25%, and #2323's NVIDIA/Linux run (12 usable LPs, 6 cores) measured the prediction at 302 s on 6 threads vs 271 s on 12, about +11%, which is why that draft goes the other way (usable_cpus(), one thread per usable logical CPU). The win here is peak RSS (9.65 -> 6.56 GiB), which matters for the OOM reports in #995/#1593, but a core count is only a proxy for memory: a 16C/32T PC with 16 GB still gets 16 workers, while a 32 GB PC that was fine pays the extra time. Could you get numbers for (a) a 16 GB machine, ideally one that OOMed on main, to show this fixes it there, and (b) Windows with NVIDIA or AMD, cold first start? If memory is the target, something like nx2/build.py's MemoryGate (start a compile only when MemAvailable minus the running ones' expected growth leaves room) might be the more direct fix; the core cap could then stay as an upper bound.

  3. The vk_shader.cpp edit makes every player translate the whole game again. nx2/build.py hashes all of vk_shader.cpp into HOOKSHOT_SHADER_STAMP, so touching program_cache_load_some invalidates everyone's hookshot_programs.bin on update. That reader is a cache load, not a compile; I'd leave it at min(cpus, 16) here (or wait for #2323, which moves it out of the hashed file).

  4. Overlap. #1610 (open, my earlier approval was dismissed) caps precompile_run with RX_VK_PRECOMPILE_JOBS/HOOKSHOT_JOBS, and the draft #2323 rewrites the same pool sizes in predict_run/precompile_run/the cache reader around the affinity mask. These three will conflict and pick different policies, so I'd like @NotRexed to choose the direction before more work goes into any of them. #2263 uses a logical-processor count for an unrelated threshold (the game's short sleeps); no conflict.

  5. CI hasn't run. Windows builder, Linux AppImage and the game-free regressions are all waiting on approval (action_required), so the new test_compile_threads.py hasn't been built with llvm-mingw yet. The branch is also 7 commits behind main.

Nits:

  • docs/tested-configurations.md: eight rows for one benchmark is a lot for a table of in-game reports. One summary row linking the measurement doc would do. docs/compile-worker-measurements.md points at local, ignored paths (build-linux/compile-benchmark/, work/...) that readers can't open, and its numbers predate the rebase. The PR body may be a better place for it.
  • docs/worklist.md: the new row describes this PR's own change (and lists Codex as the claimant). The worklist is open work, so it would need removing on merge; I'd just drop it.
  • Windows 10 confines a process to one group unless it asks for more, so on a >64-LP box the cross-group core count can exceed what the process can run on. Main's min(cpus, 64) had the same issue, so not new.

Not good to merge yet: points 1 and 3 are concrete, 2 and 4 need numbers and a decision, and CI hasn't run.

Thanks for the careful measurements, and for keeping an A/B switch. The detector itself reads right to me: on Windows it counts `RelationProcessorCore` records across groups (so hybrid E-cores count once each and >64-LP machines aren't truncated), applies the affinity mask on single-group systems, and the record walk is bounds-checked; on Linux `sched_getaffinity` covers taskset/cpusets/offline CPUs and the first CPU in `core_cpus_list` is a global id, so multi-package boxes are fine. A CPU *quota* (`docker --cpus`, cgroup `cpu.max`) isn't seen, but main doesn't see it either, so that's not a regression. What I'd like settled before this goes in: 1. **The failure fallback is a cliff.** Any detection failure gives one worker for everything: predict and precompile then build ~17,000 pipelines on a single thread (on your 5700X that's roughly 8x the 266 s), and the in-play job pool drops below its old minimum of 2. On Linux one allowed CPU without a readable topology file is enough (sandboxes that mask `/sys`, some container runtimes). Nothing logs that detection failed either: the worker-count lines just say 1. I'd fall back to the old budget (`hardware_concurrency()`, or half of it) and log `vk: CPU topology not readable, using N workers` once. 2. **What this actually changes, and what it costs.** On SMT CPUs `state_list_prepare`, `warm_libraries` and `prepare_run` already ran `cpus / 2`, which is the physical count, and on non-SMT CPUs `cpus / 2` is still below the core count. So in practice the behaviour change is `predict_run` + `precompile_run` (plus the cache reader and the job pool at the edges). It'd help to say that in the description. On those two, both measurements we have say the cap makes first start slower: your RADV run is +4.25%, and #2323's NVIDIA/Linux run (12 usable LPs, 6 cores) measured the prediction at 302 s on 6 threads vs 271 s on 12, about +11%, which is why that draft goes the other way (`usable_cpus()`, one thread per usable logical CPU). The win here is peak RSS (9.65 -> 6.56 GiB), which matters for the OOM reports in #995/#1593, but a core count is only a proxy for memory: a 16C/32T PC with 16 GB still gets 16 workers, while a 32 GB PC that was fine pays the extra time. Could you get numbers for (a) a 16 GB machine, ideally one that OOMed on main, to show this fixes it there, and (b) Windows with NVIDIA or AMD, cold first start? If memory is the target, something like `nx2/build.py`'s `MemoryGate` (start a compile only when `MemAvailable` minus the running ones' expected growth leaves room) might be the more direct fix; the core cap could then stay as an upper bound. 3. **The `vk_shader.cpp` edit makes every player translate the whole game again.** `nx2/build.py` hashes all of `vk_shader.cpp` into `HOOKSHOT_SHADER_STAMP`, so touching `program_cache_load_some` invalidates everyone's `hookshot_programs.bin` on update. That reader is a cache load, not a compile; I'd leave it at `min(cpus, 16)` here (or wait for #2323, which moves it out of the hashed file). 4. **Overlap.** #1610 (open, my earlier approval was dismissed) caps `precompile_run` with `RX_VK_PRECOMPILE_JOBS`/`HOOKSHOT_JOBS`, and the draft #2323 rewrites the same pool sizes in `predict_run`/`precompile_run`/the cache reader around the affinity mask. These three will conflict and pick different policies, so I'd like @NotRexed to choose the direction before more work goes into any of them. #2263 uses a logical-processor count for an unrelated threshold (the game's short sleeps); no conflict. 5. **CI hasn't run.** Windows builder, Linux AppImage and the game-free regressions are all waiting on approval (`action_required`), so the new `test_compile_threads.py` hasn't been built with llvm-mingw yet. The branch is also 7 commits behind main. Nits: - `docs/tested-configurations.md`: eight rows for one benchmark is a lot for a table of in-game reports. One summary row linking the measurement doc would do. `docs/compile-worker-measurements.md` points at local, ignored paths (`build-linux/compile-benchmark/`, `work/...`) that readers can't open, and its numbers predate the rebase. The PR body may be a better place for it. - `docs/worklist.md`: the new row describes this PR's own change (and lists `Codex` as the claimant). The worklist is open work, so it would need removing on merge; I'd just drop it. - Windows 10 confines a process to one group unless it asks for more, so on a >64-LP box the cross-group core count can exceed what the process can run on. Main's `min(cpus, 64)` had the same issue, so not new. Not good to merge yet: points 1 and 3 are concrete, 2 and 4 need numbers and a decision, and CI hasn't run.
@ -0,0 +1,115 @@
// Shader/pipeline pools count physical cores, not SMT siblings. Keep each pool's existing smaller limit.
hckrman101 (Migrated from github.com) commented 2026-11-11 18:25:50 +00:00

With one worker the predict/precompile pools run ~17k pipeline builds on a single thread, and the job pool goes below its old minimum of 2. I'd return 0 for "unknown" here (and from the return 1 paths above), and have compile_worker_count fall back to the previous budget and log once that topology wasn't readable.

With one worker the predict/precompile pools run ~17k pipeline builds on a single thread, and the job pool goes below its old minimum of 2. I'd return 0 for "unknown" here (and from the `return 1` paths above), and have `compile_worker_count` fall back to the previous budget and log once that topology wasn't readable.
hckrman101 (Migrated from github.com) commented 2026-11-11 18:25:50 +00:00

One allowed CPU without a readable topology file (a sandbox or container that masks part of /sys) fails detection for the whole machine. Skipping that CPU, or counting it as its own core, would degrade gracefully.

One allowed CPU without a readable topology file (a sandbox or container that masks part of `/sys`) fails detection for the whole machine. Skipping that CPU, or counting it as its own core, would degrade gracefully.
hckrman101 (Migrated from github.com) commented 2026-11-11 18:25:50 +00:00

This and precompile_run are where the behaviour really changes (the cpus / 2 pools already equal the physical count on SMT CPUs). #2323 measured this prediction at 302 s on 6 threads vs 271 s on 12 (NVIDIA, Linux), so the cap costs first-start time there as well as on your RADV run.

This and `precompile_run` are where the behaviour really changes (the `cpus / 2` pools already equal the physical count on SMT CPUs). #2323 measured this prediction at 302 s on 6 threads vs 271 s on 12 (NVIDIA, Linux), so the cap costs first-start time there as well as on your RADV run.
hckrman101 (Migrated from github.com) commented 2026-11-11 18:25:50 +00:00

Any edit to vk_shader.cpp changes HOOKSHOT_SHADER_STAMP (nx2/build.py hashes the whole file), so every player translates all programs again after updating. This is a cache reader, not a compile pool; I'd leave it as it was in this PR.

Any edit to `vk_shader.cpp` changes `HOOKSHOT_SHADER_STAMP` (`nx2/build.py` hashes the whole file), so every player translates all programs again after updating. This is a cache reader, not a compile pool; I'd leave it as it was in this PR.
polaco1782 (Migrated from github.com) reviewed 2026-11-11 19:21:27 +00:00
@ -0,0 +1,115 @@
// Shader/pipeline pools count physical cores, not SMT siblings. Keep each pool's existing smaller limit.
polaco1782 (Migrated from github.com) commented 2026-11-11 19:21:27 +00:00

Addressed in github.com/NotRexed/Hookshot@44443d9954. Every unknown/failure path now returns 0. compile_worker_count(previous_budget) keeps that exact budget when detection is unavailable (including the background minimum of two) and logs vk: CPU topology not readable, using N workers once. Windows/Linux/unsupported-platform fixtures check the fallback across budgets 64, 2 and 4 and the once-only warning. A genuine single physical core still caps at one.

Addressed in https://github.com/NotRexed/Hookshot/commit/44443d995475d2271c8bc2ab429a6744d29a7866. Every unknown/failure path now returns 0. `compile_worker_count(previous_budget)` keeps that exact budget when detection is unavailable (including the background minimum of two) and logs `vk: CPU topology not readable, using N workers` once. Windows/Linux/unsupported-platform fixtures check the fallback across budgets 64, 2 and 4 and the once-only warning. A genuine single physical core still caps at one.
polaco1782 (Migrated from github.com) reviewed 2026-11-11 19:21:29 +00:00
@ -0,0 +1,115 @@
// Shader/pipeline pools count physical cores, not SMT siblings. Keep each pool's existing smaller limit.
polaco1782 (Migrated from github.com) commented 2026-11-11 19:21:29 +00:00

Addressed in github.com/NotRexed/Hookshot@44443d9954. Each allowed Linux CPU tries both sysfs names, including when the current file exists but is unreadable. An unreadable record counts separately when other records are readable; it no longer discards the rest of the topology. If no record can be read, detection returns unknown and preserves the previous pool budget. The fixtures cover partial missing and malformed records as well as wholly missing topology.

Addressed in https://github.com/NotRexed/Hookshot/commit/44443d995475d2271c8bc2ab429a6744d29a7866. Each allowed Linux CPU tries both sysfs names, including when the current file exists but is unreadable. An unreadable record counts separately when other records are readable; it no longer discards the rest of the topology. If no record can be read, detection returns unknown and preserves the previous pool budget. The fixtures cover partial missing and malformed records as well as wholly missing topology.
polaco1782 (Migrated from github.com) reviewed 2026-11-11 19:21:31 +00:00
polaco1782 (Migrated from github.com) commented 2026-11-11 19:21:30 +00:00

Addressed in github.com/NotRexed/Hookshot@44443d9954, rebuilt from main after #2323 merged. This PR now changes neither vk_shader.cpp nor vk_progcache.cpp/.h; both cache readers retain their existing usable-logical-CPU budgets. The translator stamp and cache format are untouched relative to main, and the program-cache stamp regression passes. There is no additional whole-game retranslation from this change.

Addressed in https://github.com/NotRexed/Hookshot/commit/44443d995475d2271c8bc2ab429a6744d29a7866, rebuilt from main after #2323 merged. This PR now changes neither `vk_shader.cpp` nor `vk_progcache.cpp/.h`; both cache readers retain their existing usable-logical-CPU budgets. The translator stamp and cache format are untouched relative to main, and the program-cache stamp regression passes. There is no additional whole-game retranslation from this change.
polaco1782 (Migrated from github.com) reviewed 2026-11-11 19:21:33 +00:00
polaco1782 (Migrated from github.com) commented 2026-11-11 19:21:33 +00:00

Agreed. The description now identifies prediction/precompile as the main behavior change on an unrestricted SMT host and explicitly reports both the +4.25% historical RADV cost and the roughly +11% NVIDIA/Linux prediction cost from #2323. The old RADV experiment is labeled historical, includes its cache-reader-policy difference, and is not presented as a benchmark of this revision. Cache readers now remain unchanged. I am keeping this draft pending the maintainer policy decision and the requested Windows/16 GB measurements; the core cap alone is not evidence of an OOM fix.

Agreed. The description now identifies prediction/precompile as the main behavior change on an unrestricted SMT host and explicitly reports both the +4.25% historical RADV cost and the roughly +11% NVIDIA/Linux prediction cost from #2323. The old RADV experiment is labeled historical, includes its cache-reader-policy difference, and is not presented as a benchmark of this revision. Cache readers now remain unchanged. I am keeping this draft pending the maintainer policy decision and the requested Windows/16 GB measurements; the core cap alone is not evidence of an OOM fix.
polaco1782 commented 2026-11-11 19:23:11 +00:00 (Migrated from github.com)

Rebuilt the proposal as one clean commit from current main 3e95ca68: 44443d9954. Replies are on all four inline threads.

Concrete fixes: unknown topology preserves each previous budget and warns once; partial Linux topology degrades per CPU; cache readers and translator-stamped sources are unchanged. The merged #2323 affinity-aware budgets remain underneath the physical cap. The benchmark table is one row, the historical measurements are in the description with their limitations, and the worklist self-entry is gone.

Local validation: Linux renderer rebuilt (one worker, 68 s, zero compile failures). The full game-free suite on the initial clean base passed 389 checks, with 21 skips and the existing local-PGO test_make_game_cache.py fixture failure reproduced on upstream. After the one intervening upstream VRAM commit, all eight focused checks passed; queue tests also passed restricted to one native CPU. Final topology fixtures passed GCC and Clang.

The unchanged workflows are running on this exact head in the fork, since upstream approval is still needed. The Windows build has now passed (both package/setup and native llvm-mingw Vulkan compilation/linking); no release was published:

This remains draft. There is no current-branch Windows cold-first-start or 16 GB/OOM-reporter measurement, and the existing numbers show a time/memory tradeoff rather than a speedup or proven OOM fix. @NotRexed, with #2323 now merged and #1610 still open, please choose whether to pursue this default cap or prefer explicit job limits / memory-aware admission before extending the policy. No MemoryGate rewrite is included.

Rebuilt the proposal as one clean commit from current main `3e95ca68`: 44443d995475d2271c8bc2ab429a6744d29a7866. Replies are on all four inline threads. Concrete fixes: unknown topology preserves each previous budget and warns once; partial Linux topology degrades per CPU; cache readers and translator-stamped sources are unchanged. The merged #2323 affinity-aware budgets remain underneath the physical cap. The benchmark table is one row, the historical measurements are in the description with their limitations, and the worklist self-entry is gone. Local validation: Linux renderer rebuilt (one worker, 68 s, zero compile failures). The full game-free suite on the initial clean base passed 389 checks, with 21 skips and the existing local-PGO `test_make_game_cache.py` fixture failure reproduced on upstream. After the one intervening upstream VRAM commit, all eight focused checks passed; queue tests also passed restricted to one native CPU. Final topology fixtures passed GCC and Clang. The unchanged workflows are running on this exact head in the fork, since upstream approval is still needed. The Windows build has now passed (both package/setup and native llvm-mingw Vulkan compilation/linking); no release was published: - [Game-free Windows/Linux source checks](https://github.com/polaco1782/Hookshot/actions/runs/43086464998) - **Passed:** [Windows builder and llvm-mingw Vulkan build](https://github.com/polaco1782/Hookshot/actions/runs/43086470564) - [Linux AppImage](https://github.com/polaco1782/Hookshot/actions/runs/43086475659) This remains **draft**. There is no current-branch Windows cold-first-start or 16 GB/OOM-reporter measurement, and the existing numbers show a time/memory tradeoff rather than a speedup or proven OOM fix. @NotRexed, with #2323 now merged and #1610 still open, please choose whether to pursue this default cap or prefer explicit job limits / memory-aware admission before extending the policy. No MemoryGate rewrite is included.
github-sync changed title from WIP: feat(vk): cap compiler pools at physical cores to feat(vk): cap compiler pools at physical cores 2026-11-12 06:23:07 +00:00
Commenting is not possible because the repository is archived.
No description provided.