[beta] Revert #10427: switch from num_cpus #10737
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ehuss: no appropriate reviewer found, use r? to override |
|
rust-highfive
added
the
S-waiting-on-review
|
From a release perspective, I'd encourage us to revert on nightly/master too now, rather than hoping to remember to reapply this in case it's not changed in std itself. (Particularly given that we don't (?) have tests in Cargo for this?) |
|
Thanks for the thorough investigation on different platforms. @bors r+ |
|
📌 Commit 0cfdbc0 has been approved by |
bors
added
S-waiting-on-bors
Just opened #10739 |
|
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 9, 2022
Revert "remove num_cpus dependency" in rustc and update cargo Fixes rust-lang#97549. This PR reverts rust-lang#94524 and does a Cargo update to pull in rust-lang/cargo#10737. Rust 1.61.0 has a regression in which it misidentifies the number of available CPUs in some environments, leading to enormously increased memory usage and failing builds. In between Rust 1.60 and 1.61 both rustc and cargo replaced some uses of `num_cpus` with `available_parallelism`, which eliminated support for cgroupv1, still apparently in common use. This PR switches both rustc and cargo back to using `num_cpus` in order to support environments where the available parallelism is controlled by cgroupv1. Both can use `available_parallism` again once it handles cgroupv1 (if ever). I have confirmed that the rustc part of this PR fixes the memory usage regression in my non-Cargo environment, and others have confirmed in rust-lang#97549 that the Cargo regression was at fault for the memory usage regression in their environments.
ehuss
pushed a commit
to ehuss/rust
that referenced
this pull request
Jun 24, 2022
Revert "remove num_cpus dependency" in rustc and update cargo Fixes rust-lang#97549. This PR reverts rust-lang#94524 and does a Cargo update to pull in rust-lang/cargo#10737. Rust 1.61.0 has a regression in which it misidentifies the number of available CPUs in some environments, leading to enormously increased memory usage and failing builds. In between Rust 1.60 and 1.61 both rustc and cargo replaced some uses of `num_cpus` with `available_parallelism`, which eliminated support for cgroupv1, still apparently in common use. This PR switches both rustc and cargo back to using `num_cpus` in order to support environments where the available parallelism is controlled by cgroupv1. Both can use `available_parallism` again once it handles cgroupv1 (if ever). I have confirmed that the rustc part of this PR fixes the memory usage regression in my non-Cargo environment, and others have confirmed in rust-lang#97549 that the Cargo regression was at fault for the memory usage regression in their environments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This temporarily reverts #10427 (Use available_parallelism instead of num_cpus) per the discussion at rust-lang/rust#97549.
available_parallelismdoes not handle cgroups v1 on Linux unlike num_cpus. I am concerned that this potentially affects a significant percentage of users. For example, Docker just added cgroups v2 support last year. Various Linux distributions have only recently switched to it as the default. The following is what I can find on the web:This also appears to affect CircleCI.
The consequence is that Cargo ends up using too much parallelism and can run out of memory.
I'm not sure what to do about 1.63. If std adds support for cgroups v1, then I don't think there is anything to do there. Otherwise I think we should revert similarly if that doesn't happen.