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

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s… #94524

Merged
merged 1 commit into from
Mar 4, 2022

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 2, 2022

…ession

std::threads::available_parallelism was stabilized in rust 1.59.

r? @Mark-Simulacrum

@bjorn3 bjorn3 added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Mar 2, 2022
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 2, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 2, 2022
@@ -551,7 +551,7 @@ mod parse {
crate fn parse_threads(slot: &mut usize, v: Option<&str>) -> bool {
match v.and_then(|s| s.parse().ok()) {
Some(0) => {
*slot = ::num_cpus::get();
*slot = std::thread::available_parallelism().map_or(1, std::num::NonZeroUsize::get);
Copy link
Member Author

Choose a reason for hiding this comment

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

num_cpus defaults to returning 1 in case of errors.

@rust-log-analyzer

This comment has been minimized.

@bjorn3 bjorn3 force-pushed the remove_num_cpus branch from 6619acc to 2d854f9 Compare March 2, 2022 14:39
@Mark-Simulacrum
Copy link
Member

@bors r+

I think the differences between num_cpus and available_parallelism around cgroups at least likely don't matter for our use case, and this seems good beyond that.

@bors
Copy link
Contributor

bors commented Mar 2, 2022

📌 Commit 2d854f9 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? `@Mark-Simulacrum`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ``@Mark-Simulacrum``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ```@Mark-Simulacrum```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ````@Mark-Simulacrum````
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 3, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? `````@Mark-Simulacrum`````
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 3, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ````````@Mark-Simulacrum````````
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ``````````````@Mark-Simulacrum``````````````
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ```````````````@Mark-Simulacrum```````````````
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ````````````````@Mark-Simulacrum````````````````
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? `````````````````@Mark-Simulacrum`````````````````
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? `````````````````````````@Mark-Simulacrum`````````````````````````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2022
…lacrum

Remove num_cpus dependency from bootstrap, build-manifest and rustc_s…

…ession

`std::threads::available_parallelism` was stabilized in rust 1.59.

r? ``````````````````````````@Mark-Simulacrum``````````````````````````
@bors bors merged commit 5115bdc into rust-lang:master Mar 4, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 4, 2022
@bjorn3 bjorn3 deleted the remove_num_cpus branch March 4, 2022 20:36
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.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 15, 2022
…=jyn514

Reland changes replacing num_cpus with available_parallelism

Since rust-lang#97925 added cgroupv1 support the problem in rust-lang#97549 which lead to the previous revert should be addressed now.

Cargo has reapplied the replacement too rust-lang/cargo#10969

Reverts 1ae4b25 (part of rust-lang#97911)
Relands rust-lang#94524
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 15, 2022
…=jyn514

Reland changes replacing num_cpus with available_parallelism

Since rust-lang#97925 added cgroupv1 support the problem in rust-lang#97549 which lead to the previous revert should be addressed now.

Cargo has reapplied the replacement too rust-lang/cargo#10969

Reverts 1ae4b25 (part of rust-lang#97911)
Relands rust-lang#94524
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants