Pass -jN from Make to BOOTSTRAP_ARGS

[onur-ozkan]

Enables the same functionality as x -jN in Make by passing the -jN arg from Make to the BOOTSTRAP_ARGS if it is specified.

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[jefferyto]

Sorry but this doesn't fix #116101, see #116101 (comment).

[onur-ozkan]

Yes, this is a workaround, not a direct fix. Currently, we remove MAKEFLAGS from Cargo to prevent numerous problems (see this commit for reference). So, when we execute make -j5 in the rust/Makefile, cargo utilizes all the available resources ignoring the given arg. This PR changes that by setting the job limit equal to the argument given with make.

[jefferyto]

It seems to me that 209240d and a22af69 were workarounds to the real issue(s) (cargo being unable to read the jobserver file descriptor), and that a) it would be good to fix the actual issues; and b) perhaps things have changed in the 5 years since those commits that make fixing those issues possible (if the actual issues still exist).

[onur-ozkan]

To ensure that the removal of those lines doesn't cause any issues on various systems, we need to be thorough in testing. We've experienced a number of problems related to jobserver panics, so it's important to proceed cautiously. Implementing this change will take some time. This shouldn't be a blocker for this PR anyway.

And

I have tried patching out these lines that remove MAKEFLAGS from the environment for cargo; the MAKEFLAGS value does reach cargo but cargo continues to set up its own jobserver instead.

(Copied from #116101 (comment))

I am not sure how removing them on upstream will provide a solution(since your patch is already doing it).

Since there's currently no need to do it, we can wait until issue #116101 is fixed. Once that is resolved, we can consider removing those lines from the bootstrap.

[Mark-Simulacrum]

What is the current behavior? -j1 parallelism, or # of CPUs?

I'm broadly happy to approve this as-is, I'd also be happy to sign off on removing the env_remove calls if we had some basic tests confirming that works correctly with Cargo.

[onur-ozkan]

What is the current behavior? -j1 parallelism, or # of CPUs?

Currently, bootstrap always utilizes the maximum available parallelism on cargo. With this PR parallelism is now limited to the value specified with -j(we basically pass the -j flag from the make command to cargo).

I'd also be happy to sign off on removing the env_remove calls if we had some basic tests confirming that works correctly with Cargo.

It's in the ToDo plans.

[Mark-Simulacrum]

@bors r+

I'm ok landing this in that case. I think limiting to the top-level specified parallelism is probably slightly better than ignoring, even if the ideal behavior would be to use the Make-provided jobserver.

[bors]

📌 Commit 863d2fd has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bors]

⌛ Testing commit 863d2fd with merge b11431e...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing b11431e to master...

[rust-timer]

Finished benchmarking commit (b11431e): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-2.6%, -0.8%]	2
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 630.889s -> 631.12s (0.04%)
Artifact size: 317.38 MiB -> 317.38 MiB (0.00%)