From cf9bb354b563c97c7f5ba823ca2a086e95cd0947 Mon Sep 17 00:00:00 2001 From: Alex Saveau Date: Thu, 25 Jan 2024 09:21:29 -0800 Subject: [PATCH] use const `thread_local`s when possible (#2838) This results in a substantial performance improvement, and is compatible with our MSRV. Signed-off-by: Alex Saveau Co-authored-by: Eliza Weisman --- .../examples/sloggish/sloggish_subscriber.rs | 2 +- tracing-core/src/dispatcher.rs | 10 +++-- .../src/filter/layer_filters/mod.rs | 44 ++++++++++++------- tracing-subscriber/src/registry/sharded.rs | 6 +-- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/examples/examples/sloggish/sloggish_subscriber.rs b/examples/examples/sloggish/sloggish_subscriber.rs index 4cbc1385d..fe42d3663 100644 --- a/examples/examples/sloggish/sloggish_subscriber.rs +++ b/examples/examples/sloggish/sloggish_subscriber.rs @@ -38,7 +38,7 @@ pub struct CurrentSpanPerThread { impl CurrentSpanPerThread { pub fn new() -> Self { thread_local! { - static CURRENT: RefCell> = const { RefCell::new(vec![]) }; + static CURRENT: RefCell> = const { RefCell::new(Vec::new()) }; }; Self { current: &CURRENT } } diff --git a/tracing-core/src/dispatcher.rs b/tracing-core/src/dispatcher.rs index deb56f734..fc9f29584 100644 --- a/tracing-core/src/dispatcher.rs +++ b/tracing-core/src/dispatcher.rs @@ -183,10 +183,12 @@ enum Kind { #[cfg(feature = "std")] thread_local! { - static CURRENT_STATE: State = const { State { - default: RefCell::new(None), - can_enter: Cell::new(true), - } }; + static CURRENT_STATE: State = const { + State { + default: RefCell::new(None), + can_enter: Cell::new(true), + } + }; } static EXISTS: AtomicBool = AtomicBool::new(false); diff --git a/tracing-subscriber/src/filter/layer_filters/mod.rs b/tracing-subscriber/src/filter/layer_filters/mod.rs index 5f26625ed..f349d4ce6 100644 --- a/tracing-subscriber/src/filter/layer_filters/mod.rs +++ b/tracing-subscriber/src/filter/layer_filters/mod.rs @@ -99,12 +99,18 @@ pub struct FilterId(u64); /// /// [`Registry`]: crate::Registry /// [`Filter`]: crate::layer::Filter -#[derive(Default, Copy, Clone, Eq, PartialEq)] +#[derive(Copy, Clone, Eq, PartialEq)] pub(crate) struct FilterMap { bits: u64, } -/// The current state of `enabled` calls to per-layer filters on this +impl FilterMap { + pub(crate) const fn new() -> Self { + Self { bits: 0 } + } +} + +/// The current state of `enabled` calls to per-subscriber filters on this /// thread. /// /// When `Filtered::enabled` is called, the filter will set the bit @@ -145,7 +151,7 @@ pub(crate) struct FilterState { /// Extra counters added to `FilterState` used only to make debug assertions. #[cfg(debug_assertions)] -#[derive(Debug, Default)] +#[derive(Debug)] struct DebugCounters { /// How many per-layer filters have participated in the current `enabled` /// call? @@ -156,8 +162,18 @@ struct DebugCounters { in_interest_pass: Cell, } +#[cfg(debug_assertions)] +impl DebugCounters { + const fn new() -> Self { + Self { + in_filter_pass: Cell::new(0), + in_interest_pass: Cell::new(0), + } + } +} + thread_local! { - pub(crate) static FILTERING: FilterState = FilterState::new(); + pub(crate) static FILTERING: FilterState = const { FilterState::new() }; } /// Extension trait adding [combinators] for combining [`Filter`]. @@ -1080,13 +1096,13 @@ impl fmt::Binary for FilterMap { // === impl FilterState === impl FilterState { - fn new() -> Self { + const fn new() -> Self { Self { - enabled: Cell::new(FilterMap::default()), + enabled: Cell::new(FilterMap::new()), interest: RefCell::new(None), #[cfg(debug_assertions)] - counters: DebugCounters::default(), + counters: DebugCounters::new(), } } @@ -1095,7 +1111,7 @@ impl FilterState { { let in_current_pass = self.counters.in_filter_pass.get(); if in_current_pass == 0 { - debug_assert_eq!(self.enabled.get(), FilterMap::default()); + debug_assert_eq!(self.enabled.get(), FilterMap::new()); } self.counters.in_filter_pass.set(in_current_pass + 1); debug_assert_eq!( @@ -1140,7 +1156,7 @@ impl FilterState { #[cfg(debug_assertions)] { if this.counters.in_filter_pass.get() == 0 { - debug_assert_eq!(this.enabled.get(), FilterMap::default()); + debug_assert_eq!(this.enabled.get(), FilterMap::new()); } // Nothing enabled this event, we won't tick back down the @@ -1177,7 +1193,7 @@ impl FilterState { { let in_current_pass = self.counters.in_filter_pass.get(); if in_current_pass <= 1 { - debug_assert_eq!(self.enabled.get(), FilterMap::default()); + debug_assert_eq!(self.enabled.get(), FilterMap::new()); } self.counters .in_filter_pass @@ -1207,7 +1223,7 @@ impl FilterState { // a panic and the thread-local has been torn down, that's fine, just // ignore it ratehr than panicking. let _ = FILTERING.try_with(|filtering| { - filtering.enabled.set(FilterMap::default()); + filtering.enabled.set(FilterMap::new()); #[cfg(debug_assertions)] filtering.counters.in_filter_pass.set(0); @@ -1232,10 +1248,8 @@ impl FilterState { pub(crate) fn filter_map(&self) -> FilterMap { let map = self.enabled.get(); #[cfg(debug_assertions)] - { - if self.counters.in_filter_pass.get() == 0 { - debug_assert_eq!(map, FilterMap::default()); - } + if self.counters.in_filter_pass.get() == 0 { + debug_assert_eq!(map, FilterMap::new()); } map diff --git a/tracing-subscriber/src/registry/sharded.rs b/tracing-subscriber/src/registry/sharded.rs index 17a3775ca..7d631b08e 100644 --- a/tracing-subscriber/src/registry/sharded.rs +++ b/tracing-subscriber/src/registry/sharded.rs @@ -255,7 +255,7 @@ impl Subscriber for Registry { data.filter_map = crate::filter::FILTERING.with(|filtering| filtering.filter_map()); #[cfg(debug_assertions)] { - if data.filter_map != FilterMap::default() { + if data.filter_map != FilterMap::new() { debug_assert!(self.has_per_layer_filters()); } } @@ -481,7 +481,7 @@ impl Default for DataInner { }; Self { - filter_map: FilterMap::default(), + filter_map: FilterMap::new(), metadata: &NULL_METADATA, parent: None, ref_count: AtomicUsize::new(0), @@ -526,7 +526,7 @@ impl Clear for DataInner { }) .clear(); - self.filter_map = FilterMap::default(); + self.filter_map = FilterMap::new(); } }