From d719cfb5d9189c2fe773915d8a2485db9adc3f69 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Oct 2021 10:00:55 -0700 Subject: [PATCH 1/3] attributes: deduplicate repeated code Signed-off-by: Eliza Weisman --- tracing-attributes/src/lib.rs | 69 +++++++++++++++++------------------ 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/tracing-attributes/src/lib.rs b/tracing-attributes/src/lib.rs index 130002417c..61bbc34143 100644 --- a/tracing-attributes/src/lib.rs +++ b/tracing-attributes/src/lib.rs @@ -513,7 +513,7 @@ fn gen_block( // enter the span and then perform the rest of the body. // If `err` is in args, instrument any resulting `Err`s. if async_context { - if err { + return if err { quote_spanned!(block.span()=> let __tracing_attr_span = #span; // See comment on the default case at the end of this function @@ -554,19 +554,33 @@ fn gen_block( fut.await } ) + }; + } + + let span = quote_spanned!(block.span()=> + // These variables are left uninitialized and initialized only + // if the tracing level is statically enabled at this point. + // While the tracing level is also checked at span creation + // time, that will still create a dummy span, and a dummy guard + // and drop the dummy guard later. By lazily initializing these + // variables, Rust will generate a drop flag for them and thus + // only drop the guard if it was created. This creates code that + // is very straightforward for LLVM to optimize out if the tracing + // level is statically disabled, while not causing any performance + // regression in case the level is enabled. + let __tracing_attr_span; + let __tracing_attr_guard; + if tracing::level_enabled!(#level) { + __tracing_attr_span = #span; + __tracing_attr_guard = __tracing_attr_span.enter(); } - } else if err { - quote_spanned!(block.span()=> - // See comment on the default case at the end of this function - // for why we do this a bit roundabout. - let __tracing_attr_span; - let __tracing_attr_guard; - if tracing::level_enabled!(#level) { - __tracing_attr_span = #span; - __tracing_attr_guard = __tracing_attr_span.enter(); - } - // pacify clippy::suspicious_else_formatting - let _ = (); + // pacify clippy::suspicious_else_formatting + let _ = (); + ); + + if err { + return quote_spanned!(block.span()=> + #span #[allow(clippy::redundant_closure_call)] match (move || #block)() { #[allow(clippy::unit_arg)] @@ -576,30 +590,13 @@ fn gen_block( Err(e) } } - ) - } else { - quote_spanned!(block.span()=> - // These variables are left uninitialized and initialized only - // if the tracing level is statically enabled at this point. - // While the tracing level is also checked at span creation - // time, that will still create a dummy span, and a dummy guard - // and drop the dummy guard later. By lazily initializing these - // variables, Rust will generate a drop flag for them and thus - // only drop the guard if it was created. This creates code that - // is very straightforward for LLVM to optimize out if the tracing - // level is statically disabled, while not causing any performance - // regression in case the level is enabled. - let __tracing_attr_span; - let __tracing_attr_guard; - if tracing::level_enabled!(#level) { - __tracing_attr_span = #span; - __tracing_attr_guard = __tracing_attr_span.enter(); - } - // pacify clippy::suspicious_else_formatting - let _ = (); - #block - ) + ); } + + quote_spanned!(block.span()=> + #span + #block + ) } #[derive(Default, Debug)] From aecfc1a7eee9ee5b231deecf2b1f41865b060eec Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Oct 2021 10:03:27 -0700 Subject: [PATCH 2/3] attributes: dedup async code more Signed-off-by: Eliza Weisman --- tracing-attributes/src/lib.rs | 46 +++++++++++++++-------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/tracing-attributes/src/lib.rs b/tracing-attributes/src/lib.rs index 61bbc34143..99db762f55 100644 --- a/tracing-attributes/src/lib.rs +++ b/tracing-attributes/src/lib.rs @@ -513,12 +513,9 @@ fn gen_block( // enter the span and then perform the rest of the body. // If `err` is in args, instrument any resulting `Err`s. if async_context { - return if err { + let mk_fut = if err { quote_spanned!(block.span()=> - let __tracing_attr_span = #span; - // See comment on the default case at the end of this function - // for why we do this a bit roundabout. - let fut = async move { + async move { match async move { #block }.await { #[allow(clippy::unit_arg)] Ok(x) => Ok(x), @@ -527,34 +524,29 @@ fn gen_block( Err(e) } } - }; - if tracing::level_enabled!(#level) { - tracing::Instrument::instrument( - fut, - __tracing_attr_span - ) - .await - } else { - fut.await } ) } else { quote_spanned!(block.span()=> - let __tracing_attr_span = #span; - // See comment on the default case at the end of this function - // for why we do this a bit roundabout. - let fut = async move { #block }; - if tracing::level_enabled!(#level) { - tracing::Instrument::instrument( - fut, - __tracing_attr_span - ) - .await - } else { - fut.await - } + async move { #block } ) }; + + return quote_spanned!(block.span()=> + let __tracing_attr_span = #span; + // See comment on the default case at the end of this function + // for why we do this a bit roundabout. + let fut = #mk_fut; + if tracing::level_enabled!(#level) { + tracing::Instrument::instrument( + fut, + __tracing_attr_span + ) + .await + } else { + fut.await + } + ); } let span = quote_spanned!(block.span()=> From ff3fb4a51f00c0f3bb1a0ba60e4beef5df23a328 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Oct 2021 10:09:34 -0700 Subject: [PATCH 3/3] attributes: skip async spans if level disabled this should hopefully improve the async case a bit as well Signed-off-by: Eliza Weisman --- tracing-attributes/src/lib.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tracing-attributes/src/lib.rs b/tracing-attributes/src/lib.rs index 99db762f55..b3029341c3 100644 --- a/tracing-attributes/src/lib.rs +++ b/tracing-attributes/src/lib.rs @@ -533,18 +533,15 @@ fn gen_block( }; return quote_spanned!(block.span()=> - let __tracing_attr_span = #span; - // See comment on the default case at the end of this function - // for why we do this a bit roundabout. - let fut = #mk_fut; if tracing::level_enabled!(#level) { + let __tracing_attr_span = #span; tracing::Instrument::instrument( - fut, + #mk_fut, __tracing_attr_span ) .await } else { - fut.await + #mk_fut.await } ); }