mirror of
https://github.com/tokio-rs/tokio.git
synced 2026-08-20 00:00:08 +02:00
Eliza Weisman
197f88f3bc
## Motivation
The `Span::enter` function previously required an `&mut` reference to
enter a span. This is a relic of an earlier design where span closure
logic was determined by dropping an inner span component, and is no
longer strictly necessary.
Requiring `&mut self` to enter a span leads to awkward patterns in cases
when a user wishes to enter a span and then call methods on the span
(such as recording field values). For example, we cannot say
```rust
let mut span = span!("foo", bar);
span.enter(|| {
span.record("bar" &false);
});
```
since the span is mutably borrowed by `enter`. Instead, we must clone
the span, like so:
```rust
let mut span = span!("foo", bar);
span.clone().enter(|| {
span.record("bar" &false);
});
```
Having to clone the span is somewhat less ergonomic, and it has
performance disadvantages as well: cloning a `Span` will clone the
span's `Dispatch` handle, requiring an `Arc` bump, as well as calling
the `Subscriber`'s `clone_span` and `drop_span` functions. If we can
enter spans without a mutable borrow, we don't have to update any of
these ref counts.
The other reason we may wish to require mutable borrows to enter a span
is if we want to disallow entering a span multiple times before exiting
it. However, it is trivially possible to re-enter a span on the same
thread regardless, by cloning the span and entering it twice. Besides,
there may be a valuable semantic meaning in entering a span from inside
itself, such as when a function is called recursively, so disallowing
this is not a goal.
## Solution
This branch rewrites the `Span::enter`, `Span::record`, and
`Span::record_all` functions to no longer require mutable borrows.
In the case of `record` and `record_all`, this was trivial, as borrowing
mutably was not actually *necessary* for those functions. For `enter`,
the `Entered` guard type was reworked to consist of an `&'a Inner`
rather than an `Inner`, so it is no longer necessary to `take` the
span's `Inner`.
## Notes
In addition to allowing spans to be entered without mutable borrows,
`Entered` was changed to exit the span automatically when the guard is
dropped, so we may now observe correct span exits even when unwinding.
Furthermore, this allows us to simplify the `enter` function a bit,
leading to a minor performance improvement when entering spans.
Before:
```
test enter_span ... bench: 13 ns/iter (+/- 1)
```
...and after:
```
test enter_span ... bench: 3 ns/iter (+/- 1)
```
Note that this branch also contains a change to make the
`subscriber::enter_span` benchmark more accurate. Previously, this
benchmark constructed a new span inside of `b.iter(|| {...})`. This
means that the benchmark was measuring not only the time taken to enter
a span, but the time taken to construct a `Span` handle as well.
However, we already have benchmarks for span construction, and the
intention of this particular benchmark was to measure the overhead of
constructing a span.
I've updated the benchmark by moving the span construction out of the
`iter` closure. Now, the span is constructed a single time and entered
on every iteration. This allows us to measure only the overhead of
actually entering a span. The "before" benchmark numbers above were
recorded after backporting this change to master, so they are "fair" to
the previous implementation. Prior to this change the benchmark took
approximately 53 ns.
Signed-off-by: Eliza Weisman <[email protected]>