MagicBeanDev/Hookshot
Archived
Watch
6
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

fix(vk): bound precompile thread concurrency via half_cpus (#1593) #1610

Open
oscarlosu-lab wants to merge 1 commit from oscarlosu-lab/fix/precompile-concurrency into main
pull from: oscarlosu-lab/fix/precompile-concurrency
merge into: MagicBeanDev:main
Conversation 14 Commits 1 Files changed 3 +9 -4
oscarlosu-lab commented 2026-11-10 10:58:39 +00:00 (Migrated from github.com)

This PR bounds worker thread concurrency in precompile_run() using #2323's affinity-aware half_cpus() helper and honors RX_VK_PRECOMPILE_JOBS (#1593). Without a cap, systems with high thread counts (especially on Linux using Mesa RADV / ACO) can spawn dozens of concurrent pipeline compiles simultaneously and exhaust system RAM.

Why

In gpu/vk/vk_draw.cpp (precompile_run()), the thread pool previously spawned std::min(usable_cpus(), 64u) compiler threads. On high-core-count systems, compiling many Vulkan graphics/compute pipelines concurrently can build excessive per-thread resident memory and trigger the Linux OOM-killer (#1593).

Changes

  • Affinity-aware default cap: Uses std::clamp(half_cpus(), 1u, 16u), building on #2323's half_cpus() budget (already honoring process affinity via usable_cpus()) while keeping the compiler cap deliberately lower than predict_run()/state_list_run()'s 32 cap.
  • Explicit runtime override: Honors RX_VK_PRECOMPILE_JOBS (capped at 64u), leaving HOOKSHOT_JOBS strictly for build-time concurrency so global build settings do not throttle runtime startup.
  • Test update: Updated tests/test_prepare_cpus.py so precompile_run is checked for half_cpus().

Scope

Related to #1593 (closed, tracked in #1072). This bounds how many pipeline compiles run side by side, which is what #1593's RADV report showed. It does not fix the Intel ANV out-of-memory that mooki000 measured on a UHD 770: there a few ~250,000-word compute programs compiled together exhaust RAM even with 4 workers. That needs the large compiles serialized on their own, which is a separate follow-up.

#2414 caps the same pools by physical cores and keeps every smaller stage ceiling, so its cap would sit above this one: the two compose rather than conflict.

This PR bounds worker thread concurrency in `precompile_run()` using #2323's affinity-aware `half_cpus()` helper and honors `RX_VK_PRECOMPILE_JOBS` (#1593). Without a cap, systems with high thread counts (especially on Linux using Mesa RADV / ACO) can spawn dozens of concurrent pipeline compiles simultaneously and exhaust system RAM. ### Why In `gpu/vk/vk_draw.cpp` (`precompile_run()`), the thread pool previously spawned `std::min(usable_cpus(), 64u)` compiler threads. On high-core-count systems, compiling many Vulkan graphics/compute pipelines concurrently can build excessive per-thread resident memory and trigger the Linux OOM-killer (#1593). ### Changes - **Affinity-aware default cap**: Uses `std::clamp(half_cpus(), 1u, 16u)`, building on #2323's `half_cpus()` budget (already honoring process affinity via `usable_cpus()`) while keeping the compiler cap deliberately lower than `predict_run()`/`state_list_run()`'s 32 cap. - **Explicit runtime override**: Honors `RX_VK_PRECOMPILE_JOBS` (capped at `64u`), leaving `HOOKSHOT_JOBS` strictly for build-time concurrency so global build settings do not throttle runtime startup. - **Test update**: Updated `tests/test_prepare_cpus.py` so `precompile_run` is checked for `half_cpus()`. ### Scope Related to #1593 (closed, tracked in #1072). This bounds how many pipeline compiles run side by side, which is what #1593's RADV report showed. It does not fix the Intel ANV out-of-memory that mooki000 measured on a UHD 770: there a few ~250,000-word compute programs compiled together exhaust RAM even with 4 workers. That needs the large compiles serialized on their own, which is a separate follow-up. #2414 caps the same pools by physical cores and keeps every smaller stage ceiling, so its cap would sit above this one: the two compose rather than conflict.
hckrman101 (Migrated from github.com) reviewed 2026-11-10 16:39:35 +00:00
hckrman101 (Migrated from github.com) left a comment
No description provided.
Looks good. The change is small and correct: cpus/2 clamped to 1..16 by default, with an override env var, and the existing 64 cap still applies. CI is green. Nits, none blocking: - The description says this matches predict_run()/state_list_run(), but those use clamp(cpus / 2, 1, 32), not 16. Fine as a deliberately lower cap for the heavier compile pass, but the wording could say so. - HOOKSHOT_JOBS is the build-time jobs variable (builder / make_game.py), so it will now also throttle runtime precompile for anyone who exports it globally. Probably what you want here, but worth a line in the docs next to RX_VK_PRECOMPILE_JOBS. - atoi on a garbage value falls back to the default, which is fine.
hckrman101 commented 2026-11-10 16:59:07 +00:00 (Migrated from github.com)

Actually maybe we should do CPU*0.75?

Actually maybe we should do CPU*0.75?
hckrman101 commented 2026-11-10 16:59:31 +00:00 (Migrated from github.com)

Maybe a slider in builder too like RAM would be good

Maybe a slider in builder too like RAM would be good
oscarlosu-lab commented 2026-11-10 17:10:17 +00:00 (Migrated from github.com)

Personally, I prefer that certain things be hardcoded and that the end user doesn't have so many controls that could generate instability, something more typical of a product that goes into production, I think the best approach would be to make a game that is compatible with all systems without each user having to adjust it.

Personally, I prefer that certain things be hardcoded and that the end user doesn't have so many controls that could generate instability, something more typical of a product that goes into production, I think the best approach would be to make a game that is compatible with all systems without each user having to adjust it.
mooki000 commented 2026-11-11 13:36:38 +00:00 (Migrated from github.com)

Tested on an Intel UHD 770 (Raptor Lake-S, no mesh shaders), Mesa 26.2.5 ANV, Arch Linux, i9-13900K (32 threads). I merged this into main 872cb68 and ran it headless with the game capped at 32 GB. On this machine it doesn't get the start-up precompile through.

Same save-load scene and the same caches for both runs, start-up precompile on:

build precompile workers result
main 872cb68 32 killed by the OOM killer at 32 GB after 124 s (start-up: 702 of 17346)
this PR (7ffe45b0) 16 killed by the OOM killer at 32 GB after 199 s (start-up: 675 of 17346)

With this PR the game's memory sat at 9.6-15 GB for two minutes. Then it went to 20 GB and 30 GB in the last 30 s.

The worker count isn't what decides it here. A few compute programs are about 250,000 SPIR-V words each (53caac5f38279735, f55579c6c7b39d05, a54f19e927d8faef, dad2a8fbef3fd020), and ANV takes minutes and several GB to compile each of them. They come one after another in the list, so any pool of four or more workers compiles them at the same time. I ran the build's shader preparation with only 4 workers. Memory stayed at 3-4 GB for 16 minutes. Then those four started compiling, and it went 3.5 → 10 → 25 → 32 GB in about ten minutes and was killed. With 16 workers (the default on this CPU) the same preparation was killed after 5 minutes.

So this cap may well be right for RADV (#1593). On Intel it only changes how soon the OOM kill happens. What would help there is keeping the very large compiles from running at the same time, or leaving them out of the precompile. I haven't tested this on RADV.

Tested on an Intel UHD 770 (Raptor Lake-S, no mesh shaders), Mesa 26.2.5 ANV, Arch Linux, i9-13900K (32 threads). I merged this into main 872cb68 and ran it headless with the game capped at 32 GB. On this machine it doesn't get the start-up precompile through. Same save-load scene and the same caches for both runs, start-up precompile on: | build | precompile workers | result | |---|---|---| | main 872cb68 | 32 | killed by the OOM killer at 32 GB after 124 s (`start-up: 702 of 17346`) | | this PR (7ffe45b0) | 16 | killed by the OOM killer at 32 GB after 199 s (`start-up: 675 of 17346`) | With this PR the game's memory sat at 9.6-15 GB for two minutes. Then it went to 20 GB and 30 GB in the last 30 s. The worker count isn't what decides it here. A few compute programs are about 250,000 SPIR-V words each (`53caac5f38279735`, `f55579c6c7b39d05`, `a54f19e927d8faef`, `dad2a8fbef3fd020`), and ANV takes minutes and several GB to compile each of them. They come one after another in the list, so any pool of four or more workers compiles them at the same time. I ran the build's shader preparation with only 4 workers. Memory stayed at 3-4 GB for 16 minutes. Then those four started compiling, and it went 3.5 → 10 → 25 → 32 GB in about ten minutes and was killed. With 16 workers (the default on this CPU) the same preparation was killed after 5 minutes. So this cap may well be right for RADV (#1593). On Intel it only changes how soon the OOM kill happens. What would help there is keeping the very large compiles from running at the same time, or leaving them out of the precompile. I haven't tested this on RADV.
oscarlosu-lab commented 2026-11-11 16:15:47 +00:00 (Migrated from github.com)

Thanks a lot for testing this thoroughly on Intel ANV and sharing the exact numbers and shader keys (53caac5f38279735, f55579c6c7b39d05, a54f19e927d8faef, dad2a8fbef3fd020), @mooki000 — that pinpoints a very clear second bottleneck.

Your 4-worker preparation run confirms that thread-count clamping alone cannot prevent ANV OOM on oversized compute shaders (~250k SPIR-V words): because they appear back-to-back in computes, any pool with >= 4 workers still compiles multiple ~250k-word pipelines concurrently.

I'd like to keep this PR (#1610) focused on bounding default worker concurrency (16 cap + RX_VK_PRECOMPILE_JOBS / HOOKSHOT_JOBS) for #1593 / RADV, and address the oversized SPIR-V concurrency issue cleanly in a dedicated follow-up PR — e.g., recording shader SPIR-V word count and serializing pipelines above a large-shader threshold (via a dedicated lock across both precompile_run() and shader preparation) so at most one heavyweight compile runs at a time regardless of worker count.

Thanks a lot for testing this thoroughly on Intel ANV and sharing the exact numbers and shader keys (`53caac5f38279735`, `f55579c6c7b39d05`, `a54f19e927d8faef`, `dad2a8fbef3fd020`), @mooki000 — that pinpoints a very clear second bottleneck. Your 4-worker preparation run confirms that thread-count clamping alone cannot prevent ANV OOM on oversized compute shaders (~250k SPIR-V words): because they appear back-to-back in `computes`, any pool with `>= 4` workers still compiles multiple ~250k-word pipelines concurrently. I'd like to keep this PR (`#1610`) focused on bounding default worker concurrency (`16` cap + `RX_VK_PRECOMPILE_JOBS` / `HOOKSHOT_JOBS`) for `#1593` / RADV, and address the oversized SPIR-V concurrency issue cleanly in a dedicated follow-up PR — e.g., recording shader SPIR-V word count and serializing pipelines above a large-shader threshold (via a dedicated lock across both `precompile_run()` and shader preparation) so at most one heavyweight compile runs at a time regardless of worker count.
mooki000 commented 2026-11-11 16:35:10 +00:00 (Migrated from github.com)

Sounds good to me: #1610 for the worker cap, and the very large compiles in their own PR.

I have a local branch on a similar idea, but it only changes builds in play, and I'm not offering it as it is. On Intel, main already builds every compute pipeline in play one at a time (#496). The branch records each stage's SPIR-V word count and keeps only programs with a stage of 100,000 words or more on that thread. The others go to the shared workers (4 on this machine). It doesn't touch precompile_run() or the shader preparation. The program cache here has 54 compute programs of 100,000 words or more, and 335 if the no-mesh fallback's programs are counted.

On the UHD 770 it didn't work out. Both runs started with no caches, with the shader preparation and the start-up precompile switched off (RX_VK_NO_WARM=1 RX_VK_NO_PREDICT=1 RX_VK_PRECOMPILE=0), loaded the same save, and were allowed 480 s with the game capped at 32 GB:

build result memory peak compute dispatches a frame in play
main 49f75dc8 ran the whole 480 s 14.9 GB 4
main + the branch killed by the OOM killer after 429 s 29.0 GB 48-99

More compute work ran with the branch. But the smaller builds on the shared workers, next to the one big build, still took the game to the cap. A second run of the branch, with #2110 merged as well, was killed after 306 s. The picture was wrong too, blown out to white with coloured bands. Main's picture on this GPU is also wrong, but in a different way. I haven't found the reason, so the branch stays local.

What our runs say about a threshold:

  • In the shader preparation with one worker, one 249,540-word program (53caac5f38279735) hadn't finished compiling after 7 min 49 s. The process went from 0.6 to 8.2 GB in that time and was still growing. So one at a time keeps the memory well under the cap, but each of these compiles is very slow on ANV.
  • With four workers, compiles of up to about 61,000 words kept the process at 3-4 GB for 16 minutes.
  • In play, a cut at 100,000 words wasn't enough by itself (the table above).

So your idea and our branch do different things. Your lock covers the precompile and the shader preparation, which our branch leaves alone. Our branch only changes builds in play on Intel, where main already builds every compute pipeline one at a time, and lets the small ones run side by side. Both use a word count. I can't say yet how far one of these compiles grows by the time it finishes, because it was still growing when my run ended.

When your follow-up is up, I'll run it on the UHD 770 with the same scene and post the numbers.

Added 2026-11-11 16:46 UTC: I have since found why the branch's picture was wrong; a frame dump shows it. The four shared workers also translate the graphics shaders, and with the branch they were busy with compute builds. Eight of those took 25 to 61 s each (their largest stages are about 28,000 to 66,000 words; the machine was busy, load average 100 to 120). From the save load to the end of the run the queue stayed at 449-516 jobs and shader translation hardly moved: 2,846 ms in all when the save loaded, 2,921 ms at the end. Near the end, 2,350-2,390 draws a frame were still left out because their shaders weren't ready. The dumped frame has 230 draws with the branch and 1,170 without it, so the world wasn't drawn and only the sky reached the upscale. The coloured bands are main's own false colours on this GPU, in both runs. For a threshold this adds: in play, compute compiles of that size already block everything that waits behind them on shared workers here, well below 100,000 words.

Sounds good to me: #1610 for the worker cap, and the very large compiles in their own PR. I have a local branch on a similar idea, but it only changes builds in play, and I'm not offering it as it is. On Intel, main already builds every compute pipeline in play one at a time (#496). The branch records each stage's SPIR-V word count and keeps only programs with a stage of 100,000 words or more on that thread. The others go to the shared workers (4 on this machine). It doesn't touch `precompile_run()` or the shader preparation. The program cache here has 54 compute programs of 100,000 words or more, and 335 if the no-mesh fallback's programs are counted. On the UHD 770 it didn't work out. Both runs started with no caches, with the shader preparation and the start-up precompile switched off (`RX_VK_NO_WARM=1 RX_VK_NO_PREDICT=1 RX_VK_PRECOMPILE=0`), loaded the same save, and were allowed 480 s with the game capped at 32 GB: | build | result | memory peak | compute dispatches a frame in play | |---|---|---|---| | main 49f75dc8 | ran the whole 480 s | 14.9 GB | 4 | | main + the branch | killed by the OOM killer after 429 s | 29.0 GB | 48-99 | More compute work ran with the branch. But the smaller builds on the shared workers, next to the one big build, still took the game to the cap. A second run of the branch, with #2110 merged as well, was killed after 306 s. The picture was wrong too, blown out to white with coloured bands. Main's picture on this GPU is also wrong, but in a different way. I haven't found the reason, so the branch stays local. What our runs say about a threshold: - In the shader preparation with one worker, one 249,540-word program (`53caac5f38279735`) hadn't finished compiling after 7 min 49 s. The process went from 0.6 to 8.2 GB in that time and was still growing. So one at a time keeps the memory well under the cap, but each of these compiles is very slow on ANV. - With four workers, compiles of up to about 61,000 words kept the process at 3-4 GB for 16 minutes. - In play, a cut at 100,000 words wasn't enough by itself (the table above). So your idea and our branch do different things. Your lock covers the precompile and the shader preparation, which our branch leaves alone. Our branch only changes builds in play on Intel, where main already builds every compute pipeline one at a time, and lets the small ones run side by side. Both use a word count. I can't say yet how far one of these compiles grows by the time it finishes, because it was still growing when my run ended. When your follow-up is up, I'll run it on the UHD 770 with the same scene and post the numbers. *Added 2026-11-11 16:46 UTC: I have since found why the branch's picture was wrong; a frame dump shows it. The four shared workers also translate the graphics shaders, and with the branch they were busy with compute builds. Eight of those took 25 to 61 s each (their largest stages are about 28,000 to 66,000 words; the machine was busy, load average 100 to 120). From the save load to the end of the run the queue stayed at 449-516 jobs and shader translation hardly moved: 2,846 ms in all when the save loaded, 2,921 ms at the end. Near the end, 2,350-2,390 draws a frame were still left out because their shaders weren't ready. The dumped frame has 230 draws with the branch and 1,170 without it, so the world wasn't drawn and only the sky reached the upscale. The coloured bands are main's own false colours on this GPU, in both runs. For a threshold this adds: in play, compute compiles of that size already block everything that waits behind them on shared workers here, well below 100,000 words.*
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 21:46:02 +00:00
Joey-Magnus (Migrated from github.com) left a comment

Main still sizes the precompile pool as min(usable_cpus(), 64) (vk_draw.cpp:8084), so the cap isn't on main yet. But the conflict is more than text: #2323 replaced hardware_concurrency() with usable_cpus()/half_cpus(), which honour process affinity on Windows. A rebase should build on half_cpus() rather than going back to hardware_concurrency().

Two things decide whether this goes in at all:

  • #2414 (open) caps the same pools at physical cores, precompile_run included, so taking both would stack two policies. One of them should go.
  • mooki000's UHD 770 numbers show the 16-worker cap only delays the OOM kill (124 s to 199 s). The trigger there is the ~250k-word compute programs compiling side by side, which the follow-up you mentioned would address.

Smaller point: HOOKSHOT_JOBS is the build's jobs variable, so reusing it throttles play for anyone who exports it globally. I'd keep only RX_VK_PRECOMPILE_JOBS.

Main still sizes the precompile pool as `min(usable_cpus(), 64)` (vk_draw.cpp:8084), so the cap isn't on main yet. But the conflict is more than text: #2323 replaced `hardware_concurrency()` with `usable_cpus()`/`half_cpus()`, which honour process affinity on Windows. A rebase should build on `half_cpus()` rather than going back to `hardware_concurrency()`. Two things decide whether this goes in at all: - #2414 (open) caps the same pools at physical cores, `precompile_run` included, so taking both would stack two policies. One of them should go. - mooki000's UHD 770 numbers show the 16-worker cap only delays the OOM kill (124 s to 199 s). The trigger there is the ~250k-word compute programs compiling side by side, which the follow-up you mentioned would address. Smaller point: `HOOKSHOT_JOBS` is the build's jobs variable, so reusing it throttles play for anyone who exports it globally. I'd keep only `RX_VK_PRECOMPILE_JOBS`.
oscarlosu-lab commented 2026-11-11 22:04:00 +00:00 (Migrated from github.com)

Thanks for the review, @Joey-Magnus! Updated in e3a158b1 with both changes:

  1. Affinity-aware cap (half_cpus()): precompile_run() now uses std::clamp(half_cpus(), 1u, 16u) so it builds directly on #2323's affinity-aware half_cpus() budget instead of raw usable_cpus() / 2 or hardware_concurrency(), and tests/test_prepare_cpus.py is updated to check precompile_run against half_cpus().
  2. Removed HOOKSHOT_JOBS fallback: Keeps only RX_VK_PRECOMPILE_JOBS for runtime precompile overrides so global build-time HOOKSHOT_JOBS settings do not throttle startup/play.

As noted, #2414 vs this half_cpus()/16-worker cap is a single-policy call for maintainers, and the follow-up for serializing oversized SPIR-V compiles (~250k words) from @mooki000's UHD 770 findings stays separated from this PR.

Thanks for the review, @Joey-Magnus! Updated in `e3a158b1` with both changes: 1. **Affinity-aware cap (`half_cpus()`)**: `precompile_run()` now uses `std::clamp(half_cpus(), 1u, 16u)` so it builds directly on #2323's affinity-aware `half_cpus()` budget instead of raw `usable_cpus() / 2` or `hardware_concurrency()`, and `tests/test_prepare_cpus.py` is updated to check `precompile_run` against `half_cpus()`. 2. **Removed `HOOKSHOT_JOBS` fallback**: Keeps only `RX_VK_PRECOMPILE_JOBS` for runtime precompile overrides so global build-time `HOOKSHOT_JOBS` settings do not throttle startup/play. As noted, #2414 vs this `half_cpus()`/16-worker cap is a single-policy call for maintainers, and the follow-up for serializing oversized SPIR-V compiles (~250k words) from @mooki000's UHD 770 findings stays separated from this PR.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 22:20:16 +00:00
hckrman101 (Migrated from github.com) left a comment

Re-review at e3a158b. Both of my earlier nits are handled: HOOKSHOT_JOBS is gone, so only RX_VK_PRECOMPILE_JOBS throttles the runtime pass, and the description now says the 16 cap is deliberately lower than the 32 used by the other pools. Joey-Magnus's rebase point is done too. The default is now clamp(half_cpus(), 1, 16), so it follows process affinity like the other pools since #2323, and test_prepare_cpus.py checks for half_cpus() in precompile_run. The code looks correct to me: a garbage or zero override falls back to the default, and the override is still capped at 64. Mergeable, and CI is green except the builder package job, which is still pending.

The one open question is the one Joey raised: #2414 (still open) caps the same precompile_run pool with a different policy (physical cores through compile_worker_count()) and edits the same lines. Its test_compile_threads.py also expects precompile_run to use that helper. So only one of the two can go in as written, and the other would need reworking. Choosing between them is a policy decision for @NotRexed, so I'm leaving this as a comment rather than an approval.

For the record, mooki000's UHD 770 runs show this cap only delays the OOM on ANV (124 s to 199 s). The cause there is the very large compute programs compiling side by side. The author has scoped that to a follow-up, and mooki000 agreed, so I don't count it against this PR. The description should just not suggest this fixes Intel.

Nit (inline): the new variable isn't documented anywhere.

Re-review at e3a158b. Both of my earlier nits are handled: `HOOKSHOT_JOBS` is gone, so only `RX_VK_PRECOMPILE_JOBS` throttles the runtime pass, and the description now says the 16 cap is deliberately lower than the 32 used by the other pools. Joey-Magnus's rebase point is done too. The default is now `clamp(half_cpus(), 1, 16)`, so it follows process affinity like the other pools since #2323, and `test_prepare_cpus.py` checks for `half_cpus()` in `precompile_run`. The code looks correct to me: a garbage or zero override falls back to the default, and the override is still capped at 64. Mergeable, and CI is green except the builder package job, which is still pending. The one open question is the one Joey raised: #2414 (still open) caps the same `precompile_run` pool with a different policy (physical cores through `compile_worker_count()`) and edits the same lines. Its `test_compile_threads.py` also expects `precompile_run` to use that helper. So only one of the two can go in as written, and the other would need reworking. Choosing between them is a policy decision for @NotRexed, so I'm leaving this as a comment rather than an approval. For the record, mooki000's UHD 770 runs show this cap only delays the OOM on ANV (124 s to 199 s). The cause there is the very large compute programs compiling side by side. The author has scoped that to a follow-up, and mooki000 agreed, so I don't count it against this PR. The description should just not suggest this fixes Intel. Nit (inline): the new variable isn't documented anywhere.
gpu/vk/vk_draw.cpp
hckrman101 (Migrated from github.com) commented 2026-11-11 22:20:16 +00:00

Nit: RX_VK_PRECOMPILE_JOBS is the only knob for this pool now, but nothing mentions it. A word in the comment above Pre, next to RX_VK_PRECOMPILE_STALL_S, would help. A one-line logf of the worker count would also make tester logs easier to compare (mooki000 had to infer it). Calling atoi once into a local would also read better.

Nit: `RX_VK_PRECOMPILE_JOBS` is the only knob for this pool now, but nothing mentions it. A word in the comment above `Pre`, next to `RX_VK_PRECOMPILE_STALL_S`, would help. A one-line `logf` of the worker count would also make tester logs easier to compare (mooki000 had to infer it). Calling `atoi` once into a local would also read better.
oscarlosu-lab commented 2026-11-11 22:56:22 +00:00 (Migrated from github.com)

Rebased onto eaef1460 in 9c17e2b2. hckrman101's inline nit is in: RX_VK_PRECOMPILE_JOBS is named in the comment above Pre next to RX_VK_PRECOMPILE_STALL_S, atoi is read once into env_n, and the pass logs vk: precompile: N workers (M pipelines) so tester logs can be compared. docs/development.md has the row.

The description now says what this does and does not cover: it bounds concurrent compiles (the #1593 RADV case) and does not fix the Intel ANV OOM from the ~250k-word compute programs, which needs those compiles serialized on their own (follow-up).

On #2414: it caps the pools by physical cores and keeps every smaller stage ceiling, so this PR's 16-worker clamp would sit below its cap. They compose; which policy you want as the default is your call.

Rebased onto eaef1460 in 9c17e2b2. hckrman101's inline nit is in: `RX_VK_PRECOMPILE_JOBS` is named in the comment above `Pre` next to `RX_VK_PRECOMPILE_STALL_S`, `atoi` is read once into `env_n`, and the pass logs `vk: precompile: N workers (M pipelines)` so tester logs can be compared. docs/development.md has the row. The description now says what this does and does not cover: it bounds concurrent compiles (the #1593 RADV case) and does not fix the Intel ANV OOM from the ~250k-word compute programs, which needs those compiles serialized on their own (follow-up). On #2414: it caps the pools by physical cores and keeps every smaller stage ceiling, so this PR's 16-worker clamp would sit below its cap. They compose; which policy you want as the default is your call.
oscarlosu-lab commented 2026-11-11 23:14:47 +00:00 (Migrated from github.com)

Note on the current CI failures on 9c17e2b2 — all of them come from origin/main rather than this PR (tests/test_prepare_cpus.py, Windows Vulkan, appimage, dav1d, SPIR-V fixtures, and Python source checks (ubuntu-24.04, part 1/2) all passed):

  • Python source checks (windows-2022, part 1/2): fails only in tests/test_env_on.py (HOS_FPS_LOG in host/oot_high_fps.c:15, from 8fcc8c22 / #2470) and tests/test_psychov35.py (std::scanf under -Werror,-Wdeprecated-declarations, missing _CRT_SECURE_NO_WARNINGS, from #2481).
  • Python source checks (windows-2022, part 2/2): fails only in tests/test_pcache_header_read.py (fopen under -Werror,-Wdeprecated-declarations, missing _CRT_SECURE_NO_WARNINGS, from #2488).

Happy to rebase cleanly once those are fixed on main.

Note on the current CI failures on `9c17e2b2` — all of them come from `origin/main` rather than this PR (`tests/test_prepare_cpus.py`, Windows Vulkan, `appimage`, `dav1d`, `SPIR-V fixtures`, and `Python source checks (ubuntu-24.04, part 1/2)` all passed): - `Python source checks (windows-2022, part 1/2)`: fails only in `tests/test_env_on.py` (`HOS_FPS_LOG` in `host/oot_high_fps.c:15`, from `8fcc8c22` / #2470) and `tests/test_psychov35.py` (`std::scanf` under `-Werror,-Wdeprecated-declarations`, missing `_CRT_SECURE_NO_WARNINGS`, from #2481). - `Python source checks (windows-2022, part 2/2)`: fails only in `tests/test_pcache_header_read.py` (`fopen` under `-Werror,-Wdeprecated-declarations`, missing `_CRT_SECURE_NO_WARNINGS`, from #2488). Happy to rebase cleanly once those are fixed on `main`.
github-sync commented 2026-11-12 14:41:30 +00:00
Owner

@Joey-Magnus reviewed on GitHub · 2026-11-12 14:32 UTC · original

The rebase onto half_cpus() is done and HOOKSHOT_JOBS is gone, so my earlier points are handled. I merged 9c17e2b into current main (clean) and ran test_prepare_cpus and test_env_on, and both pass. The three red Python jobs on this head come from main's breakage yesterday (test_env_on on HOS_FPS_LOG, and test_psychov35 and test_pcache_header_read on Windows). Current main has fixed those, so a re-run should clear them.

One small thing: hookshot_env.h has env_int(name, def) for numeric switches. env_int("RX_VK_PRECOMPILE_JOBS", 0) could replace the getenv and atoi pair at vk_draw.cpp:8142.

The open question is policy, not code. #2414 is still open and caps the same precompile_run pool by physical cores, and its test_compile_threads expects its own helper there, so only one of the two can go in as written. As the description says, this doesn't fix mooki000's UHD 770 out-of-memory kill. That needs the very large compiles serialized.

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 14:32 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1610#pullrequestreview-5876200198) The rebase onto half_cpus() is done and HOOKSHOT_JOBS is gone, so my earlier points are handled. I merged 9c17e2b into current main (clean) and ran test_prepare_cpus and test_env_on, and both pass. The three red Python jobs on this head come from main's breakage yesterday (test_env_on on HOS_FPS_LOG, and test_psychov35 and test_pcache_header_read on Windows). Current main has fixed those, so a re-run should clear them. One small thing: hookshot_env.h has env_int(name, def) for numeric switches. `env_int("RX_VK_PRECOMPILE_JOBS", 0)` could replace the getenv and atoi pair at vk_draw.cpp:8142. The open question is policy, not code. #2414 is still open and caps the same precompile_run pool by physical cores, and its test_compile_threads expects its own helper there, so only one of the two can go in as written. As the description says, this doesn't fix mooki000's UHD 770 out-of-memory kill. That needs the very large compiles serialized. <!-- github-sync:r:5876200198 -->
github-sync commented 2026-11-12 16:54:44 +00:00
Owner

@oscarlosu-lab commented on GitHub · 2026-11-12 16:08 UTC · original

Rebased cleanly on latest main (51da0464) and adopted env_int("RX_VK_PRECOMPILE_JOBS", 0) and env_int("RX_VK_PRECOMPILE_STALL_S", 30) from hookshot_env.h as suggested. Local checks (test_prepare_cpus.py, test_env_on.py, test_run_all_declarations.py) all pass.

> [@oscarlosu-lab](https://github.com/oscarlosu-lab) commented on GitHub · 2026-11-12 16:08 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1610#issuecomment-6828900862) Rebased cleanly on latest `main` (`51da0464`) and adopted `env_int("RX_VK_PRECOMPILE_JOBS", 0)` and `env_int("RX_VK_PRECOMPILE_STALL_S", 30)` from `hookshot_env.h` as suggested. Local checks (`test_prepare_cpus.py`, `test_env_on.py`, `test_run_all_declarations.py`) all pass. <!-- github-sync:c:6828900862 -->
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Hookshot!1610
No description provided.