Skip to content

Use dynamic CPU count for cmake --build -j in docs and test scripts - #20436

Merged
JakeStevens merged 1 commit into
pytorch:mainfrom
ShamSaleem:fix-10887-portable-cmake-jobs
Jul 22, 2026
Merged

Use dynamic CPU count for cmake --build -j in docs and test scripts#20436
JakeStevens merged 1 commit into
pytorch:mainfrom
ShamSaleem:fix-10887-portable-cmake-jobs

Conversation

@ShamSaleem

@ShamSaleemShamSaleem commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Several build docs and test/ scripts hardcode the cmake --build -j
parallelism (-j9, -j10), which assumes a fixed machine. This replaces
them with a portable expression that derives "core count + 1" at runtime —
nproc on Linux, sysctl -n hw.ncpu on macOS:

-j$(( $(nproc 2>/dev/null || sysctl -n hw.ncpu) + 1 ))

"core count + 1" matches the guidance already documented in
docs/source/using-executorch-building-from-source.md. The nproc → sysctl
fallback keeps the commands working on both Linux and macOS, and the
arithmetic degrades gracefully to -j1 if neither tool is available.

Partial fix for #10887. Scope is limited to general (non-vendor) docs and
contributor-facing test/ build scripts (9 files). Vendor-backend scripts
(cadence, vulkan, coreml, qualcomm, mediatek, samsung, mps, nxp), CI scripts
under .ci/, and non-cmake -j flags are intentionally left for follow-ups.

Test plan

  • lintrunner passes on all changed files.
  • bash -n passes on the three modified shell scripts.
  • Verified the expression evaluates to a valid integer on Linux, e.g. 17
    on a 16-core machine.

cc @GregoryComer@digantdesai@cbilgin@JakeStevens@larryliu0820

@pytorch-bot

pytorch-botBot commented Jun 22, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20436

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ You can merge normally! (1 Unrelated Failure)

As of commit 3e079a1 with merge base 266e0dc (image):

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-clameta-claBot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 22, 2026
@linux-foundation-easycla

linux-foundation-easyclaBot commented Jun 22, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: ShamSaleem / name: ShamSaleem (b340e88)

@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.


