From f03cf9916a1932f47b788a9de9256269cc172fe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Tue, 26 May 2020 17:38:22 +0200 Subject: [PATCH 1/2] Add a fast path for `std::thread::panicking`. This is done by adding a global atomic variable (non-TLS) that counts how many threads are panicking. In order to check if the current thread is panicking, this variable is read and, if it is zero, no thread (including the one where `panicking` is being called) is panicking and `panicking` can return `false` immediately without needing to access TLS. If the global counter is not zero, the local counter is accessed from TLS to check if the current thread is panicking. --- src/libstd/panicking.rs | 63 ++++++++++++++++++++++++++++++++--------- src/libstd/rt.rs | 2 +- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index bf381896a22bb..46196960e718b 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -170,7 +170,7 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { fn default_hook(info: &PanicInfo<'_>) { // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - let backtrace_env = if update_panic_count(0) >= 2 { + let backtrace_env = if panic_count::get() >= 2 { RustBacktrace::Print(backtrace_rs::PrintFmt::Full) } else { backtrace::rust_backtrace_env() @@ -222,19 +222,56 @@ fn default_hook(info: &PanicInfo<'_>) { #[cfg(not(test))] #[doc(hidden)] #[unstable(feature = "update_panic_count", issue = "none")] -pub fn update_panic_count(amt: isize) -> usize { +pub mod panic_count { use crate::cell::Cell; - thread_local! { static PANIC_COUNT: Cell = Cell::new(0) } + use crate::sync::atomic::{AtomicUsize, Ordering}; + + // Panic count for the current thread. + thread_local! { static LOCAL_PANIC_COUNT: Cell = Cell::new(0) } + + // Sum of panic counts from all threads. The purpose of this is to have + // a fast path in `is_zero` (which is used by `panicking`). Access to + // this variable can be always be done with relaxed ordering because + // it is always guaranteed that, if `GLOBAL_PANIC_COUNT` is zero, + // `LOCAL_PANIC_COUNT` will be zero. + static GLOBAL_PANIC_COUNT: AtomicUsize = AtomicUsize::new(0); + + pub fn increase() -> usize { + GLOBAL_PANIC_COUNT.fetch_add(1, Ordering::Relaxed); + LOCAL_PANIC_COUNT.with(|c| { + let next = c.get() + 1; + c.set(next); + next + }) + } + + pub fn decrease() -> usize { + GLOBAL_PANIC_COUNT.fetch_sub(1, Ordering::Relaxed); + LOCAL_PANIC_COUNT.with(|c| { + let next = c.get() - 1; + c.set(next); + next + }) + } - PANIC_COUNT.with(|c| { - let next = (c.get() as isize + amt) as usize; - c.set(next); - next - }) + pub fn get() -> usize { + LOCAL_PANIC_COUNT.with(|c| c.get()) + } + + pub fn is_zero() -> bool { + if GLOBAL_PANIC_COUNT.load(Ordering::Relaxed) == 0 { + // Fast path: if `GLOBAL_PANIC_COUNT` is zero, all threads + // (including the current one) will have `LOCAL_PANIC_COUNT` + // equal to zero, so TLS access can be avoided. + true + } else { + LOCAL_PANIC_COUNT.with(|c| c.get() == 0) + } + } } #[cfg(test)] -pub use realstd::rt::update_panic_count; +pub use realstd::rt::panic_count; /// Invoke a closure, capturing the cause of an unwinding panic if one occurs. pub unsafe fn r#try R>(f: F) -> Result> { @@ -284,7 +321,7 @@ pub unsafe fn r#try R>(f: F) -> Result> #[cold] unsafe fn cleanup(payload: *mut u8) -> Box { let obj = Box::from_raw(__rust_panic_cleanup(payload)); - update_panic_count(-1); + panic_count::decrease(); obj } @@ -314,7 +351,7 @@ pub unsafe fn r#try R>(f: F) -> Result> /// Determines whether the current thread is unwinding because of panic. pub fn panicking() -> bool { - update_panic_count(0) != 0 + !panic_count::is_zero() } /// The entry point for panicking with a formatted message. @@ -452,7 +489,7 @@ fn rust_panic_with_hook( message: Option<&fmt::Arguments<'_>>, location: &Location<'_>, ) -> ! { - let panics = update_panic_count(1); + let panics = panic_count::increase(); // If this is the third nested call (e.g., panics == 2, this is 0-indexed), // the panic hook probably triggered the last panic, otherwise the @@ -514,7 +551,7 @@ fn rust_panic_with_hook( /// This is the entry point for `resume_unwind`. /// It just forwards the payload to the panic runtime. pub fn rust_panic_without_hook(payload: Box) -> ! { - update_panic_count(1); + panic_count::increase(); struct RewrapBox(Box); diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index 2426b2dead712..fb825ab16ebd7 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -15,7 +15,7 @@ #![doc(hidden)] // Re-export some of our utilities which are expected by other crates. -pub use crate::panicking::{begin_panic, begin_panic_fmt, update_panic_count}; +pub use crate::panicking::{begin_panic, begin_panic_fmt, panic_count}; // To reduce the generated code of the new `lang_start`, this function is doing // the real work. From 771a1d8e0ad4e6702aaab91cb5e58adbf3ec3708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Wed, 24 Jun 2020 18:17:27 +0200 Subject: [PATCH 2/2] Make `std::panicking::panic_count::is_zero` inline and move the slow path into a separate cold function. --- src/libstd/panicking.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 46196960e718b..9a5d5e2b5ae9f 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -258,6 +258,7 @@ pub mod panic_count { LOCAL_PANIC_COUNT.with(|c| c.get()) } + #[inline] pub fn is_zero() -> bool { if GLOBAL_PANIC_COUNT.load(Ordering::Relaxed) == 0 { // Fast path: if `GLOBAL_PANIC_COUNT` is zero, all threads @@ -265,9 +266,17 @@ pub mod panic_count { // equal to zero, so TLS access can be avoided. true } else { - LOCAL_PANIC_COUNT.with(|c| c.get() == 0) + is_zero_slow_path() } } + + // Slow path is in a separate function to reduce the amount of code + // inlined from `is_zero`. + #[inline(never)] + #[cold] + fn is_zero_slow_path() -> bool { + LOCAL_PANIC_COUNT.with(|c| c.get() == 0) + } } #[cfg(test)] @@ -350,6 +359,7 @@ pub unsafe fn r#try R>(f: F) -> Result> } /// Determines whether the current thread is unwinding because of panic. +#[inline] pub fn panicking() -> bool { !panic_count::is_zero() }