Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable broken Make jobserver support on OSX to fix parallel builds #229

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kesyog
Copy link

@kesyog kesyog commented Dec 27, 2024

On OSX, CMake spawns child processes via libuv, which uses the Apple-specific posix_spawn attribute
POSIX_SPAWN_CLOEXEC_DEFAULT. This attribute blocks child processes, including make, from inheriting all non-stdio file descriptors from the parent process, including those of the Cargo jobserver.

Because make is passed jobserver information via MAKEFLAGS but is unable to access the jobserver, it prints the error shown below and proceeds to run a single-threaded build:

make: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.

As a workaround, disable jobserver support on OSX so that cmake-rs instead passes --parallel $NUM_JOBS to CMake, allowing for a faster, multi-threaded build.

Fixes #177

@NobodyXu
Copy link

Note that newer jobserver protocol supports fifo, which passes a path to the filesystem instead of fds.

Jobserver already supports this but I believe it's not the default.

@kesyog
Copy link
Author

kesyog commented Dec 28, 2024

Note that newer jobserver protocol supports fifo, which passes a path to the filesystem instead of fds.

Jobserver already supports this but I believe it's not the default.

Yes, but as you noted in rust-lang/cargo#13483 (comment), this is unlikely to change in Cargo for backwards compatibility reasons. The named pipe jobserver isn’t supported by older versions of make.

An alternate approach I considered is to call make directly rather than cmake --build (for make builds only) so that the jobserver fd’s are forwarded correctly.
However:

  • Using cmake --build is more portable
  • We'd have to translate some options meant for cmake into the relevant make arguments, essentially replicating cmake --build functionality
  • It’d be a breaking change that would make build_arg awkward to define and use. Edit: this is wrong, as build args are passed to the build executable (e.g. make), not cmake.

@NobodyXu
Copy link

I think it's possible for cargo to inherit jobserver?

I.e. a newer version of make creates a fifo and passes it to cargo, cargo passes it to build-script

@kesyog
Copy link
Author

kesyog commented Dec 29, 2024

Based on https://github.com/rust-lang/cargo/blob/master/src/cargo/core/compiler/build_runner/mod.rs#L93-L110, I think you're right. Cargo will first try to inherit a jobserver if available and will otherwise create a file descriptor-based jobserver. While the default version of make on macOS Sequoia predates FIFO-based jobservers, one could theoretically have installed a newer version of make and be using a nested Cargo instance within a make job.

While this feels like a niche use case, we could handle this without much added complexity by allowing jobserver usage on OSX if and only if CARGO_MAKEFLAGS indicates we are using the FIFO-based jobserver protocol. Let me take a stab at that.

On OSX, CMake (via libuv) spawns child processes, including make, via
the Apple-specific `posix_spawn` attribute
`POSIX_SPAWN_CLOEXEC_DEFAULT`. This blocks make from accessing all
non-stdio file descriptors from the parent process, including those of
the jobserver.

Because make was getting passed jobserver information via MAKEFLAGS but
was unable to access the jobserver, it prints an error like the
following and proceeds to run a single-threaded build:

```
make: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
```

As a workaround, disable jobserver support on OSX so that cmake-rs
instead passes `--parallel $NUM_JOBS` to CMake, allowing for a faster
multi-threaded build. However, there is an exception made if the
available jobserver is based on a named pipe. Since access to this pipe
does not rely on the inheritance of file descriptors, the underlying
build will be able to access the jobserver in this case.
@kesyog
Copy link
Author

kesyog commented Dec 29, 2024

I updated the commit to add an exception that allows the jobserver on macOS iff the jobserver is based on a named pipe/fifo

Copy link

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thanks for adopting my advice!

Not a maintainer but I have another nit suggestion regarding fifo checking:

src/lib.rs Outdated Show resolved Hide resolved
This allows a fifo path to contain non-utf8 characters

Co-authored-by: Jiahao XU <[email protected]>
Copy link

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thanks for taking my advice!

I'm not a maintainer so you would need to get someone else to review & merge it

@kesyog
Copy link
Author

kesyog commented Dec 29, 2024

Thanks for the review @NobodyXu

@tgross35 any chance you could help get this merged and tag a new crate version?

@kesyog
Copy link
Author

kesyog commented Jan 6, 2025

@tgross35 any chance you could help get this merged and tag a new crate version?

Or perhaps @thomcc based on #127 (comment) and https://www.rust-lang.org/governance/teams/library? Hopefully not being too spammy here.

@tgross35
Copy link
Contributor

tgross35 commented Jan 6, 2025

Thom knows this crate better so probably a better reviewer, but I can merge this if we don't hear back in a few days.

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.

jobserver doesn't always work
3 participants