--wip-- [skip ci]

Author: GuillaumeLagrange

crates/runner-shared/src/metadata.rs

@@ -9,7 +9,7 @@ use std::path::PathBuf;
 use crate::debug_info::{DebugInfoPidMapping, ModuleDebugInfo};
 use crate::fifo::MarkerType;
 use crate::perf_map::ProcessModuleLoadBias;
-use crate::unwind_data::UnwindDataPidMapping;
+use crate::unwind_data::MappedProcessUnwindData;
 
 #[derive(Serialize, Deserialize)]
 pub struct PerfMetadata {
@@ -45,7 +45,7 @@ pub struct PerfMetadata {
 
     /// Per-pid unwind data references, mapping PID to list of unwind data index + mounting info
     #[serde(default, skip_serializing_if = "HashMap::is_empty")]
-    pub unwind_data_pid_mappings_by_pid: HashMap<pid_t, Vec<UnwindDataPidMapping>>,
+    pub unwind_data_pid_mappings_by_pid: HashMap<pid_t, Vec<MappedProcessUnwindData>>,
 
     /// Per-pid symbol references, mapping PID to list of perf map index + load bias
     #[serde(default, skip_serializing_if = "HashMap::is_empty")]

crates/runner-shared/src/unwind_data.rs

@@ -163,18 +163,23 @@ impl Debug for UnwindDataV3 {
 }
 
 /// Per-pid mounting info referencing a deduplicated unwind data entry.
-#[derive(Serialize, Deserialize, Clone)]
-pub struct UnwindDataPidMapping {
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct MappedProcessUnwindData {
     pub unwind_data_key: String,
+    #[serde(flatten)]
+    pub inner: ProcessUnwindData,
+}
+
+#[derive(Serialize, Deserialize, Clone)]
+pub struct ProcessUnwindData {
     pub timestamp: Option<u64>,
     pub avma_range: Range<u64>,
     pub base_avma: u64,
 }
 
-impl Debug for UnwindDataPidMapping {
+impl Debug for ProcessUnwindData {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("UnwindDataPidMapping")
-            .field("unwind_data_key", &self.unwind_data_key)
+        f.debug_struct("ProcessUnwindData")
             .field("timestamp", &self.timestamp)
             .field("avma_range", &format_args!("{:x?}", self.avma_range))
             .field("base_avma", &format_args!("{:x}", self.base_avma))

src/bin/compare_walltime_output.rs

@@ -177,10 +177,10 @@ fn load_new_format(
             entries.push(NormalizedEntry {
                 path: v3.path.clone(),
                 base_svma: v3.base_svma,
-                base_avma: mapping.base_avma,
-                avma_start: mapping.avma_range.start,
-                avma_end: mapping.avma_range.end,
-                timestamp: mapping.timestamp,
+                base_avma: mapping.inner.base_avma,
+                avma_start: mapping.inner.avma_range.start,
+                avma_end: mapping.inner.avma_range.end,
+                timestamp: mapping.inner.timestamp,
                 eh_frame_hdr_hash: hash_bytes(&v3.eh_frame_hdr),
                 eh_frame_hash: hash_bytes(&v3.eh_frame),
             });

src/executor/wall_time/perf/debug_info.rs

@@ -1,4 +1,4 @@
-use crate::executor::wall_time::perf::perf_map::ModuleSymbols;
+use crate::executor::wall_time::perf::module_symbols::ModuleSymbols;
 use crate::prelude::*;
 use addr2line::{fallible_iterator::FallibleIterator, gimli};
 use object::{Object, ObjectSection};

src/executor/wall_time/perf/jit_dump.rs

@@ -1,16 +1,14 @@
 use crate::{
-    executor::wall_time::perf::perf_map::{ModuleSymbols, Symbol},
+    executor::wall_time::perf::module_symbols::{ModuleSymbols, Symbol},
     prelude::*,
 };
 use linux_perf_data::jitdump::{JitDumpReader, JitDumpRecord};
-use runner_shared::unwind_data::UnwindData;
+use runner_shared::unwind_data::{ProcessUnwindData, UnwindData};
 use std::{
     collections::{HashMap, HashSet},
     path::{Path, PathBuf},
 };
 
-use super::parse_perf_file::ProcessUnwindData;
-
 struct JitDump {
     path: PathBuf,
 }

src/executor/wall_time/perf/parse_perf_file.rs

@@ -1,25 +1,25 @@
-use super::perf_map::ModuleSymbols;
+use super::module_symbols::ModuleSymbols;
 use super::unwind_data::unwind_data_from_elf;
 use crate::prelude::*;
 use libc::pid_t;
 use linux_perf_data::PerfFileReader;
 use linux_perf_data::PerfFileRecord;
 use linux_perf_data::linux_perf_event_reader::EventRecord;
 use linux_perf_data::linux_perf_event_reader::RecordType;
+use runner_shared::unwind_data::ProcessUnwindData;
 use runner_shared::unwind_data::UnwindData;
 use std::collections::HashMap;
 use std::collections::HashSet;
-use std::ops::Range;
 use std::path::Path;
 use std::path::PathBuf;
 
-// TODO: Move this somewhere else ?
 #[derive(Default)]
 pub struct MountedModule {
     /// Symbols extracted from the mapped ELF file
     pub module_symbols: Option<ModuleSymbols>,
     /// Unwind data extracted from the mapped ELF file
     pub unwind_data: Option<UnwindData>,
+    /// Per-process mounting information
     pub process_mounted_module: HashMap<pid_t, ProcessMountedModule>,
 }
 
@@ -32,15 +32,6 @@ pub struct ProcessMountedModule {
     pub process_unwind_data: Option<ProcessUnwindData>,
 }
 
-// TODO: Move this somehwere else ?
-// Or determine if we cannot use the metadata type directly
-#[derive(Debug)]
-pub struct ProcessUnwindData {
-    pub timestamp: Option<u64>,
-    pub base_avma: u64,
-    pub avma_range: Range<u64>,
-}
-
 impl MountedModule {
     pub fn pids(&self) -> impl Iterator<Item = pid_t> {
         self.process_mounted_module.keys().copied()
@@ -243,7 +234,6 @@ fn process_mmap2_record(
     // Extract unwind_data
     match unwind_data_from_elf(
         record_path_string.as_bytes(),
-        record.page_offset,
         record.address,
         end_addr,
         None,

src/executor/wall_time/perf/save_artifacts.rs

@@ -1,21 +1,21 @@
 use crate::executor::valgrind::helpers::ignored_objects_path::get_objects_path_to_ignore;
 use crate::executor::wall_time::perf::debug_info::debug_info_by_path;
 use crate::executor::wall_time::perf::naming;
-use crate::executor::wall_time::perf::parse_perf_file::{MountedModule, ProcessUnwindData};
+use crate::executor::wall_time::perf::parse_perf_file::MountedModule;
 use crate::prelude::*;
 use libc::pid_t;
 use rayon::prelude::*;
 use runner_shared::debug_info::{DebugInfoPidMapping, ModuleDebugInfo};
 use runner_shared::perf_map::ProcessModuleLoadBias;
-use runner_shared::unwind_data::{UnwindData, UnwindDataPidMapping};
+use runner_shared::unwind_data::{MappedProcessUnwindData, ProcessUnwindData, UnwindData};
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
 pub struct SavedArtifacts {
     pub symbol_pid_mappings_by_pid: HashMap<pid_t, Vec<ProcessModuleLoadBias>>,
     pub debug_info: HashMap<String, ModuleDebugInfo>,
     pub debug_info_pid_mappings_by_pid: HashMap<pid_t, Vec<DebugInfoPidMapping>>,
-    pub unwind_data_pid_mappings_by_pid: HashMap<pid_t, Vec<UnwindDataPidMapping>>,
+    pub unwind_data_pid_mappings_by_pid: HashMap<pid_t, Vec<MappedProcessUnwindData>>,
     pub ignored_modules: Vec<(String, u64, u64)>,
     pub key_to_path: HashMap<String, PathBuf>,
 }
@@ -182,7 +182,7 @@ fn save_unwind_data(
     mounted_modules_by_path: &HashMap<PathBuf, MountedModule>,
     jit_unwind_data_by_pid: &HashMap<pid_t, Vec<(UnwindData, ProcessUnwindData)>>,
     path_to_key: &mut HashMap<PathBuf, String>,
-) -> HashMap<pid_t, Vec<UnwindDataPidMapping>> {
+) -> HashMap<pid_t, Vec<MappedProcessUnwindData>> {
     let unwind_data_count = mounted_modules_by_path
         .values()
         .filter(|m| m.unwind_data.is_some())
@@ -196,7 +196,7 @@ fn save_unwind_data(
         }
     });
 
-    let mut mappings_by_pid: HashMap<pid_t, Vec<UnwindDataPidMapping>> = HashMap::new();
+    let mut mappings_by_pid: HashMap<pid_t, Vec<MappedProcessUnwindData>> = HashMap::new();
     for (path, m) in mounted_modules_by_path {
         if m.unwind_data.is_none() {
             continue;
@@ -207,11 +207,13 @@ fn save_unwind_data(
                 mappings_by_pid
                     .entry(pid)
                     .or_default()
-                    .push(UnwindDataPidMapping {
+                    .push(MappedProcessUnwindData {
                         unwind_data_key: key.clone(),
-                        timestamp: pud.timestamp,
-                        avma_range: pud.avma_range.clone(),
-                        base_avma: pud.base_avma,
+                        inner: runner_shared::unwind_data::ProcessUnwindData {
+                            timestamp: pud.timestamp,
+                            avma_range: pud.avma_range.clone(),
+                            base_avma: pud.base_avma,
+                        },
                     });
             }
         }
@@ -226,11 +228,13 @@ fn save_unwind_data(
             mappings_by_pid
                 .entry(pid)
                 .or_default()
-                .push(UnwindDataPidMapping {
+                .push(MappedProcessUnwindData {
                     unwind_data_key: key,
-                    timestamp: process_unwind_data.timestamp,
-                    avma_range: process_unwind_data.avma_range.clone(),
-                    base_avma: process_unwind_data.base_avma,
+                    inner: runner_shared::unwind_data::ProcessUnwindData {
+                        timestamp: process_unwind_data.timestamp,
+                        avma_range: process_unwind_data.avma_range.clone(),
+                        base_avma: process_unwind_data.base_avma,
+                    },
                 });
         }
     }

src/executor/wall_time/perf/unwind_data.rs

@@ -5,16 +5,13 @@ use anyhow::{Context, bail};
 use debugid::CodeId;
 use object::Object;
 use object::ObjectSection;
+use runner_shared::unwind_data::ProcessUnwindData;
 use runner_shared::unwind_data::UnwindData;
 use std::ops::Range;
-use std::path::Path;
-
-use super::parse_perf_file::ProcessUnwindData;
 
 // Based on: https://github.com/mstange/linux-perf-stuff/blob/22ca6531b90c10dd2a4519351c843b8d7958a451/src/main.rs#L747-L893
 pub fn unwind_data_from_elf(
     path_slice: &[u8],
-    runtime_file_offset: u64,
     runtime_start_addr: u64,
     runtime_end_addr: u64,
     build_id: Option<&[u8]>,
@@ -105,11 +102,10 @@ mod tests {
         let object = object::File::parse(&file_data[..]).expect("Failed to parse test binary");
         let load_bias =
             elf_helper::compute_load_bias(start_addr, end_addr, file_offset, &object).unwrap();
-        println!("Load bias for {}: 0x{:x}", module_path, load_bias);
+        println!("Load bias for {module_path}: 0x{load_bias:x}");
         assert_eq!(
             load_bias, expected_load_bias,
-            "Invalid load bias: {:x} != {:x}",
-            load_bias, expected_load_bias
+            "Invalid load bias: {load_bias:x} != {expected_load_bias:x}"
         );
         load_bias
     }
@@ -142,7 +138,6 @@ mod tests {
             assert_and_get_load_bias(start_addr, end_addr, file_offset, MODULE_PATH, 0x0);
         insta::assert_debug_snapshot!(unwind_data_from_elf(
             MODULE_PATH.as_bytes(),
-            file_offset,
             start_addr,
             end_addr,
             None,
@@ -166,7 +161,6 @@ mod tests {
 
         insta::assert_debug_snapshot!(unwind_data_from_elf(
             MODULE_PATH.as_bytes(),
-            file_offset,
             start_addr,
             end_addr,
             None,
@@ -189,7 +183,6 @@ mod tests {
         );
         insta::assert_debug_snapshot!(unwind_data_from_elf(
             MODULE_PATH.as_bytes(),
-            file_offset,
             start_addr,
             end_addr,
             None,
@@ -218,7 +211,6 @@ mod tests {
         );
         insta::assert_debug_snapshot!(unwind_data_from_elf(
             MODULE_PATH.as_bytes(),
-            file_offset,
             start_addr,
             end_addr,
             None,
@@ -248,7 +240,6 @@ mod tests {
 
         insta::assert_debug_snapshot!(unwind_data_from_elf(
             MODULE_PATH.as_bytes(),
-            file_offset,
             start_addr,
             end_addr,
             None,