From 1c4ae7aa4a3c9f4ffa23a81b83cebbb34f51e7d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Fri, 1 Apr 2022 20:50:16 +0200 Subject: [PATCH 1/6] turn `exec` comment into doc comment --- compiler/rustc_data_structures/src/profiling.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index fd6ff086b08..fa6c4a02cab 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -183,11 +183,11 @@ pub fn new( } } - // This shim makes sure that calls only get executed if the filter mask - // lets them pass. It also contains some trickery to make sure that - // code is optimized for non-profiling compilation sessions, i.e. anything - // past the filter check is never inlined so it doesn't clutter the fast - // path. + /// This shim makes sure that calls only get executed if the filter mask + /// lets them pass. It also contains some trickery to make sure that + /// code is optimized for non-profiling compilation sessions, i.e. anything + /// past the filter check is never inlined so it doesn't clutter the fast + /// path. #[inline(always)] fn exec(&self, event_filter: EventFilter, f: F) -> TimingGuard<'_> where From 75852696735110cb41903e0699a4fc0e8617fce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Fri, 1 Apr 2022 20:54:12 +0200 Subject: [PATCH 2/6] add `generic_activity_with_arg_recorder` to the self-profiler This allows profiling costly arguments to be recorded only when `-Zself-profile-events=args` is on: using a closure that takes an `EventArgRecorder` and call its `record_arg` or `record_args` methods. --- .../rustc_data_structures/src/profiling.rs | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/compiler/rustc_data_structures/src/profiling.rs b/compiler/rustc_data_structures/src/profiling.rs index fa6c4a02cab..3f7a90a8467 100644 --- a/compiler/rustc_data_structures/src/profiling.rs +++ b/compiler/rustc_data_structures/src/profiling.rs @@ -97,6 +97,7 @@ pub use measureme::EventId; use measureme::{EventIdBuilder, Profiler, SerializableString, StringId}; use parking_lot::RwLock; +use smallvec::SmallVec; bitflags::bitflags! { struct EventFilter: u32 { @@ -288,6 +289,66 @@ pub fn generic_activity_with_arg( }) } + /// Start profiling a generic activity, allowing costly arguments to be recorded. Profiling + /// continues until the `TimingGuard` returned from this call is dropped. + /// + /// If the arguments to a generic activity are cheap to create, use `generic_activity_with_arg` + /// or `generic_activity_with_args` for their simpler API. However, if they are costly or + /// require allocation in sufficiently hot contexts, then this allows for a closure to be called + /// only when arguments were asked to be recorded via `-Z self-profile-events=args`. + /// + /// In this case, the closure will be passed a `&mut EventArgRecorder`, to help with recording + /// one or many arguments within the generic activity being profiled, by calling its + /// `record_arg` method for example. + /// + /// This `EventArgRecorder` may implement more specific traits from other rustc crates, e.g. for + /// richer handling of rustc-specific argument types, while keeping this single entry-point API + /// for recording arguments. + /// + /// Note: recording at least one argument is *required* for the self-profiler to create the + /// `TimingGuard`. A panic will be triggered if that doesn't happen. This function exists + /// explicitly to record arguments, so it fails loudly when there are none to record. + /// + #[inline(always)] + pub fn generic_activity_with_arg_recorder( + &self, + event_label: &'static str, + mut f: F, + ) -> TimingGuard<'_> + where + F: FnMut(&mut EventArgRecorder<'_>), + { + // Ensure this event will only be recorded when self-profiling is turned on. + self.exec(EventFilter::GENERIC_ACTIVITIES, |profiler| { + let builder = EventIdBuilder::new(&profiler.profiler); + let event_label = profiler.get_or_alloc_cached_string(event_label); + + // Ensure the closure to create event arguments will only be called when argument + // recording is turned on. + let event_id = if profiler.event_filter_mask.contains(EventFilter::FUNCTION_ARGS) { + // Set up the builder and call the user-provided closure to record potentially + // costly event arguments. + let mut recorder = EventArgRecorder { profiler, args: SmallVec::new() }; + f(&mut recorder); + + // It is expected that the closure will record at least one argument. If that + // doesn't happen, it's a bug: we've been explicitly called in order to record + // arguments, so we fail loudly when there are none to record. + if recorder.args.is_empty() { + panic!( + "The closure passed to `generic_activity_with_arg_recorder` needs to \ + record at least one argument" + ); + } + + builder.from_label_and_args(event_label, &recorder.args) + } else { + builder.from_label(event_label) + }; + TimingGuard::start(profiler, profiler.generic_activity_event_kind, event_id) + }) + } + /// Record the size of an artifact that the compiler produces /// /// `artifact_kind` is the class of artifact (e.g., query_cache, object_file, etc.) @@ -443,6 +504,33 @@ pub fn get_self_profiler(&self) -> Option> { } } +/// A helper for recording costly arguments to self-profiling events. Used with +/// `SelfProfilerRef::generic_activity_with_arg_recorder`. +pub struct EventArgRecorder<'p> { + /// The `SelfProfiler` used to intern the event arguments that users will ask to record. + profiler: &'p SelfProfiler, + + /// The interned event arguments to be recorded in the generic activity event. + /// + /// The most common case, when actually recording event arguments, is to have one argument. Then + /// followed by recording two, in a couple places. + args: SmallVec<[StringId; 2]>, +} + +impl EventArgRecorder<'_> { + /// Records a single argument within the current generic activity being profiled. + /// + /// Note: when self-profiling with costly event arguments, at least one argument + /// needs to be recorded. A panic will be triggered if that doesn't happen. + pub fn record_arg(&mut self, event_arg: A) + where + A: Borrow + Into, + { + let event_arg = self.profiler.get_or_alloc_cached_string(event_arg); + self.args.push(event_arg); + } +} + pub struct SelfProfiler { profiler: Profiler, event_filter_mask: EventFilter, From 3a8006714be2524b650f046f028836578ddc7e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Fri, 1 Apr 2022 21:01:47 +0200 Subject: [PATCH 3/6] simplify a self-profiling activity call in the LLVM backend and so that it doesn't allocate unless event argument recording is turned on --- compiler/rustc_codegen_llvm/src/back/write.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index c18719d4ad7..7ef3b12cd08 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -721,8 +721,7 @@ pub(crate) fn link( let mut linker = Linker::new(first.module_llvm.llmod()); for module in elements { - let _timer = - cgcx.prof.generic_activity_with_arg("LLVM_link_module", format!("{:?}", module.name)); + let _timer = cgcx.prof.generic_activity_with_arg("LLVM_link_module", &*module.name); let buffer = ModuleBuffer::new(module.module_llvm.llmod()); linker.add(buffer.data()).map_err(|()| { let msg = format!("failed to serialize module {:?}", module.name); From af22801db08b9cd3e7dac9531bdab8ba8ad91d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Fri, 1 Apr 2022 21:02:14 +0200 Subject: [PATCH 4/6] simplify a self-profiling activity call in the cg_gcc backend --- compiler/rustc_codegen_gcc/src/back/write.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_gcc/src/back/write.rs b/compiler/rustc_codegen_gcc/src/back/write.rs index b503bd020f6..efcf18d31eb 100644 --- a/compiler/rustc_codegen_gcc/src/back/write.rs +++ b/compiler/rustc_codegen_gcc/src/back/write.rs @@ -11,7 +11,7 @@ use crate::{GccCodegenBackend, GccContext}; pub(crate) unsafe fn codegen(cgcx: &CodegenContext, _diag_handler: &Handler, module: ModuleCodegen, config: &ModuleConfig) -> Result { - let _timer = cgcx.prof.generic_activity_with_arg("LLVM_module_codegen", &module.name[..]); + let _timer = cgcx.prof.generic_activity_with_arg("LLVM_module_codegen", &*module.name); { let context = &module.module_llvm.context; From b6a7b5accd03da6655e8150bcbd36defe1e43081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Fri, 1 Apr 2022 21:10:44 +0200 Subject: [PATCH 5/6] remove allocation from a self-profiling call in the LLVM backend --- compiler/rustc_codegen_llvm/src/back/lto.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index 0f5b1c08ec2..7a747a9cdee 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -313,7 +313,9 @@ fn fat_lto( for (bc_decoded, name) in serialized_modules { let _timer = cgcx .prof - .generic_activity_with_arg("LLVM_fat_lto_link_module", format!("{:?}", name)); + .generic_activity_with_arg_recorder("LLVM_fat_lto_link_module", |recorder| { + recorder.record_arg(format!("{:?}", name)) + }); info!("linking {:?}", name); let data = bc_decoded.data(); linker.add(data).map_err(|()| { From 1906b7e967f98e8c735054af61906e86cfc80ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 7 Apr 2022 10:32:58 +0200 Subject: [PATCH 6/6] port `codegen_module` activity to arg recorder API --- compiler/rustc_codegen_llvm/src/base.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index dd3ada44389..7a7335f2be2 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -74,10 +74,11 @@ pub fn compile_codegen_unit(tcx: TyCtxt<'_>, cgu_name: Symbol) -> (ModuleCodegen fn module_codegen(tcx: TyCtxt<'_>, cgu_name: Symbol) -> ModuleCodegen { let cgu = tcx.codegen_unit(cgu_name); - let _prof_timer = tcx.prof.generic_activity_with_args( - "codegen_module", - &[cgu_name.to_string(), cgu.size_estimate().to_string()], - ); + let _prof_timer = + tcx.prof.generic_activity_with_arg_recorder("codegen_module", |recorder| { + recorder.record_arg(cgu_name.to_string()); + recorder.record_arg(cgu.size_estimate().to_string()); + }); // Instantiate monomorphizations without filling out definitions yet... let llvm_module = ModuleLlvm::new(tcx, cgu_name.as_str()); {