On Linux/Android, put markers from marker files on the thread where t…

…he marker file was opened.
mstange · Mar 5, 2024 · f9de978 · f9de978
1 parent 50364c2
commit f9de978
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -17,22 +17,26 @@ use super::utils::open_file_with_fallback;
 
 #[derive(Debug)]
 pub struct JitDumpManager {
-    pending_jitdump_paths: Vec<(PathBuf, Option<PathBuf>)>,
+    pending_jitdump_paths: Vec<(ThreadHandle, PathBuf, Option<PathBuf>)>,
     processors: Vec<SingleJitDumpProcessor>,
-    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<PathBuf>, fallback_dir: Option<PathBuf>) {
-        self.pending_jitdump_paths.push((path.into(), fallback_dir));
+    pub fn add_jitdump_path(
+        &mut self,
+        thread: ThreadHandle,
+        path: impl Into<PathBuf>,
+        fallback_dir: Option<PathBuf>,
+    ) {
+        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<Symbol>,
-    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<std::fs::File>,
         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,