Skip to content

Commit 731fd3d

Browse files
authored
Unrolled build for #144708
Rollup merge of #144708 - Stypox:add-tracing-to-step, r=RalfJung Add tracing to step.rs and friends Adds tracing calls to functions in `step.rs` (01717ff), to friend functions related to evaluation and stepping (cbfa7c4), and adds a new trait method `EnteredTraceSpan::or_if_tracing_disabled` (f0d0d1f). Adding `EnteredTraceSpan::or_if_tracing_disabled` is optional and is only useful to avoid having both `tracing::info!()` calls (that existed before) and `enter_trace_span!()` calls (that this PR adds) that would be redundant and would slow down the collection of traces. I say it is optional because it adds some cognitive complexity around `EnteredTraceSpan`, which is possibly not worth the reduced redundancy. Let me know if I should revert that commit. The tracing calls added in this PR are meant to make it easier to understand what was being executing at a particular point when looking at a trace. But they are likely not useful for the purpose of understanding which components are fast/slow, hence why I used `tracing_separate_thread` for them. After opening a trace generated using the code in this PR in https://ui.perfetto.dev, and after executing the following query and then pressing on "Show debug track", you will see something like the following image in the timeline: ```sql select slices.id, ts, dur, track_id, category, args.string_value as name, depth, stack_id, parent_stack_id, parent_id, slices.arg_set_id, thread_ts, thread_instruction_count, thread_instruction_delta, cat, slice_id from slices inner join args USING (arg_set_id) where args.key = "args." || slices.name and name = "step" ``` <img width="739" height="87" alt="image" src="https://github.com/user-attachments/assets/74ad9619-9a1f-40e5-9ef4-3db31e33d6e1" />
2 parents e3ee7f7 + 88c9a25 commit 731fd3d

File tree

5 files changed

+75
-9
lines changed

5 files changed

+75
-9
lines changed

compiler/rustc_const_eval/src/interpret/call.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ use rustc_middle::ty::{self, AdtDef, Instance, Ty, VariantDef};
1111
use rustc_middle::{bug, mir, span_bug};
1212
use rustc_span::sym;
1313
use rustc_target::callconv::{ArgAbi, FnAbi, PassMode};
14+
use tracing::field::Empty;
1415
use tracing::{info, instrument, trace};
1516

1617
use super::{
1718
CtfeProvenance, FnVal, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, PlaceTy,
1819
Projectable, Provenance, ReturnAction, ReturnContinuation, Scalar, StackPopInfo, interp_ok,
1920
throw_ub, throw_ub_custom, throw_unsup_format,
2021
};
21-
use crate::fluent_generated as fluent;
22+
use crate::interpret::EnteredTraceSpan;
23+
use crate::{enter_trace_span, fluent_generated as fluent};
2224

2325
/// An argument passed to a function.
2426
#[derive(Clone, Debug)]
@@ -344,6 +346,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
344346
destination: &PlaceTy<'tcx, M::Provenance>,
345347
mut cont: ReturnContinuation,
346348
) -> InterpResult<'tcx> {
349+
let _span = enter_trace_span!(M, step::init_stack_frame, %instance, tracing_separate_thread = Empty);
350+
347351
// Compute callee information.
348352
// FIXME: for variadic support, do we have to somehow determine callee's extra_args?
349353
let callee_fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?;
@@ -523,7 +527,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
523527
target: Option<mir::BasicBlock>,
524528
unwind: mir::UnwindAction,
525529
) -> InterpResult<'tcx> {
526-
trace!("init_fn_call: {:#?}", fn_val);
530+
let _span =
531+
enter_trace_span!(M, step::init_fn_call, tracing_separate_thread = Empty, ?fn_val)
532+
.or_if_tracing_disabled(|| trace!("init_fn_call: {:#?}", fn_val));
527533

528534
let instance = match fn_val {
529535
FnVal::Instance(instance) => instance,

compiler/rustc_const_eval/src/interpret/operand.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@ use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter};
1313
use rustc_middle::ty::{ConstInt, ScalarInt, Ty, TyCtxt};
1414
use rustc_middle::{bug, mir, span_bug, ty};
1515
use rustc_span::DUMMY_SP;
16+
use tracing::field::Empty;
1617
use tracing::trace;
1718

