Skip to content

tests: use std::thread::available_parallelism() instead of num_cpus to get thread count #8491

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

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

matthiaskrgr
Copy link
Member

removes the dependency added in #8451

Please write a short comment explaining your change (or "none" for internal only changes)

changelog: tests: use std::thread::available_parallelism() instead of num_cpus to get thread count

@rust-highfive
Copy link

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 1, 2022
@matthiaskrgr
Copy link
Member Author

Note that this also returns thread count and not physical core count on my system (as did the previous implementation using num_cpus)

Comment on lines 172 to 175
match std::thread::available_parallelism() {
Ok(threads) => threads.get(),
_ => 1,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't that be the same as

Suggested change
match std::thread::available_parallelism() {
Ok(threads) => threads.get(),
_ => 1,
}
std::thread::available_parallelism().map_or(1, |t| t.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.

To map_or not to map, that is the question!

Yeah, should be identical, thanks :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Also apparently we don't have a lint for that yet :P

Copy link
Member Author

Choose a reason for hiding this comment

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

@flip1995
Copy link
Member

flip1995 commented Mar 2, 2022

I really like what GitHub did to the formatting of the PR title 😄

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Mar 2, 2022

📌 Commit 7a15061 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Mar 2, 2022

⌛ Testing commit 7a15061 with merge 1a25d19...

@bors
Copy link
Contributor

bors commented Mar 2, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 1a25d19 to master...

@bors bors merged commit 1a25d19 into rust-lang:master Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants