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

Replace num_cpus::get with std::thread::available_parallelism #500

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

rukai
Copy link
Contributor

@rukai rukai commented Jul 31, 2024

std::thread::available_parallelism was stabilized in rust 1.59, so there should be no problem in using it in this crate with an MSRV of 1.70

To remain consistent with the existing usage of num_cpus::get we default to a value of 1 if info on parallelism is inaccessible for whatever reason.
The old implementation did a std::cmp::max(1, ..) but that was not needed with num_cpus since it was documented to return a max of 1, and its not needed now since we handle an error by using 1.

Copy link
Member

@tobz tobz left a comment

Choose a reason for hiding this comment

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

Seems fine to me! 👍🏻

@tobz tobz added C-util Component: utility classes and helpers. T-chore Type: chore. S-awaiting-release Status: awaiting a release to be considered fixed/implemented. labels Aug 1, 2024
@tobz tobz merged commit 3b55c14 into metrics-rs:main Aug 1, 2024
12 checks passed
tobz pushed a commit that referenced this pull request Sep 21, 2024
Signed-off-by: Toby Lawrence <toby@nuclearfurnace.com>
@tobz
Copy link
Member

tobz commented Oct 12, 2024

Released as part of metrics-util@v0.18.0.

Thanks again for your contribution!

@tobz tobz removed the S-awaiting-release Status: awaiting a release to be considered fixed/implemented. label Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-util Component: utility classes and helpers. T-chore Type: chore.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants