Skip to content

Commit

Permalink
use const thread_locals when possible (#2838)
Browse files Browse the repository at this point in the history
This results in a substantial performance improvement,
and is compatible with our MSRV.

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
2 people authored and hds committed Nov 21, 2024
1 parent c4aed1d commit ce5b644
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 23 deletions.
2 changes: 1 addition & 1 deletion examples/examples/sloggish/sloggish_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub struct CurrentSpanPerThread {
impl CurrentSpanPerThread {
pub fn new() -> Self {
thread_local! {
static CURRENT: RefCell<Vec<Id>> = const { RefCell::new(vec![]) };
static CURRENT: RefCell<Vec<Id>> = const { RefCell::new(Vec::new()) };
};
Self { current: &CURRENT }
}
Expand Down
10 changes: 6 additions & 4 deletions tracing-core/src/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,12 @@ enum Kind<T> {

#[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);
Expand Down
44 changes: 29 additions & 15 deletions tracing-subscriber/src/filter/layer_filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand All @@ -156,8 +162,18 @@ struct DebugCounters {
in_interest_pass: Cell<usize>,
}

#[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`].
Expand Down Expand Up @@ -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(),
}
}

Expand All @@ -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!(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions tracing-subscriber/src/registry/sharded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -526,7 +526,7 @@ impl Clear for DataInner {
})
.clear();

self.filter_map = FilterMap::default();
self.filter_map = FilterMap::new();
}
}

Expand Down

0 comments on commit ce5b644

Please sign in to comment.