1819
use super::{
1920
CtfeProvenance, Frame, InterpCx, InterpResult, MPlaceTy, Machine, MemPlace, MemPlaceMeta,
2021
OffsetMode, PlaceTy, Pointer, Projectable, Provenance, Scalar, alloc_range, err_ub,
2122
from_known_layout, interp_ok, mir_assign_valid_types, throw_ub,
2223
};
24+
use crate::enter_trace_span;
2325

2426
/// An `Immediate` represents a single immediate self-contained Rust value.
2527
///
@@ -770,6 +772,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
770772
mir_place: mir::Place<'tcx>,
771773
layout: Option<TyAndLayout<'tcx>>,
772774
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
775+
let _span = enter_trace_span!(
776+
M,
777+
step::eval_place_to_op,
778+
?mir_place,
779+
tracing_separate_thread = Empty
780+
);
781+
773782
// Do not use the layout passed in as argument if the base we are looking at
774783
// here is not the entire place.
775784
let layout = if mir_place.projection.is_empty() { layout } else { None };
@@ -813,6 +822,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
813822
mir_op: &mir::Operand<'tcx>,
814823
layout: Option<TyAndLayout<'tcx>>,
815824
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
825+
let _span =
826+
enter_trace_span!(M, step::eval_operand, ?mir_op, tracing_separate_thread = Empty);
827+
816828
use rustc_middle::mir::Operand::*;
817829
let op = match mir_op {
818830
// FIXME: do some more logic on `move` to invalidate the old location

compiler/rustc_const_eval/src/interpret/place.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ use rustc_abi::{BackendRepr, HasDataLayout, Size};
99
use rustc_middle::ty::Ty;
1010
use rustc_middle::ty::layout::TyAndLayout;
1111
use rustc_middle::{bug, mir, span_bug};
12+
use tracing::field::Empty;
1213
use tracing::{instrument, trace};
1314

1415
use super::{
1516
AllocInit, AllocRef, AllocRefMut, CheckAlignMsg, CtfeProvenance, ImmTy, Immediate, InterpCx,
1617
InterpResult, Machine, MemoryKind, Misalignment, OffsetMode, OpTy, Operand, Pointer,
1718
Projectable, Provenance, Scalar, alloc_range, interp_ok, mir_assign_valid_types,
1819
};
20+
use crate::enter_trace_span;
1921

2022
#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
2123
/// Information required for the sound usage of a `MemPlace`.
@@ -524,6 +526,9 @@ where
524526
&self,
525527
mir_place: mir::Place<'tcx>,
526528
) -> InterpResult<'tcx, PlaceTy<'tcx, M::Provenance>> {
529+
let _span =
530+
enter_trace_span!(M, step::eval_place, ?mir_place, tracing_separate_thread = Empty);
531+
527532
let mut place = self.local_to_place(mir_place.local)?;
528533
// Using `try_fold` turned out to be bad for performance, hence the loop.
529534
for elem in mir_place.projection.iter() {

compiler/rustc_const_eval/src/interpret/step.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ use rustc_middle::ty::{self, Instance, Ty};
99
use rustc_middle::{bug, mir, span_bug};
1010
use rustc_span::source_map::Spanned;
1111
use rustc_target::callconv::FnAbi;
12+
use tracing::field::Empty;
1213
use tracing::{info, instrument, trace};
1314

1415
use super::{
1516
FnArg, FnVal, ImmTy, Immediate, InterpCx, InterpResult, Machine, MemPlaceMeta, PlaceTy,
1617
Projectable, Scalar, interp_ok, throw_ub, throw_unsup_format,
1718
};
18-
use crate::util;
19+
use crate::interpret::EnteredTraceSpan;
20+
use crate::{enter_trace_span, util};
1921

2022
struct EvaluatedCalleeAndArgs<'tcx, M: Machine<'tcx>> {
2123
callee: FnVal<'tcx, M::ExtraFnVal>,
@@ -74,7 +76,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
7476
///
7577
/// This does NOT move the statement counter forward, the caller has to do that!
7678
pub fn eval_statement(&mut self, stmt: &mir::Statement<'tcx>) -> InterpResult<'tcx> {
77-
info!("{:?}", stmt);
79+
let _span = enter_trace_span!(
80+
M,
81+
step::eval_statement,
82+
stmt = ?stmt.kind,
83+
span = ?stmt.source_info.span,
84+
tracing_separate_thread = Empty,
85+
)
86+
.or_if_tracing_disabled(|| info!(stmt = ?stmt.kind));
7887

7988
use rustc_middle::mir::StatementKind::*;
8089

@@ -456,7 +465,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
456465
}
457466

458467
fn eval_terminator(&mut self, terminator: &mir::Terminator<'tcx>) -> InterpResult<'tcx> {
459-
info!("{:?}", terminator.kind);
468+
let _span = enter_trace_span!(
469+
M,
470+
step::eval_terminator,
471+
terminator = ?terminator.kind,
472+
span = ?terminator.source_info.span,
473+
tracing_separate_thread = Empty,
474+
)
475+
.or_if_tracing_disabled(|| info!(terminator = ?terminator.kind));
460476

461477
use rustc_middle::mir::TerminatorKind::*;
462478
match terminator.kind {

compiler/rustc_const_eval/src/interpret/util.rs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,24 @@ pub(crate) fn create_static_alloc<'tcx>(
4848

4949
/// A marker trait returned by [crate::interpret::Machine::enter_trace_span], identifying either a
5050
/// real [tracing::span::EnteredSpan] in case tracing is enabled, or the dummy type `()` when
51-
/// tracing is disabled.
52-
pub trait EnteredTraceSpan {}
53-
impl EnteredTraceSpan for () {}
54-
impl EnteredTraceSpan for tracing::span::EnteredSpan {}
51+
/// tracing is disabled. Also see [crate::enter_trace_span!] below.
52+
pub trait EnteredTraceSpan {
53+
/// Allows executing an alternative function when tracing is disabled. Useful for example if you
54+
/// want to open a trace span when tracing is enabled, and alternatively just log a line when
55+
/// tracing is disabled.
56+
fn or_if_tracing_disabled(self, f: impl FnOnce()) -> Self;
57+
}
58+
impl EnteredTraceSpan for () {
59+
fn or_if_tracing_disabled(self, f: impl FnOnce()) -> Self {
60+
f(); // tracing is disabled, execute the function
61+
self
62+
}
63+
}
64+
impl EnteredTraceSpan for tracing::span::EnteredSpan {
65+
fn or_if_tracing_disabled(self, _f: impl FnOnce()) -> Self {
66+
self // tracing is enabled, don't execute anything
67+
}
68+
}
5569

5670
/// Shortand for calling [crate::interpret::Machine::enter_trace_span] on a [tracing::info_span!].
5771
/// This is supposed to be compiled out when [crate::interpret::Machine::enter_trace_span] has the
@@ -112,6 +126,19 @@ impl EnteredTraceSpan for tracing::span::EnteredSpan {}
112126
/// # type M = rustc_const_eval::const_eval::CompileTimeMachine<'static>;
113127
/// let _span = enter_trace_span!(M, step::eval_statement, tracing_separate_thread = tracing::field::Empty);
114128
/// ```
129+
///
130+
/// ### Executing something else when tracing is disabled
131+
///
132+
/// [crate::interpret::Machine::enter_trace_span] returns [EnteredTraceSpan], on which you can call
133+
/// [EnteredTraceSpan::or_if_tracing_disabled], to e.g. log a line as an alternative to the tracing
134+
/// span for when tracing is disabled. For example:
135+
/// ```rust
136+
/// # use rustc_const_eval::enter_trace_span;
137+
/// # use rustc_const_eval::interpret::EnteredTraceSpan;
138+
/// # type M = rustc_const_eval::const_eval::CompileTimeMachine<'static>;
139+
/// let _span = enter_trace_span!(M, step::eval_statement)
140+
/// .or_if_tracing_disabled(|| tracing::info!("eval_statement"));
141+
/// ```
115142
#[macro_export]
116143
macro_rules! enter_trace_span {
117144
($machine:ty, $name:ident :: $subname:ident $($tt:tt)*) => {

0 commit comments

Comments
 (0)