Skip to content

Conversation

@steveisok
Copy link
Member

@steveisok steveisok commented Feb 5, 2026

Benchmarking showed ninja provides ~8.8% faster builds on macOS (avg 654s vs 718s per build). This change defaults to using ninja for both MacOS and Linux.

Changes:

  • eng/build.sh: default to using ninja
  • eng/native/build-commons.sh: Default -ninja on macOS, add -ninja false opt-out

Users can opt out with --ninja false (or -ninja false for native builds).

Contributes to #54022 (#54022)

Benchmarking showed ninja provides ~8.8% faster builds on macOS
(avg 654s vs 718s per build). This change defaults to using ninja
when the host OS is macOS.

Changes:
- eng/build.sh: Default --ninja true on macOS, track explicit user override
- eng/native/build-commons.sh: Default -ninja on macOS, add -ninja false opt-out

Users can opt out with --ninja false (or -ninja false for native builds).
@steveisok steveisok requested review from a team and Copilot February 5, 2026 13:01
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves macOS build performance by defaulting to the Ninja build system instead of Make, achieving approximately 8.8% faster build times (654s vs 718s average). The change is opt-out via --ninja false to maintain flexibility.

Changes:

  • Default ninja build system on macOS for improved performance
  • Add explicit flag tracking to prevent overriding user preferences
  • Update help text to document the new default and opt-out mechanism

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
eng/build.sh Adds macOS-specific ninja default with tracking to avoid overriding explicit user settings
eng/native/build-commons.sh Sets ninja as default on macOS and enhances -ninja flag to accept true/false values

@jkotas
Copy link
Member

jkotas commented Feb 5, 2026

docs\workflow\requirements\macos-requirements.md needs to be updated - ninja is no longer optional by default after this change.

@jkotas jkotas added area-Infrastructure and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 5, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

@am11
Copy link
Member

am11 commented Feb 5, 2026

Windows also defaults to ninja. Maybe we should flip the switch: --skipninja and cherry-pick installation script change a2b49ec. It's a tiny script dependency considering everything else required by the build.

@jkotas
Copy link
Member

jkotas commented Feb 5, 2026

--skipninja

Nit: Avoiding negations in names is a generally better UX design. I like -ninja false better than -skipninja.

- Use single useNinja variable in eng/build.sh instead of tracking explicit-ness
- Initialize __UseNinja=0 in build-commons.sh, then override to 1 on macOS
- Remove __UseNinja=0 initialization from build-runtime.sh and tests/build.sh
  so the macOS default takes effect
- Update macOS requirements docs to reflect ninja is now default
@jkoritzinsky
Copy link
Member

I was just talking with @agocke about requiring Ninja on macOS to make it easier to maintain our Swift usage (as CMake only supports Swift with Ninja and XCode) so we don't need to manually invoke the Swift compiler and do our own object file handling.

I think this is a great first step in that direction!

Based on PR feedback, default to using Ninja for native builds on any
Unix platform where ninja is installed, rather than only on macOS.
This aligns with the Mono subtree behavior and provides faster builds
when ninja is available.
Copilot AI review requested due to automatic review settings February 5, 2026 22:48
@steveisok steveisok changed the title Default to ninja on macOS for faster builds Default to ninja if installed for faster builds (mac & linux) Feb 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

@steveisok
Copy link
Member Author

@jkotas @jkoritzinsky can I get your approval?

@jkotas
Copy link
Member

jkotas commented Feb 7, 2026

Do we have some numbers for the build time improvement on Linux?

@steveisok
Copy link
Member Author

steveisok commented Feb 7, 2026

Do we have some numbers for the build time improvement on Linux?

I didn't run it b/c there was preference to just turn it on for both.

I'll run something just so we know.

@jkoritzinsky
Copy link
Member

Last time we got numbers the improvement was marginal (on the order of a few seconds) but there was an improvement. I think for such a situation a default of "use if available" is reasonable.

#54022 (comment)

@am11
Copy link
Member

am11 commented Feb 7, 2026

It was implemented that way when you first asked, but later reverted due to concerns about build determinism (see #124041 (comment)). I believe build determinism is a separate issue, and Ninja by itself cannot meaningfully affect it: even small differences in toolchain versions or machine environments prevent bit-for-bit identical outputs. Docker largely addresses this on Linux by fixing the environment, but on other platforms there are more fundamental sources of nondeterminism to address, well beyond what Ninja alone can affect.

@steveisok
Copy link
Member Author

Ok, @am11 votes for use if installed. @jkoritzinsky you appear to be as well.

@jkotas?

I don't have a strong opinion, but if I had to pick, I'd choose ninja by default and only opt out explicitly.

@jkotas
Copy link
Member

jkotas commented Feb 7, 2026

numbers for the build time improvement on Linux

I see 5% improvement for ./build.sh -s clr -c release in mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-net11.0-cross-amd64 container on standard issue devbox. (~610seconds without ninja, 580seconds with ninja, average of 10+ builds)

I'd choose ninja by default

Yes, I think we should have it on by default for macOS and Linux at least, so that folks do not accidentally forget to install it and get worse build times.

other platforms

I am fine with off by default (or automatic light up) for other platforms if it helps.

@jkoritzinsky
Copy link
Member

I'm okay with on-by-default as well. I just figured that use-if-available is an easier change to accept if we weren't sure if we wanted to switch.

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Copilot AI review requested due to automatic review settings February 7, 2026 19:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

When -ninja was followed by another flag (e.g. --ninja --subset clr),
the parser would consume the next argument as a ninja value and break
argument parsing. Now check if $2 starts with '-' before treating it
as a true/false value.
@steveisok
Copy link
Member Author

I see 5% improvement for ./build.sh -s clr -c release in mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-net11.0-cross-amd64 container on standard issue devbox. (~610seconds without ninja, 580seconds with ninja, average of 10+ builds)

Ran on WSL for a 4 hour loop. 12% better there on average.

Copilot AI review requested due to automatic review settings February 8, 2026 04:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@wfurt
Copy link
Member

wfurt commented Feb 9, 2026

This broke runtime-libraries enterprise-linux (that was skipped on the PR). #124187
It would be nice if it falls back automatically if Ninja is not present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants