ci: Cap the check job at 8 concurrent compile jobs - #1703
Open
daniel-noland wants to merge 2 commits into
Open
Conversation
The lab runners give us 10 cores, and nothing was holding the build to them. `jobs` was already 8, but it feeds `nix build --max-jobs`, which counts derivations, not threads. The per-derivation thread count comes from `--cores`, which we never passed, so nix used its default of 0 and handed every derivation the whole machine: crane's configureCargoCommonVarsHook turns NIX_BUILD_CORES into CARGO_BUILD_JOBS, and the C derivations we build with enableParallelBuilding turn it into `make -j`. The ceiling was 8 derivations times 10 threads. So add a `cores` variable to pass through to `--cores`. It defaults to "0", leaving every other caller exactly where it was, and the check job sets `jobs=1 cores=8` to bring the product down to 8. Serializing derivations costs little here because one cargo build of the workspace dominates the job, and that build is the one that wants the 8 threads. The env block the check job sets this in is the `check-env` anchor, so the sanitize and test_each jobs inherit the same cap. They are compile bound on the same runners, so that is the behaviour we want there too. Note that `cargo nextest run` is still uncapped: the `test` recipe runs it on the host once the nix build is done, outside the sandbox, where `--cores` no longer reaches it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
📝 WalkthroughWalkthroughThe change adds configurable core limits to Nix, Cargo, nextest, and GitHub Actions workloads. Standard jobs use eight cores, cross jobs use four cores, and Miri applies its own core setting. ChangesResource controls
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens CI build parallelism on lab runners by distinguishing between Nix derivation concurrency (--max-jobs) and per-derivation thread parallelism (--cores), then setting CI defaults to avoid oversubscribing CPU.
Changes:
- Introduces a
coresvariable injustfileand threads it through tonix build --cores. - Updates the CI
checkjob’sJUST_VARS(via thecheck-envanchor) to usejobs=1 cores=8, capping total compile/link parallelism.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| justfile | Adds a cores variable and passes it to nix build --cores to control per-derivation parallelism. |
| .github/workflows/dev.yml | Sets jobs/cores in the check-env anchor so check (and inheriting jobs) are CPU-capped on lab runners. |
The previous commit capped `check` (and, through the `check-env` anchor, `sanitize` and `test_each`) at one nix derivation times eight compile threads. Every other job on the lab runners was still taking the whole machine, so do the same for them. `build`, `wasm`, `cross`, `concurrency`, and `publish` all reach `nix build` eventually, so they only need `jobs=1 cores=8` added to whichever `JUST_VARS` (or argv) they already assemble. `publish` needed one more thing: the `push` recipe re-invokes `just` per container and forwards an explicit list of variables, so `jobs` and `cores` had to join that list or they would have been dropped on the way to `push-container`. `cross` gets `cores=4` rather than 8. It is the only job with `max-parallel: 2`, so if both matrix entries land on the same runner the 8 would have been 16 threads on a 10 core box. `miri` is the odd one out. It never calls `nix build` at all -- it runs `cargo miri nextest run` on the host inside `nix-shell` -- so `--cores` has nothing to attach to and the root justfile's variable cannot reach the module. The miri module therefore gets its own `cores`, which it hands to cargo as CARGO_BUILD_JOBS for the compile and to nextest as `--test-threads` for the run. It defaults to "0" (unset, every core), so a local `just miri::test` is unchanged. Note that the run-phase cap is mostly a formality on the two `concurrency` steps: they set `seeds=8`, which the miri nextest profile turns into `threads-required = 8`, and that already limited them to one test at a time on a 10 core box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@justfile`:
- Around line 341-342: Update the nested just invocation near the publish
workflow to pass docker_sock="{{docker_sock}}", and add docker_sock to the
preserved-parameters comment. Keep the existing parameter forwarding and command
behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbd09652-b523-4e45-82d4-44e27bdf52a6
📒 Files selected for processing (3)
.github/workflows/dev.ymljustfilemiri.just
Comment on lines
+341
to
+342
| # Note: deliberately ignores all recipe parameters save version, debug_justfile, | ||
| # oci_repo, and the jobs/cores build-parallelism caps. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Forward docker_sock to the nested just invocation.
The publish workflow passes docker_sock=/run/docker/docker.sock to the outer just push invocation. The nested just command at Line 353 does not receive this variable. Unless DOCKER_HOST is already exported, it falls back to /var/run/docker.sock from Line 128. On runners that require the configured socket, release publishing fails.
Add docker_sock="{{docker_sock}}" to the nested command. Include it in the preserved-parameters comment.
Proposed fix
-# oci_repo, and the jobs/cores build-parallelism caps.
+# oci_repo, docker_sock, and the jobs/cores build-parallelism caps.
...
- just jobs="{{jobs}}" cores="{{cores}}" debug_justfile="{{debug_justfile}}" oci_repo="{{oci_repo}}" version="{{version}}" profile=release platform="${platform}" sanitize= instrument=none push-container "${container}"
+ just jobs="{{jobs}}" cores="{{cores}}" docker_sock="{{docker_sock}}" debug_justfile="{{debug_justfile}}" oci_repo="{{oci_repo}}" version="{{version}}" profile=release platform="${platform}" sanitize= instrument=none push-container "${container}"Also applies to: 353-353
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@justfile` around lines 341 - 342, Update the nested just invocation near the
publish workflow to pass docker_sock="{{docker_sock}}", and add docker_sock to
the preserved-parameters comment. Keep the existing parameter forwarding and
command behavior unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The lab runners give us 10 cores, and nothing was holding the build to them.
jobswas already 8, but it feedsnix build --max-jobs, which counts derivations, not threads. The per-derivation thread count comes from--cores, which we never passed, so nix used its default of 0 and handed every derivation the whole machine: crane'sconfigureCargoCommonVarsHookturnsNIX_BUILD_CORESintoCARGO_BUILD_JOBS, and the C derivations we build withenableParallelBuildingturn it intomake -j. The ceiling was 8 derivations times 10 threads.So this adds a
coresvariable to pass through to--cores. It defaults to"0", leaving every other caller exactly where it was, and thecheckjob setsjobs=1 cores=8to bring the product down to 8. Serializing derivations costs little here because one cargo build of the workspace dominates the job, and that build is the one that wants the 8 threads.🤖 Generated with Claude Code