Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core, tracing: don't inline dispatcher::get_default #994

Merged
merged 1 commit into from
Sep 28, 2020
Merged

core, tracing: don't inline dispatcher::get_default #994

merged 1 commit into from
Sep 28, 2020

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Sep 28, 2020 •
edited
Loading

Motivation

Presently, functions for recording new spans and events that can get the
current dispatcher (Event::dispatch, Event::child_of, Span::new,
and Span::new_root) are marked as #[inline]. This results in these
methods getting inlined in most cases, and dispatcher::get_default is
sometimes inlined into those methods. This, then, inlines all of
dispatcher::get_default into the callsites of the span! and event!
macros, generating way too much code (including thread local accesses)
inline. Since there's an expect in get_default, this also means that
unwinding code is inlined into functions that wouldn't otherwise panic,
which is also not great. Inlining too much code this results in code that
optimizes poorly in many cases.

Solution

This PR removes #[inline] attributes from these functions, ensuring
that the TLS hits are always behind a function call.

@hawkw hawkw added crate/core Related to the `tracing-core` crate crate/tracing Related to the `tracing` crate labels Sep 28, 2020
@hawkw hawkw requested a review from a team as a code owner September 28, 2020 15:56
@hawkw hawkw self-assigned this Sep 28, 2020
@hawkw hawkw merged commit 4bbdc7c into master Sep 28, 2020
hawkw added a commit that referenced this pull request Sep 28, 2020
@hawkw
core: prepare to release 0.1.17
61faef1 
Fixed

- Incorrect inlining of `Event::dispatch` and `Event::child_of`, which
  could result in `dispatcher::get_default` being inlined at the
  callsite (#994)

Added

- `Copy` implementations for `Level` and `LevelFilter` (#992)

Thanks to new contributors @jyn514 and @TaKO8Ki for contributing to this
release!

Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw mentioned this pull request Sep 28, 2020
Merged
hawkw added a commit that referenced this pull request Sep 28, 2020
@hawkw
core: prepare to release 0.1.17 (#996)
182684e 
### Fixed

- Incorrect inlining of `Event::dispatch` and `Event::child_of`, which
  could result in `dispatcher::get_default` being inlined at the
  callsite (#994)

### Added

- `Copy` implementations for `Level` and `LevelFilter` (#992)

Thanks to new contributors @jyn514 and @TaKO8Ki for contributing 
to this release!

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Sep 28, 2020
@hawkw
tracing: prepare to release 0.1.21
ee5eadd 
Fixed

- Incorrect inlining of `Span::new`, `Span::new_root`, and
  `Span::new_child_of`, which could result in `dispatcher::get_default`
  being  inlined at the callsite ([#994])
- Regression where using a struct field as a span or event field when
  other fields on that struct are borrowed mutably would fail to compile
  ([#987])

Changed

- Updated `tracing-core` to 0.1.17 ([#992])

Added

- `Instrument` trait and `Instrumented` type for attaching a `Span` to a
  `Future` ([#808])
- `Copy` implementations for `Level` and `LevelFilter` ([#992])
- Multiple documentation fixes and improvements ([#964], [#980], [#981])

Thanks to @nagisa, and new contributors @securityinsanity, @froydnj,
@jyn514 and @TaKO8Ki for contributing to this release!

[#994]: #994
[#992]: #992
[#987]: #987
[#980]: #980
[#981]: #981
[#964]: #964
[#808]: #808

Signed-off-by: Eliza Weisman <[email protected]>
@hawkw hawkw mentioned this pull request Sep 28, 2020
Merged
hawkw added a commit that referenced this pull request Sep 28, 2020
@hawkw
tracing: prepare to release 0.1.21 (#997)
386969b 
### Fixed

- Incorrect inlining of `Span::new`, `Span::new_root`, and
  `Span::new_child_of`, which could result in `dispatcher::get_default`
  being  inlined at the callsite ([#994])
- Regression where using a struct field as a span or event field when
  other fields on that struct are borrowed mutably would fail to compile
  ([#987])

### Changed

- Updated `tracing-core` to 0.1.17 ([#992])

### Added

- `Instrument` trait and `Instrumented` type for attaching a `Span` to a
  `Future` ([#808])
- `Copy` implementations for `Level` and `LevelFilter` ([#992])
- Multiple documentation fixes and improvements ([#964], [#980], [#981])

Thanks to @nagisa, and new contributors @securityinsanity, @froydnj,
@jyn514 and @TaKO8Ki for contributing to this release!

[#994]: #994
[#992]: #992
[#987]: #987
[#980]: #980
[#981]: #981
[#964]: #964
[#808]: #808

Signed-off-by: Eliza Weisman <[email protected]>
dvdplm added a commit to dvdplm/tracing that referenced this pull request Oct 7, 2020
@dvdplm
Merge remote-tracking branch 'upstream/master' into dp-target-is-cow
381d574 
* upstream/master:
  subscriber: warn if trying to enable a statically disabled level (tokio-rs#990)
  subscriber: use macros for module declarations (tokio-rs#1009)
  chore: remove `stdlib.rs` (tokio-rs#1008)
  core: fix linked list tests reusing `Registration`s (tokio-rs#1016)
  subscriber: support dash in target names (tokio-rs#1012)
  docs: switch to intra-doc links in tracing-core (tokio-rs#1010)
  tracing-opentelemetry: implement additional record types (bool, i64, u64) (tokio-rs#1007)
  core: add intrusive linked list for callsite registry (tokio-rs#988)
  serde: allow tracing-serde to work on no_std. (tokio-rs#960)
  tracing: remove `Into<Option<Id>>` impl for `Span` (tokio-rs#1003)
  tracing: make `Entered` `!Send` (tokio-rs#1001)
  chore: fix nightly clippy warnings (tokio-rs#991)
  chore: bump all crate versions (tokio-rs#998)
  macros: fix the `tracing-macros` crate not compiling (tokio-rs#1000)
  tracing: prepare to release 0.1.21 (tokio-rs#997)
  core: prepare to release 0.1.17 (tokio-rs#996)
  subscriber: make `PartialOrd` & `Ord` impls more correct (tokio-rs#995)
  core, tracing: don't inline dispatcher::get_default (tokio-rs#994)
  core: make `Level` and `LevelFilter` `Copy` (tokio-rs#992)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LucioFranco LucioFranco LucioFranco approved these changes

@davidbarsky davidbarsky Awaiting requested review from davidbarsky davidbarsky is a code owner

Assignees

@hawkw hawkw

Labels
crate/core Related to the `tracing-core` crate crate/tracing Related to the `tracing` crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hawkw @LucioFranco