Skip to content

fix(cmake): Explicitly use the logical CPU count when JOBS is unset.#116

Closed
jackluo923 wants to merge 7 commits into
mainfrom
fix/cmake-conditional-parallel
Closed

fix(cmake): Explicitly use the logical CPU count when JOBS is unset.#116
jackluo923 wants to merge 7 commits into
mainfrom
fix/cmake-conditional-parallel

Conversation

@jackluo923

@jackluo923 jackluo923 commented Jun 30, 2026

Copy link
Copy Markdown
Member

Description

cmake:build always emitted --parallel "{{.JOBS}}" with JOBS defaulting to the empty string, so an unset JOBS rendered cmake --build <dir> --parallel "". cmake turns that into bare gmake -f Makefile -j, i.e. unbounded parallelism — a single build could spawn far more compiler processes than there are cores, oversubscribing the machine and risking OOM on large dependencies like folly.

Bound the default to the number of logical CPUs (nproc/sysctl, falling back to 1), so --parallel always receives a finite, right-sized value and a single build no longer oversubscribes. Callers can still override via JOBS (or CMAKE_JOBS, which flows through install-remote-tar -> build).

This matches boost:build-and-install (b2 defaults to the core count for boost >= 1.76.0). Cross-build scheduling — running several cmake:build invocations concurrently — remains the caller's responsibility; set JOBS lower to avoid oversubscribing across builds.

Cross-platform: nproc (Linux) -> sysctl -n hw.logicalcpu (macOS) -> echo 1.

Validation performed

task lint:check and task test pass.

Scratch Taskfile.yml including utils/utils.yaml, wrapping utils:cmake:build (host has 12 logical CPUs):

Input Rendered command
no JOBS cmake --build "build" --parallel "12"
JOBS= (explicit empty) cmake --build "build" --parallel "12"
JOBS=5 cmake --build "build" --parallel "5" (override respected)

For reference, the pre-fix empty case rendered --parallel "" -> bare gmake -f Makefile -j (unlimited).

Summary by CodeRabbit

  • Bug Fixes
    • Build tasks now default to a sensible CPU-count-based parallel job value when no job count is provided, improving performance out of the box.
    • Updated installation task documentation so its job-setting guidance matches the build task’s CPU-count default behavior.

`cmake:build` always emitted `--parallel "{{.JOBS}}"`, so an empty/unset
`JOBS` rendered `cmake --build <dir> --parallel ""`, which cmake turns into
bare `gmake -f Makefile -j` — GNU make's bare `-j` means *unlimited*
parallelism (no cap on concurrent jobs). For a large dependency this can
spawn hundreds of compiler processes and risk OOM.

This made the documented behavior ("If omitted, the native build tool's
default number is used") untrue. `boost.yaml` already guards its `-j` with
`{{- if .JOBS}}`; apply the same conditional to `cmake:build` so an empty
`JOBS` omits `--parallel` entirely and the native build tool's default
applies. `install-remote-tar` delegates to `build`, so it is covered too.
Consumers that pass a `JOBS` value are unaffected.
@jackluo923 jackluo923 requested a review from a team as a code owner June 30, 2026 00:52
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The CMake taskfile now sets a default JOBS value from the local CPU count when unset, and updates the related CMAKE_JOBS comment to match the build task behaviour.

Changes

CMake JOBS default handling

Layer / File(s) Summary
Default JOBS from CPU count
exports/taskfiles/utils/cmake.yaml
The build task now computes _CPU_COUNT with nproc or sysctl with a fallback of 1, uses it as the default JOBS, and updates the CMAKE_JOBS comment to reference the build task’s JOBS.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: defaulting unset JOBS to the logical CPU count.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cmake-conditional-parallel

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

`cmake:build` always emitted `--parallel "{{.JOBS}}"` with `JOBS` defaulting to
the empty string, so an unset `JOBS` rendered `cmake --build <dir> --parallel ""`.
cmake turns that into bare `gmake -f Makefile -j`, i.e. unbounded parallelism —
a single build could spawn far more compiler processes than there are cores,
oversubscribing the machine and risking OOM on large dependencies.

Bound the default to the number of logical CPUs (`nproc`/`sysctl`, falling back
to `1`), so `--parallel` always receives a finite, right-sized value and a
single build no longer oversubscribes. Callers can still override via `JOBS`
(or `CMAKE_JOBS`, which flows through `install-remote-tar` -> `build`).

This matches `boost:build-and-install` (b2 defaults to the core count for
boost >= 1.76.0). Cross-build scheduling — running several `cmake:build`
invocations concurrently — remains the caller's responsibility; set `JOBS`
lower to avoid oversubscribing across builds.

Cross-platform: `nproc` (Linux) -> `sysctl -n hw.logicalcpu` (macOS) -> `echo 1`.
@jackluo923 jackluo923 changed the title fix(cmake): Only pass --parallel when JOBS is set, matching boost fix(cmake): Bound build parallelism to the CPU count when JOBS is unset Jun 30, 2026
The `JOBS` doc said the nproc default "matches the `boost:build-and-install`
default", but that coupling is only conditionally true (b2 defaults to the
core count for boost >= 1.76.0, via a different mechanism — omitting `-j` vs
passing `--parallel`), and it makes `cmake.yaml`'s param doc depend on another
module's behaviour. Drop the cross-reference so the comment is self-contained.
@Bill-hbrhbr Bill-hbrhbr self-requested a review June 30, 2026 03:45
Comment thread exports/taskfiles/utils/cmake.yaml Outdated
Comment thread exports/taskfiles/utils/cmake.yaml Outdated
Comment thread exports/taskfiles/utils/cmake.yaml Outdated
jackluo923 and others added 3 commits June 29, 2026 22:13
Co-authored-by: Bingran Hu <bingran.hu@yscope.com>
Co-authored-by: Bingran Hu <bingran.hu@yscope.com>
Co-authored-by: Bingran Hu <bingran.hu@yscope.com>
@jackluo923 jackluo923 requested a review from Bill-hbrhbr June 30, 2026 05:14
Bill-hbrhbr
Bill-hbrhbr previously approved these changes Jun 30, 2026
Comment thread exports/taskfiles/utils/cmake.yaml Outdated
@Bill-hbrhbr Bill-hbrhbr changed the title fix(cmake): Bound build parallelism to the CPU count when JOBS is unset fix(cmake): Explicitly use the logical CPU count when JOBS is unset. Jun 30, 2026
@jackluo923

Copy link
Copy Markdown
Member Author

After chatting with @davidlion offline, we've decided on a more comprehensive fix.

@jackluo923 jackluo923 closed this Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants