From f9de97811cfa616018a5c604a56db4ffbb3a9577 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 5 Mar 2024 11:44:40 -0500 Subject: [PATCH] On Linux/Android, put markers from marker files on the thread where the marker file was opened. --- samply/src/linux_shared/converter.rs | 10 +++++--- samply/src/linux_shared/process.rs | 9 ++------ samply/src/mac/task_profiler.rs | 10 ++++++-- samply/src/shared/jitdump_manager.rs | 34 ++++++++++++++-------------- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/samply/src/linux_shared/converter.rs b/samply/src/linux_shared/converter.rs index c19db224..0185f1d1 100644 --- a/samply/src/linux_shared/converter.rs +++ b/samply/src/linux_shared/converter.rs @@ -667,9 +667,13 @@ where if filename.starts_with("jit-") && filename.ends_with(".dump") { let jitdump_path = Path::new(path); let process = self.processes.get_by_pid(pid, &mut self.profile); - process - .jitdump_manager - .add_jitdump_path(jitdump_path, self.extra_binary_artifact_dir.clone()); + let thread = process.threads.get_thread_by_tid(tid, &mut self.profile); + let profile_thread = thread.profile_thread; + process.jitdump_manager.add_jitdump_path( + profile_thread, + jitdump_path, + self.extra_binary_artifact_dir.clone(), + ); return true; } diff --git a/samply/src/linux_shared/process.rs b/samply/src/linux_shared/process.rs index e2f6eb2f..8272275b 100644 --- a/samply/src/linux_shared/process.rs +++ b/samply/src/linux_shared/process.rs @@ -57,7 +57,7 @@ where Self { profile_process: process_handle, unwinder: U::default(), - jitdump_manager: JitDumpManager::new_for_process(main_thread_handle), + jitdump_manager: JitDumpManager::new(), lib_mapping_ops: Default::default(), name, pid, @@ -164,12 +164,7 @@ where None }; - // TODO: Load marker files - - let jitdump_manager = std::mem::replace( - &mut self.jitdump_manager, - JitDumpManager::new_for_process(self.threads.main_thread.profile_thread), - ); + let jitdump_manager = std::mem::replace(&mut self.jitdump_manager, JitDumpManager::new()); 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 07b12f96..7c76c7d2 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_for_process(main_thread_handle), + jitdump_manager: JitDumpManager::new(), marker_file_paths: Vec::new(), lib_mapping_ops: Default::default(), unresolved_samples: Default::default(), @@ -524,7 +524,13 @@ impl TaskProfiler { while let Ok(jitdump_or_marker_file_path) = self.path_receiver.try_recv() { match jitdump_or_marker_file_path { JitdumpOrMarkerPath::JitdumpPath(jitdump_path) => { - self.jitdump_manager.add_jitdump_path(jitdump_path, None); + // TODO: Detect which thread the jitdump file is opened on, and use that thread's + // thread handle so that the JitFunctionAdd markers are put on that thread in the profile. + self.jitdump_manager.add_jitdump_path( + self.main_thread_handle, + jitdump_path, + None, + ); } JitdumpOrMarkerPath::MarkerFilePath(marker_file_path) => { // TODO: Detect which thread the marker file is opened on, and use that thread's diff --git a/samply/src/shared/jitdump_manager.rs b/samply/src/shared/jitdump_manager.rs index a17235fa..bc40df27 100644 --- a/samply/src/shared/jitdump_manager.rs +++ b/samply/src/shared/jitdump_manager.rs @@ -17,22 +17,26 @@ use super::utils::open_file_with_fallback; #[derive(Debug)] pub struct JitDumpManager { - pending_jitdump_paths: Vec<(PathBuf, Option)>, + pending_jitdump_paths: Vec<(ThreadHandle, PathBuf, Option)>, processors: Vec, - main_thread_handle: ThreadHandle, } impl JitDumpManager { - pub fn new_for_process(main_thread_handle: ThreadHandle) -> Self { + pub fn new() -> Self { JitDumpManager { pending_jitdump_paths: Vec::new(), processors: Vec::new(), - main_thread_handle, } } - pub fn add_jitdump_path(&mut self, path: impl Into, fallback_dir: Option) { - self.pending_jitdump_paths.push((path.into(), fallback_dir)); + pub fn add_jitdump_path( + &mut self, + thread: ThreadHandle, + path: impl Into, + fallback_dir: Option, + ) { + self.pending_jitdump_paths + .push((thread, path.into(), fallback_dir)); } pub fn process_pending_records( @@ -43,7 +47,7 @@ impl JitDumpManager { timestamp_converter: &TimestampConverter, ) { self.pending_jitdump_paths - .retain_mut(|(path, fallback_dir)| { + .retain_mut(|(thread, path, fallback_dir)| { fn jitdump_reader_for_path( path: &Path, fallback_dir: Option<&Path>, @@ -62,11 +66,8 @@ impl JitDumpManager { reader.header(), profile, ); - self.processors.push(SingleJitDumpProcessor::new( - reader, - lib_handle, - self.main_thread_handle, - )); + self.processors + .push(SingleJitDumpProcessor::new(reader, lib_handle, *thread)); false // "Do not retain", i.e. remove from pending_jitdump_paths }); @@ -102,7 +103,7 @@ struct SingleJitDumpProcessor { lib_handle: LibraryHandle, lib_mapping_ops: LibMappingOpQueue, symbols: Vec, - main_thread_handle: ThreadHandle, + thread_handle: ThreadHandle, /// The relative_address of the next JIT function. /// @@ -118,14 +119,14 @@ impl SingleJitDumpProcessor { pub fn new( reader: JitDumpReader, lib_handle: LibraryHandle, - main_thread_handle: ThreadHandle, + thread_handle: ThreadHandle, ) -> Self { Self { reader: Some(reader), lib_handle, lib_mapping_ops: Default::default(), symbols: Default::default(), - main_thread_handle, + thread_handle, cumulative_address: 0, } } @@ -178,11 +179,10 @@ impl SingleJitDumpProcessor { name: symbol_name.to_owned(), }); - let main_thread = self.main_thread_handle; let timestamp = timestamp_converter.convert_time(raw_jitdump_record.timestamp); let timing = MarkerTiming::Instant(timestamp); profile.add_marker( - main_thread, + self.thread_handle, "JitFunctionAdd", JitFunctionAddMarker(symbol_name.to_owned()), timing,