Rollup merge of rust-lang#143520 - Stypox:enter_trace_span-closure, r=RalfJung

Fix perf regression caused by tracing

See rust-lang#143334, this is another alternative that may be worth benchmarking as suggested in rust-lang#143334 (comment).

r? `@RalfJung`

Author: jhpratt

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -96,7 +96,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
     /// This inherent method takes priority over the trait method with the same name in LayoutOf,
     /// and allows wrapping the actual [LayoutOf::layout_of] with a tracing span.
     /// See [LayoutOf::layout_of] for the original documentation.
-    #[inline]
+    #[inline(always)]
     pub fn layout_of(
         &self,
         ty: Ty<'tcx>,

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -18,8 +18,8 @@ use rustc_target::callconv::FnAbi;
 
 use super::{
     AllocBytes, AllocId, AllocKind, AllocRange, Allocation, CTFE_ALLOC_SALT, ConstAllocation,
-    CtfeProvenance, FnArg, Frame, ImmTy, InterpCx, InterpResult, MPlaceTy, MemoryKind,
-    Misalignment, OpTy, PlaceTy, Pointer, Provenance, RangeSet, interp_ok, throw_unsup,
+    CtfeProvenance, EnteredTraceSpan, FnArg, Frame, ImmTy, InterpCx, InterpResult, MPlaceTy,
+    MemoryKind, Misalignment, OpTy, PlaceTy, Pointer, Provenance, RangeSet, interp_ok, throw_unsup,
 };
 
 /// Data returned by [`Machine::after_stack_pop`], and consumed by
@@ -147,12 +147,6 @@ pub trait Machine<'tcx>: Sized {
     /// already been checked before.
     const ALL_CONSTS_ARE_PRECHECKED: bool = true;
 
-    /// Determines whether rustc_const_eval functions that make use of the [Machine] should make
-    /// tracing calls (to the `tracing` library). By default this is `false`, meaning the tracing
-    /// calls will supposedly be optimized out. This flag is set to `true` inside Miri, to allow
-    /// tracing the interpretation steps, among other things.
-    const TRACING_ENABLED: bool = false;
-
     /// Whether memory accesses should be alignment-checked.
     fn enforce_alignment(ecx: &InterpCx<'tcx, Self>) -> bool;
 
@@ -634,6 +628,17 @@ pub trait Machine<'tcx>: Sized {
     /// Compute the value passed to the constructors of the `AllocBytes` type for
     /// abstract machine allocations.
     fn get_default_alloc_params(&self) -> <Self::Bytes as AllocBytes>::AllocParams;
+
+    /// Allows enabling/disabling tracing calls from within `rustc_const_eval` at compile time, by
+    /// delegating the entering of [tracing::Span]s to implementors of the [Machine] trait. The
+    /// default implementation corresponds to tracing being disabled, meaning the tracing calls will
+    /// supposedly be optimized out completely. To enable tracing, override this trait method and
+    /// return `span.entered()`. Also see [crate::enter_trace_span].
+    #[must_use]
+    #[inline(always)]
+    fn enter_trace_span(_span: impl FnOnce() -> tracing::Span) -> impl EnteredTraceSpan {
+        ()
+    }
 }
 
 /// A lot of the flexibility above is just needed for `Miri`, but all "compile-time" machines

compiler/rustc_const_eval/src/interpret/mod.rs

@@ -37,6 +37,7 @@ pub use self::place::{MPlaceTy, MemPlaceMeta, PlaceTy, Writeable};
 use self::place::{MemPlace, Place};
 pub use self::projection::{OffsetMode, Projectable};
 pub use self::stack::{Frame, FrameInfo, LocalState, ReturnContinuation, StackPopInfo};
+pub use self::util::EnteredTraceSpan;
 pub(crate) use self::util::create_static_alloc;
 pub use self::validity::{CtfeValidationMode, RangeSet, RefTracking};
 pub use self::visitor::ValueVisitor;

compiler/rustc_const_eval/src/interpret/util.rs

@@ -46,21 +46,20 @@ pub(crate) fn create_static_alloc<'tcx>(
     interp_ok(ecx.ptr_to_mplace(Pointer::from(alloc_id).into(), layout))
 }
 
-/// This struct is needed to enforce `#[must_use]` on [tracing::span::EnteredSpan]
-/// while wrapping them in an `Option`.
-#[must_use]
-pub enum MaybeEnteredSpan {
-    Some(tracing::span::EnteredSpan),
-    None,
-}
+/// A marker trait returned by [crate::interpret::Machine::enter_trace_span], identifying either a
+/// real [tracing::span::EnteredSpan] in case tracing is enabled, or the dummy type `()` when
+/// tracing is disabled.
+pub trait EnteredTraceSpan {}
+impl EnteredTraceSpan for () {}
+impl EnteredTraceSpan for tracing::span::EnteredSpan {}
 
+/// Shortand for calling [crate::interpret::Machine::enter_trace_span] on a [tracing::info_span].
+/// This is supposed to be compiled out when [crate::interpret::Machine::enter_trace_span] has the
+/// default implementation (i.e. when it does not actually enter the span but instead returns `()`).
+/// Note: the result of this macro **must be used** because the span is exited when it's dropped.
 #[macro_export]
 macro_rules! enter_trace_span {
     ($machine:ident, $($tt:tt)*) => {
-        if $machine::TRACING_ENABLED {
-            $crate::interpret::util::MaybeEnteredSpan::Some(tracing::info_span!($($tt)*).entered())
-        } else {
-            $crate::interpret::util::MaybeEnteredSpan::None
-        }
+        $machine::enter_trace_span(|| tracing::info_span!($($tt)*))
     }
 }

src/tools/miri/src/machine.rs

@@ -1014,8 +1014,6 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
 
     const PANIC_ON_ALLOC_FAIL: bool = false;
 
-    const TRACING_ENABLED: bool = cfg!(feature = "tracing");
-
     #[inline(always)]
     fn enforce_alignment(ecx: &MiriInterpCx<'tcx>) -> bool {
         ecx.machine.check_alignment != AlignmentCheck::None
@@ -1827,6 +1825,16 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
         #[cfg(not(target_os = "linux"))]
         MiriAllocParams::Global
     }
+
+    fn enter_trace_span(span: impl FnOnce() -> tracing::Span) -> impl EnteredTraceSpan {
+        #[cfg(feature = "tracing")]
+        { span().entered() }
+        #[cfg(not(feature = "tracing"))]
+        {
+            let _ = span; // so we avoid the "unused variable" warning
+            ()
+        }
+    }
 }
 
 /// Trait for callbacks handling asynchronous machine operations.