-
Notifications
You must be signed in to change notification settings - Fork 220
[runtime/tokio] support for core pinned dedicated tasks #3549
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
base: main
Are you sure you want to change the base?
Changes from 12 commits
1b43392
fd20460
8c076c1
df22aa9
cc2321b
53bff09
f00c82e
d3877f5
595bbcc
a3b5f97
a0d9699
cd17564
e6077ad
ab37233
bb4238b
cc4a3b7
017c4c7
64ffa2d
eb5ac73
2ee66b0
1d33fe6
d6bdfdf
6ef8a13
380231c
61be589
aff0464
d478b2a
2885b19
6a9261e
72bc5ea
201683d
412967a
8df7099
87f22f0
34c0cdc
2e18622
1fb2e83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,9 @@ | ||
| //! Helpers for resolving the configured thread stack size. | ||
|
|
||
| #[cfg(target_os = "linux")] | ||
| use commonware_utils::sync::Once; | ||
| use std::{env, sync::OnceLock, thread}; | ||
|
|
||
| /// Cached configured thread stack size. | ||
| static SYSTEM_THREAD_STACK_SIZE: OnceLock<usize> = OnceLock::new(); | ||
|
|
||
| /// Rust's default thread stack size. | ||
| /// | ||
| /// See <https://doc.rust-lang.org/std/thread/#stack-size>. | ||
|
|
@@ -32,7 +31,10 @@ fn rust_min_stack() -> Option<usize> { | |
| /// | ||
| /// On other platforms, or if the platform-specific query fails, this falls back | ||
| /// to [RUST_DEFAULT_THREAD_STACK_SIZE]. | ||
| /// | ||
| /// The result is cached after the first call. | ||
| pub(crate) fn system_thread_stack_size() -> usize { | ||
| static SYSTEM_THREAD_STACK_SIZE: OnceLock<usize> = OnceLock::new(); | ||
| *SYSTEM_THREAD_STACK_SIZE.get_or_init(|| { | ||
| rust_min_stack() | ||
| .or(system_thread_stack_size_impl()) | ||
|
|
@@ -111,3 +113,94 @@ where | |
| .spawn(f) | ||
| .expect("failed to spawn thread") | ||
| } | ||
|
|
||
| /// Returns the number of available CPUs, or `None` if it cannot be determined. | ||
| /// | ||
| /// The result is cached after the first call. | ||
| #[cfg(unix)] | ||
| pub fn available_cores() -> Option<usize> { | ||
| static CORES: OnceLock<Option<usize>> = OnceLock::new(); | ||
| *CORES.get_or_init(|| { | ||
| // SAFETY: `sysconf(_SC_NPROCESSORS_ONLN)` is a read-only query with no | ||
| // preconditions. | ||
| let n = unsafe { libc::sysconf(libc::_SC_NPROCESSORS_ONLN) }; | ||
| if n <= 0 { | ||
| None | ||
| } else { | ||
| Some(n as usize) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /// Returns the number of available CPUs, or `None` if it cannot be determined. | ||
| /// | ||
| /// Always returns `None` on non-Unix platforms. | ||
| #[cfg(not(unix))] | ||
| pub const fn available_cores() -> Option<usize> { | ||
| None | ||
| } | ||
|
|
||
| /// Pins the current thread to the given core. | ||
| /// | ||
| /// If the CPU count cannot be queried or `sched_setaffinity` fails, a warning | ||
| /// is logged once and the thread continues unpinned. | ||
| /// | ||
| /// # Panics | ||
| /// | ||
| /// Panics if `core` is greater than or equal to the number of available CPUs. | ||
| #[cfg(target_os = "linux")] | ||
| pub(crate) fn pin_to_core(core: usize) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are designing your application with core pinning in mind, I wonder if there is some credence to just panicking if core pining fails? For example, I suspect glommio just dies if it can't pin?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in a lot of cases core pinning is mostly a performance optimization but not necessarily something that MUST happen, i.e. a best-effort attempt, since it doesn't incur any correctness issue if it doesn't happen. Maybe here we can just return a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My concern (maybe too pedantic) is that correct/smooth behavior does actually rely on pinning in the applications that want it (or else there is such a big performance hit it doesn't work well). 🤔
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the behavior could be controlled by a runtime configuration option? IMO it should default to "best-effort", but can be set to strict in which case we panic if pinning fails. |
||
| static WARN_CPUS: Once = Once::new(); | ||
| static WARN_AFFINITY: Once = Once::new(); | ||
|
|
||
| let Some(num_cores) = available_cores() else { | ||
| WARN_CPUS.call_once(|| { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the idea that warning each time would become very noisy/preference would just be to continue?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, if this fails once, it will very likely just keep failing forever (i.e. the syscall just isn't allowed for some reason). I can remove and just warn every time though. |
||
| tracing::warn!("failed to query CPU count, skipping core pinning"); | ||
| }); | ||
| return; | ||
| }; | ||
| assert!( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to assert in two locations?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't assert in |
||
| core < num_cores, | ||
| "core {core} out of range ({num_cores} available)" | ||
| ); | ||
|
|
||
| // SAFETY: `cpu_set` is zeroed and then a single valid CPU index is set. | ||
| unsafe { | ||
| let mut cpu_set: libc::cpu_set_t = std::mem::zeroed(); | ||
| libc::CPU_SET(core, &mut cpu_set); | ||
| let result = libc::sched_setaffinity( | ||
| 0, // current thread | ||
| std::mem::size_of::<libc::cpu_set_t>(), | ||
| &cpu_set, | ||
| ); | ||
| if result != 0 { | ||
| WARN_AFFINITY.call_once(|| { | ||
| tracing::warn!(core, "sched_setaffinity failed, skipping core pinning"); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Pins the current thread to the given core. | ||
| /// | ||
| /// No-op on non-Linux platforms. | ||
| #[cfg(not(target_os = "linux"))] | ||
| pub(crate) const fn pin_to_core(_core: usize) {} | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[cfg(unix)] | ||
| #[test] | ||
| fn test_available_cores() { | ||
| let n = available_cores().expect("available_cores returned None on Unix"); | ||
| assert!(n >= 1, "expected at least 1 core, got {n}"); | ||
| } | ||
|
|
||
| #[cfg(not(unix))] | ||
| #[test] | ||
| fn test_available_cores_non_unix() { | ||
| assert!(available_cores().is_none()); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.