Skip to content

Commit

Permalink
core: add a limited form of the linked-list callsite registry for v0.…
Browse files Browse the repository at this point in the history
…1.x (#2083)

## Motivation

Currently on `v0.1.x`, the global callsite registry is implemented as a
`Mutex<Vec<&'static dyn Callsite>>`. This has a few downsides:

* Every time a callsite is registered, the `Mutex` must be locked. This
  can cause a deadlock when a `register_callsite` implementation calls
  into code that _also_ registers a callsite. See #2020 for details.

* Registering callsites is relatively slow and forces the program to
  synchronize on the callsite registry (and not on *individual*
  callsites). This means that if two threads are both registering
  totally different callsites, both threads must wait for the lock.
  Although this overhead is amortized over the entire rest of the
  program, it does have an impact in short-running applications where
  any given callsite may only be hit once or twice.

* Memory allocation may occur while the registry is locked. This makes
  the use of `tracing` inside of memory allocators difficult or
  impossible.

On the `master` branch (v0.2.x), PR #988 rewrote the callsite registry
to use an intrusive atomic singly-linked list of `Callsite`s. This
removed the need for locking the callsite registry, and made it possible
to register callsites without ever allocating memory. Because the
callsite registry on v0.2 will *never* allocate, this also makes it
possible to compile `tracing-core` for `no_std` platforms without
requiring `liballoc`. Unfortunately, however, we cannot use an identical
implementation on v0.1.x, because using the intrusive linked-list
registry for *all* callsites requires a breaking change to the
`Callsite` trait (in order to add a way to get the callsite's
linked-list node), which we cannot make on v0.1.x.

## Solution

This branch adds a linked-list callsite registry for v0.1.x in a way
that allows us to benefit from *some* of the advantages of this approach
in a majority of cases. The trick is introducing a new `DefaultCallsite`
type in `tracing-core` that implements the `Callsite` trait. This type
can contain an intrusive list node, and *when a callsite is a
`DefaultCallsite`*, we can register it without having to push it to the
`Mutex<Vec<...>>`. The locked vec still _exists_, for `Callsite`s that
are *not* `DefaultCallsite`s, so we can't remove the `liballoc`
dependency, but in most cases, we can avoid the mutex and allocation.

Naturally, `tracing` has been updated to use the `DefaultCallsite` type
from `tracing-core`, so the `Vec` will only be used in the following
cases:

* User code has a custom implementation of the `Callsite` trait, which
  is [not terribly common][1].
* An older version of the `tracing` crate is in use.

This fixes the deadlock described in #2020 when `DefaultCallsite`s are
used. Additionally, it should reduce the performance impact and memory
usage of the callsite registry.

Furthermore, I've changed the subscriber registry so that a
`RwLock<Vec<dyn Dispatch>>` is only used when there actually are
multiple subscribers in use. When there's only a global default
subscriber, we can once again avoid locking for the registry of
subscribers. When the `std` feature is disabled, thread-local scoped
dispatchers are unavailable, so the single global subscriber will always
be used on `no_std`.

We can make additional changes, such as the ones from #2020, to _also_
resolve potential deadlocks when non-default callsites are in use, but
since this branch rewrites a lot of the callsite registry code, that
should probably be done in a follow-up.

[1]: https://cs.github.com/?scopeName=All+repos&scope=&q=%28%2Fimpl+.*Callsite%2F+AND+language%3Arust%29+NOT+%28path%3A%2Ftracing%2F**+OR+path%3A%2Ftracing-*%2F**+OR+path%3A%2Ftokio-trace*%2F**%29%29

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw committed Apr 22, 2022
1 parent cdbd122 commit 45a5df1
Show file tree
Hide file tree
Showing 7 changed files with 517 additions and 226 deletions.

0 comments on commit 45a5df1

Please sign in to comment.