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
SUPERCILEX and hawkw authored Jan 25, 2024
1 parent b461897 commit baeba47
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 20 deletions.
2 changes: 1 addition & 1 deletion examples/examples/sloggish/sloggish_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub struct CurrentSpanPerThread {
impl CurrentSpanPerThread {
pub fn new() -> Self {
thread_local! {
static CURRENT: RefCell<Vec<Id>> = RefCell::new(vec![]);
static CURRENT: RefCell<Vec<Id>> = const { RefCell::new(Vec::new()) };
};
Self { current: &CURRENT }
}
Expand Down
8 changes: 5 additions & 3 deletions tracing-core/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,11 @@ enum Kind<T> {

#[cfg(feature = "std")]
thread_local! {
static CURRENT_STATE: State = 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),
}
};
}

Expand Down
38 changes: 27 additions & 11 deletions tracing-subscriber/src/filter/subscriber_filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,17 @@ pub struct FilterId(u64);
///
/// [`Registry`]: crate::Registry
/// [`Filter`]: crate::subscribe::Filter
#[derive(Default, Copy, Clone, Eq, PartialEq)]
#[derive(Copy, Clone, Eq, PartialEq)]
pub(crate) struct FilterMap {
bits: u64,
}

impl FilterMap {
pub(crate) const fn new() -> Self {
Self { bits: 0 }
}
}

/// The current state of `enabled` calls to per-subscriber filters on this
/// thread.
///
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-subscriber 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 @@ -1072,13 +1088,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 @@ -1087,7 +1103,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 @@ -1132,7 +1148,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 @@ -1169,7 +1185,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 @@ -1199,7 +1215,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 @@ -1225,7 +1241,7 @@ impl FilterState {
let map = self.enabled.get();
#[cfg(debug_assertions)]
if self.counters.in_filter_pass.get() == 0 {
debug_assert_eq!(map, FilterMap::default());
debug_assert_eq!(map, FilterMap::new());
}

map
Expand Down
2 changes: 1 addition & 1 deletion tracing-subscriber/src/fmt/fmt_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ where

fn on_event(&self, event: &Event<'_>, ctx: Context<'_, C>) {
thread_local! {
static BUF: RefCell<String> = RefCell::new(String::new());
static BUF: RefCell<String> = const { RefCell::new(String::new()) };
}

BUF.with(|buf| {
Expand Down
8 changes: 4 additions & 4 deletions tracing-subscriber/src/registry/sharded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ thread_local! {
/// track how many subscribers have processed the close.
/// For additional details, see [`CloseGuard`].
///
static CLOSE_COUNT: Cell<usize> = Cell::new(0);
static CLOSE_COUNT: Cell<usize> = const { Cell::new(0) };
}

impl Collect for Registry {
Expand Down Expand Up @@ -255,7 +255,7 @@ impl Collect 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_subscriber_filters());
}
}
Expand Down Expand Up @@ -477,7 +477,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 @@ -522,7 +522,7 @@ impl Clear for DataInner {
})
.clear();

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

Expand Down

0 comments on commit baeba47

Please sign in to comment.