From fe108d3aabfcdbd335412f61ffb49fe1657395d7 Mon Sep 17 00:00:00 2001 From: Vladimir Vukicevic Date: Fri, 29 Mar 2024 15:50:42 -0700 Subject: [PATCH 1/2] Add --unlink-aux-files option --- samply/src/linux_shared/process.rs | 4 ++-- samply/src/mac/task_profiler.rs | 5 ++++- samply/src/main.rs | 8 ++++++++ samply/src/shared/jitdump_manager.rs | 10 ++++++++-- samply/src/shared/recording_props.rs | 2 ++ 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/samply/src/linux_shared/process.rs b/samply/src/linux_shared/process.rs index ad830f61..3d213451 100644 --- a/samply/src/linux_shared/process.rs +++ b/samply/src/linux_shared/process.rs @@ -60,7 +60,7 @@ where Self { profile_process: process_handle, unwinder: U::default(), - jitdump_manager: JitDumpManager::new(), + jitdump_manager: JitDumpManager::new(false), lib_mapping_ops: Default::default(), name, pid, @@ -181,7 +181,7 @@ where None }; - let jitdump_manager = std::mem::replace(&mut self.jitdump_manager, JitDumpManager::new()); + let jitdump_manager = std::mem::replace(&mut self.jitdump_manager, JitDumpManager::new(false)); let jitdump_ops = jitdump_manager.finish( jit_category_manager, profile, diff --git a/samply/src/mac/task_profiler.rs b/samply/src/mac/task_profiler.rs index c2e62ac1..472f5a80 100644 --- a/samply/src/mac/task_profiler.rs +++ b/samply/src/mac/task_profiler.rs @@ -254,7 +254,7 @@ impl TaskProfiler { ignored_errors: Vec::new(), unwinder: UnwinderNative::new(), path_receiver, - jitdump_manager: JitDumpManager::new(), + jitdump_manager: JitDumpManager::new(profile_creation_props.unlink_aux_files), marker_file_paths: Vec::new(), lib_mapping_ops: Default::default(), unresolved_samples: Default::default(), @@ -602,6 +602,9 @@ impl TaskProfiler { name: span.name, } })); + if self.profile_creation_props.unlink_aux_files { + std::fs::remove_file(marker_file_path).ok(); + } } } let process_sample_data = ProcessSampleData::new( diff --git a/samply/src/main.rs b/samply/src/main.rs index 7acbc4b0..24b858b6 100644 --- a/samply/src/main.rs +++ b/samply/src/main.rs @@ -180,6 +180,12 @@ pub struct ProfileCreationArgs { /// Fold repeated frames at the base of the stack. #[arg(long)] fold_recursive_prefix: bool, + + /// If a process produces jitdump or marker files, unlink them after + /// opening. This ensures that the files will not be left in /tmp, + /// but it will also be impossible to look at JIT disassembly. + #[arg(long)] + unlink_aux_files: bool, } fn main() { @@ -293,6 +299,7 @@ impl ImportArgs { profile_name, reuse_threads: self.profile_creation_args.reuse_threads, fold_recursive_prefix: self.profile_creation_args.fold_recursive_prefix, + unlink_aux_files: self.profile_creation_args.unlink_aux_files, } } } @@ -371,6 +378,7 @@ impl RecordArgs { profile_name, reuse_threads: self.profile_creation_args.reuse_threads, fold_recursive_prefix: self.profile_creation_args.fold_recursive_prefix, + unlink_aux_files: self.profile_creation_args.unlink_aux_files, } } } diff --git a/samply/src/shared/jitdump_manager.rs b/samply/src/shared/jitdump_manager.rs index dffdf0c1..34486e16 100644 --- a/samply/src/shared/jitdump_manager.rs +++ b/samply/src/shared/jitdump_manager.rs @@ -19,13 +19,15 @@ use super::utils::open_file_with_fallback; pub struct JitDumpManager { pending_jitdump_paths: Vec<(ThreadHandle, PathBuf, Option)>, processors: Vec, + unlink_after_open: bool, } impl JitDumpManager { - pub fn new() -> Self { + pub fn new(unlink_after_open: bool) -> Self { JitDumpManager { pending_jitdump_paths: Vec::new(), processors: Vec::new(), + unlink_after_open, } } @@ -51,13 +53,17 @@ impl JitDumpManager { fn jitdump_reader_for_path( path: &Path, fallback_dir: Option<&Path>, + unlink_after_open: bool, ) -> Option<(JitDumpReader, PathBuf)> { let (file, path) = open_file_with_fallback(path, fallback_dir).ok()?; let reader = JitDumpReader::new(file).ok()?; + if unlink_after_open { + std::fs::remove_file(&path).ok()?; + } Some((reader, path)) } let Some((reader, actual_path)) = - jitdump_reader_for_path(path, fallback_dir.as_deref()) + jitdump_reader_for_path(path, fallback_dir.as_deref(), self.unlink_after_open) else { return true; }; diff --git a/samply/src/shared/recording_props.rs b/samply/src/shared/recording_props.rs index c29e605c..8805f8cd 100644 --- a/samply/src/shared/recording_props.rs +++ b/samply/src/shared/recording_props.rs @@ -17,6 +17,8 @@ pub struct ProfileCreationProps { pub reuse_threads: bool, /// Fold repeated frames at the base of the stack. pub fold_recursive_prefix: bool, + /// Unlink jitdump/marker files + pub unlink_aux_files: bool, } /// Properties which are meaningful for launching and recording a fresh process. From 947bb45219a8eb3b32bb52d1fdcaa207bde7f199 Mon Sep 17 00:00:00 2001 From: Vladimir Vukicevic Date: Sun, 21 Apr 2024 15:14:34 +0200 Subject: [PATCH 2/2] linux support --- samply/src/import/perf.rs | 4 +--- samply/src/linux/profiler.rs | 4 +--- samply/src/linux_shared/converter.rs | 14 ++++++++------ samply/src/linux_shared/process.rs | 6 ++++-- samply/src/linux_shared/processes.rs | 9 ++++++++- samply/src/main.rs | 3 ++- 6 files changed, 24 insertions(+), 16 deletions(-) diff --git a/samply/src/import/perf.rs b/samply/src/import/perf.rs index 295ec3cd..dbf1b3c7 100644 --- a/samply/src/import/perf.rs +++ b/samply/src/import/perf.rs @@ -104,7 +104,7 @@ where let interpretation = EventInterpretation::divine_from_attrs(attributes); let mut converter = Converter::::new( - &profile_creation_props.profile_name, + &profile_creation_props, Some(Box::new(move |name| { format!("{name} on {host} (perf version {perf_version})") })), @@ -115,8 +115,6 @@ where cache, extra_dir, interpretation.clone(), - profile_creation_props.reuse_threads, - profile_creation_props.fold_recursive_prefix, ); let mut last_timestamp = 0; diff --git a/samply/src/linux/profiler.rs b/samply/src/linux/profiler.rs index 0d3becbf..709c1634 100644 --- a/samply/src/linux/profiler.rs +++ b/samply/src/linux/profiler.rs @@ -356,7 +356,7 @@ fn make_converter( }; Converter::>::new( - &profile_creation_props.profile_name, + &profile_creation_props, None, HashMap::new(), machine_info.as_ref().map(|info| info.release.as_str()), @@ -365,8 +365,6 @@ fn make_converter( framehop::CacheNative::new(), None, interpretation, - profile_creation_props.reuse_threads, - profile_creation_props.fold_recursive_prefix, ) } diff --git a/samply/src/linux_shared/converter.rs b/samply/src/linux_shared/converter.rs index 4f71fbe6..4dfa800c 100644 --- a/samply/src/linux_shared/converter.rs +++ b/samply/src/linux_shared/converter.rs @@ -38,6 +38,7 @@ use super::vdso::VdsoObject; use crate::shared::jit_category_manager::JitCategoryManager; use crate::shared::process_sample_data::RssStatMember; +use crate::shared::recording_props::ProfileCreationProps; use crate::shared::timestamp_converter::TimestampConverter; use crate::shared::types::{StackFrame, StackMode}; use crate::shared::unresolved_samples::{ @@ -83,7 +84,7 @@ where { #[allow(clippy::too_many_arguments)] pub fn new( - product: &str, + profile_creation_props: &ProfileCreationProps, delayed_product_name_generator: Option, build_ids: HashMap, linux_version: Option<&str>, @@ -92,15 +93,13 @@ where cache: U::Cache, extra_binary_artifact_dir: Option<&Path>, interpretation: EventInterpretation, - reuse_threads: bool, - fold_recursive_prefix: bool, ) -> Self { let interval = match interpretation.sampling_is_time_based { Some(nanos) => SamplingInterval::from_nanos(nanos), None => SamplingInterval::from_millis(1), }; let profile = Profile::new( - product, + &profile_creation_props.profile_name, ReferenceTimestamp::from_system_time(SystemTime::now()), interval, ); @@ -124,7 +123,10 @@ where Self { profile, cache, - processes: Processes::new(reuse_threads), + processes: Processes::new( + profile_creation_props.reuse_threads, + profile_creation_props.unlink_aux_files, + ), timestamp_converter, current_sample_time: first_sample_time, build_ids, @@ -140,7 +142,7 @@ where kernel_symbols, pe_mappings: PeMappings::new(), jit_category_manager: JitCategoryManager::new(), - fold_recursive_prefix, + fold_recursive_prefix: profile_creation_props.fold_recursive_prefix, } } diff --git a/samply/src/linux_shared/process.rs b/samply/src/linux_shared/process.rs index 3d213451..add20dd9 100644 --- a/samply/src/linux_shared/process.rs +++ b/samply/src/linux_shared/process.rs @@ -56,11 +56,12 @@ where name: Option, thread_recycler: Option, jit_function_recycler: Option, + unlink_aux_files: bool, ) -> Self { Self { profile_process: process_handle, unwinder: U::default(), - jitdump_manager: JitDumpManager::new(false), + jitdump_manager: JitDumpManager::new(unlink_aux_files), lib_mapping_ops: Default::default(), name, pid, @@ -181,7 +182,8 @@ where None }; - let jitdump_manager = std::mem::replace(&mut self.jitdump_manager, JitDumpManager::new(false)); + let jitdump_manager = + std::mem::replace(&mut self.jitdump_manager, JitDumpManager::new(false)); let jitdump_ops = jitdump_manager.finish( jit_category_manager, profile, diff --git a/samply/src/linux_shared/processes.rs b/samply/src/linux_shared/processes.rs index 56c25335..52d20193 100644 --- a/samply/src/linux_shared/processes.rs +++ b/samply/src/linux_shared/processes.rs @@ -25,13 +25,16 @@ where /// The sample data for all removed processes. process_sample_datas: Vec, + + /// Whether aux files (like jitdump) should be unlinked on open + unlink_aux_data: bool, } impl Processes where U: Unwinder + Default, { - pub fn new(allow_reuse: bool) -> Self { + pub fn new(allow_reuse: bool, unlink_aux_data: bool) -> Self { let process_recycler = if allow_reuse { Some(ProcessRecycler::new()) } else { @@ -41,6 +44,7 @@ where processes_by_pid: HashMap::new(), process_recycler, process_sample_datas: Vec::new(), + unlink_aux_data, } } @@ -70,6 +74,7 @@ where name, Some(thread_recycler), Some(jit_function_recycler), + self.unlink_aux_data, ); return entry.insert(process); } @@ -101,6 +106,7 @@ where name, thread_recycler, jit_function_recycler, + self.unlink_aux_data, ); entry.insert(process) } @@ -130,6 +136,7 @@ where None, // no name thread_recycler, jit_function_recycler, + self.unlink_aux_data, ) }) } diff --git a/samply/src/main.rs b/samply/src/main.rs index 24b858b6..c2fdbd27 100644 --- a/samply/src/main.rs +++ b/samply/src/main.rs @@ -183,7 +183,8 @@ pub struct ProfileCreationArgs { /// If a process produces jitdump or marker files, unlink them after /// opening. This ensures that the files will not be left in /tmp, - /// but it will also be impossible to look at JIT disassembly. + /// but it will also be impossible to look at JIT disassembly, and line + /// numbers will be missing for JIT frames. #[arg(long)] unlink_aux_files: bool, }