```bash
cmake --build cmake-out -j9 --target install --config Release
cmake --build cmake-out -j$(( $(nproc 2>/dev/null || sysctl -n hw.ncpu) + 1 )) --target install --config Release

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this work on a macos?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ShamSaleem, could you clarify on this review comment?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, I was on vacation. But yeah, it should work on macOS. nproc isn't available there by default, so nproc 2>/dev/null fails quietly and it falls back to sysctl -n hw.ncpu, which is the macOS equivalent. The $(( ... + 1 )) is POSIX arithmetic, so it's fine in both bash and zsh.

I don't have a Mac to test on though — if someone can confirm, that'd be great.

@nil-is-allnil-is-all added module: xnnpack Issues related to xnnpack delegation and the code under backends/xnnpack/ module: build/install Issues related to the cmake and buck2 builds, and to installing ExecuTorch labels Jul 14, 2026
Replace hardcoded -j values (-j9, -j10) in the general build
documentation and the test/ build scripts with a portable expression
that derives "core count + 1" at runtime: nproc on Linux and
sysctl -n hw.ncpu on macOS. This matches the recommendation already
stated in docs/source/using-executorch-building-from-source.md and
avoids machine-specific job counts that don't fit the user's hardware.
Scope is limited to general (non-vendor) docs and contributor-facing
test/ scripts. Vendor-backend scripts, CI scripts under .ci/, and
non-cmake -j flags are intentionally left for a follow-up.
Partially addresses pytorch#10887.
@ShamSaleem
ShamSaleemforce-pushed the fix-10887-portable-cmake-jobs branch from b340e88 to 3e079a1CompareJuly 22, 2026 11:17
@JakeStevens
JakeStevens merged commit 7f86746 into pytorch:mainJul 22, 2026
194 of 195 checks passed
@nil-is-allnil-is-all added enhancement Not as big of a feature, but technically not a bug. Should be easy to fix module: cleanup Issues/PRs which cleanup code across the repository labels Jul 29, 2026
ShamSaleem added a commit to ShamSaleem/executorch that referenced this pull request Sep 1, 2026
Follow-up to pytorch#20436, which converted the general build docs and test/
scripts. This covers the vendor backend scripts, example scripts, and
their documentation: 59 sites across 34 files, replacing pinned values
from -j4 to -j100 with
-j$(( $(nproc 2>/dev/null || sysctl -n hw.ncpu) + 1 ))
nproc on Linux, sysctl on macOS, degrading to -j1 if neither exists.
"Core count + 1" matches the guidance in the building-from-source doc.
backends/mlx/test/test_utils.py builds an argv list with no shell, so a
shell expression would reach cmake as a literal string; it uses
f"-j{(os.cpu_count() or 1) + 1}" instead.
.ci/ and .github/workflows/ are left alone: those runners are fixed-size
and the parallelism there is resource tuning, not a portability defect.
Partial fix for pytorch#10887.
JakeStevens pushed a commit that referenced this pull request Sep 6, 2026
### Summary
Follow-up to #20436, which replaced the hardcoded `cmake --build -j`
parallelism in the general
build docs and `test/` scripts. This PR finishes the same job for the
vendor backend scripts,
example scripts, and their documentation — 65 sites across 37 files, all
mechanical:
-j$(( $(nproc 2>/dev/null || sysctl -n hw.ncpu) + 1 ))
`nproc` on Linux, `sysctl -n hw.ncpu` on macOS (the mps and coreml
scripts are Apple-only), and the
arithmetic degrades to `-j1` if neither tool exists. "Core count + 1" is
the guidance already in
`docs/source/using-executorch-building-from-source.md`. The pinned
values being removed ranged from
`-j4` to `-j100`, including `-j64` in the Vulkan test scripts and
`-j100` in
`tools/cmake/preset/README.md`.
Two Python sites differ. `extension/llm/export/quantizer_lib.py` holds a
shell command inside a
user-facing error string, so it takes the same shell expression.
`backends/mlx/test/test_utils.py`
builds an argv list handed to `subprocess.run` with no shell, where a
shell expression would reach
`cmake` as a literal string, so it uses `f"-j{(os.cpu_count() or 1) +
1}"` instead.
Deliberately out of scope: `.ci/**` and `.github/workflows/**`, where
the runners are fixed-size and
the parallelism is a resource-tuning decision rather than a portability
problem (`cuda.yml` pins
`-j4`, likely to bound peak memory); the `-j4` in `backends/mlx`'s
READMEs and `run_all_tests.py`,
which is a test-worker count and not a build flag; and
`docs/source/archive/`. Happy to take the CI
files in a separate PR if you'd like them changed.
Review order: the three groups are independent — vendor backend scripts
under `backends/`, example
scripts and docs under `examples/`, then the two Python files, which are
the only sites that are not
a pure token swap.
Partial fix for #10887.
### Test plan
ExecuTorch does not build on my Windows host, so verification is static
and per-site:
- `bash -n` passes on all 20 modified shell scripts.
- Every edited command line was re-run with `cmake --build`/`make`
swapped for `echo`, confirming all
65 sites expand to a single valid integer flag (`-j17` on this 16-core
machine) with zero
expansion failures. This covers the markdown sites too, including the
two lines that begin with
`&& ` and the one with a `$ ` prompt prefix.
- `git diff` normalised on the `-j` token shows every removed line has a
matching added line, so
nothing outside the flag changed. Every changed markdown line contains a
`-j` token.
- Both Python files parse; the argv site renders `-j17`; the instruction
string was extracted via
`ast` and shell-expanded to confirm a user pasting it gets `-j17`.
- `black --check` reports both Python files unchanged. Line endings are
unchanged (still LF) and
`git diff --check` reports no whitespace errors.
- `lintrunner` was **not** run locally — it is not installed on this
Windows host and is unavailable
in my WSL environment, so CI lint is the gate for that. E501 is in the
repo's flake8 ignore list,
so the one long instruction string in `quantizer_lib.py` (already 165
chars before this change) is
not a new violation.
This PR was authored with AI assistance (Claude Code); the diff and
every verification step above
were reviewed by me.
cc @GregoryComer@digantdesai@cbilgin@JakeStevens@larryliu0820
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.enhancementNot as big of a feature, but technically not a bug. Should be easy to fixmodule: build/installIssues related to the cmake and buck2 builds, and to installing ExecuTorchmodule: cleanupIssues/PRs which cleanup code across the repositorymodule: xnnpackIssues related to xnnpack delegation and the code under backends/xnnpack/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ShamSaleem@digantdesai@JakeStevens@nil-is-all