From aac2f734dec39a19e412b46fcdc0e67a6eafa3ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Wed, 1 Jul 2020 16:59:50 +0200 Subject: [PATCH 1/2] Improve comments from https://github.com/rust-lang/rust/pull/72617, as suggested by RalfJung. --- src/libstd/panicking.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 97d62d958ca4f..3a81b1f9a0531 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -229,10 +229,10 @@ pub mod panic_count { 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. + // a fast path in `is_zero` (which is used by `panicking`). In any particular + // thread, if that thread currently views `GLOBAL_PANIC_COUNT` as being zero, + // then `LOCAL_PANIC_COUNT` in that thread is zero. This invariant holds before + // and after increase and decrease, but not necessarily during their execution. static GLOBAL_PANIC_COUNT: AtomicUsize = AtomicUsize::new(0); pub fn increase() -> usize { @@ -263,6 +263,11 @@ pub mod panic_count { // 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. + // + // A relaxed atomic load is equivalent to a normal aligned memory read + // (e.g., a `mov` instruction in x86), while a TLS access might require + // calling a non-inlinable function (such as `__tls_get_addr` when using + // the GD TLS model). true } else { is_zero_slow_path() From 0f1adc8ec812df494df33640c1be147f35e5f6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Thu, 2 Jul 2020 13:47:19 +0200 Subject: [PATCH 2/2] Further improve comments in libstd/panicking.rs. --- src/libstd/panicking.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 3a81b1f9a0531..9542e7209b4cf 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -264,10 +264,11 @@ pub mod panic_count { // (including the current one) will have `LOCAL_PANIC_COUNT` // equal to zero, so TLS access can be avoided. // - // A relaxed atomic load is equivalent to a normal aligned memory read - // (e.g., a `mov` instruction in x86), while a TLS access might require - // calling a non-inlinable function (such as `__tls_get_addr` when using - // the GD TLS model). + // In terms of performance, a relaxed atomic load is similar to a normal + // aligned memory read (e.g., a mov instruction in x86), but with some + // compiler optimization restrictions. On the other hand, a TLS access + // might require calling a non-inlinable function (such as `__tls_get_addr` + // when using the GD TLS model). true } else { is_zero_slow_path()