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

Do not pass -jN to make, use cargo jobserver instead #92

Open
tmpolaczyk opened this issue Jul 5, 2024 · 3 comments
Open

Do not pass -jN to make, use cargo jobserver instead #92

tmpolaczyk opened this issue Jul 5, 2024 · 3 comments

Comments

@tmpolaczyk
Copy link

When compiling a crate that has jemallocator-sys as a dependency with cargo build -j4, I see a short spike where the number of cpu cores used is 8 instead of 4. The cause is this crate using the NUM_JOBS env var to call make, which ignores that cargo already has other running jobs, thus bringing the total number of running jobs to be higher than NUM_JOBS. This doesn't happen in any of the other *-sys crates, I guess because they use the cc crate with the parallel feature enabled instead of manually calling make.

The fix is to remove the -jN arg to make if the CARGO_MAKEFLAGS env var exists, and set MAKEFLAGS to CARGO_MAKEFLAGS instead.

I expect the implementation to be similar to this:

https://github.com/rust-lang/cmake-rs/blob/c4a60dd154dd90e469dffc41a1faa717704f90b3/src/lib.rs#L830

I guess using a crate to call make instead of doing it manually would handle this and other issues automatically, but I don't know what's the standard so I can't recommend any crates.

@BusyJay
Copy link
Member

BusyJay commented Jul 5, 2024

The fix is to remove the -jN arg to make if the CARGO_MAKEFLAGS env var exists, and set MAKEFLAGS to CARGO_MAKEFLAGS instead.

Make sense. Would you like to send a PR?

@tmpolaczyk
Copy link
Author

I tried to run cargo build but it gave me a strange error "configure: error: cannot find sources (Makefile.in)", and I didn't find any documentation on what should I do to fix it, so I will let some other contributor implement it.

@BusyJay
Copy link
Member

BusyJay commented Jul 5, 2024

Likely the same problem as #32.

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

No branches or pull requests

2 participants