Skip to content

Commit

Permalink
subscriber: update sharded-slab, pool hashmaps (tokio-rs#1064)
Browse files Browse the repository at this point in the history
This backports tokio-rs#1062 to v0.1.6. This has already been approved on
master.

hawkw/sharded-slab#45 changes `sharded-slab` so that the per-shard
metadata is allocated only when a new shard is created, rather than all
up front when the slab is created. This fixes the very large amount of
memory allocated by simply creating a new `Registry` without actually
collecting any traces.

This branch updates `tracing-subscriber` to depend on `sharded-slab`
0.1.0, which includes the upstream fix.

In addition, this branch the registry from using `sharded_slab::Slab` to
`sharded_slab::Pool`. This allows us to clear hashmap allocations for
extensions in-place, retaining the already allocated maps. This should
improve `new_span` performance a bit.

Fixes tokio-rs#1005

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw authored and kaffarell committed May 22, 2024
1 parent daae722 commit 2a07738
Showing 1 changed file with 84 additions and 0 deletions.
84 changes: 84 additions & 0 deletions tracing-subscriber/src/registry/sharded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,90 @@ impl Clear for DataInner {
}
}

// === impl DataInner ===

impl Default for DataInner {
fn default() -> Self {
// Since `DataInner` owns a `&'static Callsite` pointer, we need
// something to use as the initial default value for that callsite.
// Since we can't access a `DataInner` until it has had actual span data
// inserted into it, the null metadata will never actually be accessed.
struct NullCallsite;
impl tracing_core::callsite::Callsite for NullCallsite {
fn set_interest(&self, _: Interest) {
unreachable!(
"/!\\ Tried to register the null callsite /!\\\n \
This should never have happened and is definitely a bug. \
A `tracing` bug report would be appreciated."
)
}

fn metadata(&self) -> &Metadata<'_> {
unreachable!(
"/!\\ Tried to access the null callsite's metadata /!\\\n \
This should never have happened and is definitely a bug. \
A `tracing` bug report would be appreciated."
)
}
}

static NULL_CALLSITE: NullCallsite = NullCallsite;
static NULL_METADATA: Metadata<'static> = tracing_core::metadata! {
name: "",
target: "",
level: tracing_core::Level::TRACE,
fields: &[],
callsite: &NULL_CALLSITE,
kind: tracing_core::metadata::Kind::SPAN,
};

Self {
metadata: &NULL_METADATA,
parent: None,
ref_count: AtomicUsize::new(0),
extensions: RwLock::new(ExtensionsInner::new()),
}
}
}

impl Clear for DataInner {
/// Clears the span's data in place, dropping the parent's reference count.
fn clear(&mut self) {
// A span is not considered closed until all of its children have closed.
// Therefore, each span's `DataInner` holds a "reference" to the parent
// span, keeping the parent span open until all its children have closed.
// When we close a span, we must then decrement the parent's ref count
// (potentially, allowing it to close, if this child is the last reference
// to that span).
// We have to actually unpack the option inside the `get_default`
// closure, since it is a `FnMut`, but testing that there _is_ a value
// here lets us avoid the thread-local access if we don't need the
// dispatcher at all.
if self.parent.is_some() {
// Note that --- because `Layered::try_close` works by calling
// `try_close` on the inner subscriber and using the return value to
// determine whether to call the `Layer`'s `on_close` callback ---
// we must call `try_close` on the entire subscriber stack, rather
// than just on the registry. If the registry called `try_close` on
// itself directly, the layers wouldn't see the close notification.
let subscriber = dispatcher::get_default(Dispatch::clone);
if let Some(parent) = self.parent.take() {
let _ = subscriber.try_close(parent);
}
}

// Clear (but do not deallocate!) the pooled `HashMap` for the span's extensions.
self.extensions
.get_mut()
.unwrap_or_else(|l| {
// This function can be called in a `Drop` impl, such as while
// panicking, so ignore lock poisoning.
l.into_inner()
})
.clear();
}
}

#[cfg(test)]
mod tests {
use super::Registry;
Expand Down

0 comments on commit 2a07738

Please sign in to comment.