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

Use std::thread::available_parallelism() instead of num_cpus #6211

Closed
c410-f3r opened this issue Dec 11, 2023 · 8 comments
Closed

Use std::thread::available_parallelism() instead of num_cpus #6211

c410-f3r opened this issue Dec 11, 2023 · 8 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-runtime Module: tokio/runtime S-blocked-on-msrv Status: need an MSRV bump to progress

Comments

@c410-f3r
Copy link
Contributor

c410-f3r commented Dec 11, 2023

Requires a to bump 1.64 but it will allow the decrease of the number of transient dependencies for downstream crates.

1.63 only makes use of cgroupv2 and such exclusive behavior led to issues like rust-lang/rust#97549. In 1.64, cgroupv1 is also supported (rust-lang/rust#97925, https://releases.rs/docs/1.64.0/).

cc #6210

@c410-f3r c410-f3r added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Dec 11, 2023
@c410-f3r c410-f3r changed the title Replace Use std::thread::available_parallelism() instead of num_cpus Dec 11, 2023
@c410-f3r c410-f3r mentioned this issue Dec 11, 2023
@Darksonn Darksonn added the M-runtime Module: tokio/runtime label Dec 11, 2023
@seanmonstar
Copy link
Member

Worth noting that available_parallelism() is worse for users on older rustc. An additional point that I don't have the answer to: does it support all the targets that num_cpus does?

@carllerche
Copy link
Member

@c410-f3r are you able to address @seanmonstar's points re: num_cpus. I know that that num_cpus has had updates over the years to address edge cases that are not initially obvious (e.g. reducing available CPUs to a process via cgroups)

@c410-f3r
Copy link
Contributor Author

c410-f3r commented Dec 15, 2023

!!! This report is a best effort subject to possible errors !!!

Targets found in the rustc repository.

  • Unsupported: itron, sgx, teeos, wasi

  • Supported: aix, android, dragonfly, emscripten, freebsd, fuchsia, haiku, hermit, hurd, illumos, ios, linux, macos, solaris, tvos, netbsd, nto, openbsd, windows, xous

Targets founds in the num_cpus repository:

  • Unsupported: None, everything that is not supported has a fixed 1 value.
    Supported: aix, android, dragonfly, freebsd, fuchsia, haiku, hermit, illumos, ios, linux, macos, nacl, netbsd, openbsd, solaris, windows

In regards to implementation, they differ in some points. For example, aarch64 uses libc::_SC_NPROCESSORS_CONF in num_cpus (https://github.com/seanmonstar/num_cpus/blob/7c03fc930cc47a9b94e8ca66ca44ef1a454c8f51/src/lib.rs#L368) but rustc uses libc::_SC_NPROCESSORS_ONLN in non-android platforms (https://github.com/rust-lang/rust/blob/d253bf61ad38a59cc579aee688f81a06c31283d3/library/std/src/sys/unix/thread.rs#L347).

That said, it seems that the current Rustc version supports everything but nacl.

cc https://github.com/tokio-rs/mio/blob/c0919cc4cee75904904c2042f5066726b2dc9397/Makefile#L2

@carllerche
Copy link
Member

@seanmonstar You have thoughts? We would need to wait until our MSRV includes the method, but besides that?

@c410-f3r
Copy link
Contributor Author

Closing due to inactivity.

@Darksonn
Copy link
Contributor

There's no need to close the issue. Even if we can't upgrade right now, we will be able to in the future when we bump the MSRV.

@Darksonn Darksonn reopened this Jan 15, 2024
@maminrayej maminrayej added the S-blocked-on-msrv Status: need an MSRV bump to progress label Mar 17, 2024
@c410-f3r
Copy link
Contributor Author

Closing due to inactivity.

If desirable, please create another issue to track this feature.

@Darksonn
Copy link
Contributor

I'm guessing you don't want long-term open issues under your account? I've opened a different issue #6432 to track it. We want to make sure that we do not forget to make this change when we bump our MSRV.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-runtime Module: tokio/runtime S-blocked-on-msrv Status: need an MSRV bump to progress
Projects
None yet
Development

No branches or pull requests

5 participants