--wip-- [skip ci]

Author: GuillaumeLagrange

src/executor/wall_time/perf/debug_info.rs

@@ -135,7 +135,8 @@ mod tests {
     fn test_golang_debug_info() {
         let (start_addr, end_addr, file_offset) =
             (0x0000000000402000_u64, 0x000000000050f000_u64, 0x2000);
-        let module_symbols = ModuleSymbols::new(
+        let module_symbols = ModuleSymbols::from_elf("testdata/perf_map/go_fib.bin").unwrap();
+        let load_bias = ModuleSymbols::compute_load_bias(
             "testdata/perf_map/go_fib.bin",
             start_addr,
             end_addr,
@@ -145,7 +146,7 @@ mod tests {
         let module_debug_info = ModuleDebugInfo::from_symbols(
             "testdata/perf_map/go_fib.bin",
             &module_symbols,
-            module_symbols.load_bias(),
+            load_bias,
         )
         .unwrap();
         insta::assert_debug_snapshot!(module_debug_info.debug_infos);
@@ -155,7 +156,9 @@ mod tests {
     fn test_cpp_debug_info() {
         let (start_addr, end_addr, file_offset) =
             (0x0000000000400000_u64, 0x0000000000459000_u64, 0x0);
-        let module_symbols = ModuleSymbols::new(
+        let module_symbols =
+            ModuleSymbols::from_elf("testdata/perf_map/cpp_my_benchmark.bin").unwrap();
+        let load_bias = ModuleSymbols::compute_load_bias(
             "testdata/perf_map/cpp_my_benchmark.bin",
             start_addr,
             end_addr,
@@ -165,7 +168,7 @@ mod tests {
         let mut module_debug_info = ModuleDebugInfo::from_symbols(
             "testdata/perf_map/cpp_my_benchmark.bin",
             &module_symbols,
-            module_symbols.load_bias(),
+            load_bias,
         )
         .unwrap();
 
@@ -178,29 +181,33 @@ mod tests {
     fn test_rust_divan_debug_info() {
         const MODULE_PATH: &str = "testdata/perf_map/divan_sleep_benches.bin";
 
-        let module_symbols =
-            ModuleSymbols::new(MODULE_PATH, 0x00005555555a2000, 0x0000555555692000, 0x4d000)
-                .unwrap();
+        let module_symbols = ModuleSymbols::from_elf(MODULE_PATH).unwrap();
+        let load_bias = ModuleSymbols::compute_load_bias(
+            MODULE_PATH,
+            0x00005555555a2000,
+            0x0000555555692000,
+            0x4d000,
+        )
+        .unwrap();
         let module_debug_info =
-            ModuleDebugInfo::from_symbols(MODULE_PATH, &module_symbols, module_symbols.load_bias())
-                .unwrap();
+            ModuleDebugInfo::from_symbols(MODULE_PATH, &module_symbols, load_bias).unwrap();
         insta::assert_debug_snapshot!(module_debug_info.debug_infos);
     }
 
     #[test]
     fn test_the_algorithms_debug_info() {
         const MODULE_PATH: &str = "testdata/perf_map/the_algorithms.bin";
 
-        let module_symbols = ModuleSymbols::new(
+        let module_symbols = ModuleSymbols::from_elf(MODULE_PATH).unwrap();
+        let load_bias = ModuleSymbols::compute_load_bias(
             MODULE_PATH,
             0x00005573e59fe000,
             0x00005573e5b07000,
             0x00052000,
         )
         .unwrap();
         let module_debug_info =
-            ModuleDebugInfo::from_symbols(MODULE_PATH, &module_symbols, module_symbols.load_bias())
-                .unwrap();
+            ModuleDebugInfo::from_symbols(MODULE_PATH, &module_symbols, load_bias).unwrap();
         insta::assert_debug_snapshot!(module_debug_info.debug_infos);
     }
 
@@ -210,11 +217,12 @@ mod tests {
 
         let (start_addr, end_addr, file_offset) =
             (0x0000555555e6d000_u64, 0x0000555556813000_u64, 0x918000);
-        let module_symbols =
-            ModuleSymbols::new(MODULE_PATH, start_addr, end_addr, file_offset).unwrap();
-        let module_debug_info =
-            ModuleDebugInfo::from_symbols(MODULE_PATH, &module_symbols, module_symbols.load_bias())
+        let module_symbols = ModuleSymbols::from_elf(MODULE_PATH).unwrap();
+        let load_bias =
+            ModuleSymbols::compute_load_bias(MODULE_PATH, start_addr, end_addr, file_offset)
                 .unwrap();
+        let module_debug_info =
+            ModuleDebugInfo::from_symbols(MODULE_PATH, &module_symbols, load_bias).unwrap();
         insta::assert_debug_snapshot!(module_debug_info.debug_infos);
     }
 }

src/executor/wall_time/perf/save_artifacts.rs

@@ -31,44 +31,44 @@ fn get_or_insert_key(path_to_key: &mut HashMap<PathBuf, String>, path: &Path) ->
     key
 }
 
-/// Save all artifacts (symbols, debug info, unwind data) from mounted modules and JIT data.
-pub fn save_artifacts(
-    profile_folder: &Path,
+/// Pre-register all paths from the mounted modules map.
+fn register_paths(
+    path_to_key: &mut HashMap<PathBuf, String>,
     mounted_modules_by_path: &HashMap<PathBuf, MountedModule>,
-    jit_unwind_data_by_pid: &HashMap<pid_t, Vec<(UnwindData, ProcessUnwindData)>>,
-) -> SavedArtifacts {
-    let mut path_to_key = HashMap::<PathBuf, String>::new();
+) {
+    for path in mounted_modules_by_path.keys() {
+        get_or_insert_key(path_to_key, path);
+    }
+}
 
-    // --- 1. Save symbol files and build per-pid symbol mappings ---
+/// Save deduplicated symbol files to disk and build per-pid mappings.
+fn save_symbols(
+    profile_folder: &Path,
+    mounted_modules_by_path: &HashMap<PathBuf, MountedModule>,
+    path_to_key: &HashMap<PathBuf, String>,
+) -> HashMap<pid_t, Vec<ProcessModuleLoadBias>> {
     let symbols_count = mounted_modules_by_path
         .values()
         .filter(|m| m.module_symbols.is_some())
         .count();
     debug!("Saving symbols ({symbols_count} unique entries)");
 
-    // Pre-register all paths
-    for path in mounted_modules_by_path.keys() {
-        get_or_insert_key(&mut path_to_key, path);
-    }
-
-    // Save symbol files in parallel
     mounted_modules_by_path.par_iter().for_each(|(path, m)| {
         if let Some(ref symbols) = m.module_symbols {
             let key = &path_to_key[path];
             symbols.save_to_keyed_file(profile_folder, key).unwrap();
         }
     });
 
-    // Build per-pid symbol mappings
-    let mut symbol_pid_mappings_by_pid: HashMap<pid_t, Vec<ProcessModuleLoadBias>> = HashMap::new();
+    let mut mappings_by_pid: HashMap<pid_t, Vec<ProcessModuleLoadBias>> = HashMap::new();
     for (path, m) in mounted_modules_by_path {
         if m.module_symbols.is_none() {
             continue;
         }
         let key = &path_to_key[path];
         for (&pid, pm) in &m.process_mounted_module {
             if let Some(load_bias) = pm.symbols_load_bias {
-                symbol_pid_mappings_by_pid
+                mappings_by_pid
                     .entry(pid)
                     .or_default()
                     .push(ProcessModuleLoadBias {
@@ -78,19 +78,26 @@ pub fn save_artifacts(
             }
         }
     }
-    // Sort each pid's mappings by key for deterministic output
-    for mappings in symbol_pid_mappings_by_pid.values_mut() {
+    for mappings in mappings_by_pid.values_mut() {
         mappings.sort_by(|a, b| a.perf_map_key.cmp(&b.perf_map_key));
     }
+    mappings_by_pid
+}
 
-    // --- 2. Compute debug info and build per-pid debug info mappings ---
+/// Compute debug info from symbols and build per-pid debug info mappings.
+fn save_debug_info(
+    mounted_modules_by_path: &HashMap<PathBuf, MountedModule>,
+    path_to_key: &mut HashMap<PathBuf, String>,
+) -> (
+    HashMap<String, ModuleDebugInfo>,
+    HashMap<pid_t, Vec<DebugInfoPidMapping>>,
+) {
     debug!("Saving debug_info");
 
     let debug_info_by_elf_path = debug_info_by_path(mounted_modules_by_path);
 
-    // Pre-register debug info paths
     for path in debug_info_by_elf_path.keys() {
-        get_or_insert_key(&mut path_to_key, path);
+        get_or_insert_key(path_to_key, path);
     }
 
     let debug_info: HashMap<String, ModuleDebugInfo> = debug_info_by_elf_path
@@ -101,8 +108,7 @@ pub fn save_artifacts(
         })
         .collect();
 
-    let mut debug_info_pid_mappings_by_pid: HashMap<pid_t, Vec<DebugInfoPidMapping>> =
-        HashMap::new();
+    let mut mappings_by_pid: HashMap<pid_t, Vec<DebugInfoPidMapping>> = HashMap::new();
     for (path, m) in mounted_modules_by_path {
         if m.module_symbols.is_none() {
             continue;
@@ -112,7 +118,7 @@ pub fn save_artifacts(
         };
         for (&pid, pm) in &m.process_mounted_module {
             if let Some(load_bias) = pm.symbols_load_bias {
-                debug_info_pid_mappings_by_pid
+                mappings_by_pid
                     .entry(pid)
                     .or_default()
                     .push(DebugInfoPidMapping {
@@ -122,41 +128,43 @@ pub fn save_artifacts(
             }
         }
     }
-    for mappings in debug_info_pid_mappings_by_pid.values_mut() {
+    for mappings in mappings_by_pid.values_mut() {
         mappings.sort_by(|a, b| a.debug_info_key.cmp(&b.debug_info_key));
     }
 
-    // --- 3. Save unwind data files and build per-pid unwind data mappings ---
+    (debug_info, mappings_by_pid)
+}
+
+/// Save deduplicated unwind data files to disk and build per-pid mappings,
+/// including JIT unwind data.
+fn save_unwind_data(
+    profile_folder: &Path,
+    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>> {
     let unwind_data_count = mounted_modules_by_path
         .values()
         .filter(|m| m.unwind_data.is_some())
         .count();
     debug!("Saving unwind data ({unwind_data_count} unique entries)");
 
-    // Pre-register all paths for unwind data
-    for path in mounted_modules_by_path.keys() {
-        get_or_insert_key(&mut path_to_key, path);
-    }
-
-    // Save unwind data files in parallel
     mounted_modules_by_path.par_iter().for_each(|(path, m)| {
         if let Some(ref unwind_data) = m.unwind_data {
             let key = &path_to_key[path];
             unwind_data.save_to(profile_folder, key).unwrap();
         }
     });
 
-    // Build per-pid unwind data mappings from mounted modules
-    let mut unwind_data_pid_mappings_by_pid: HashMap<pid_t, Vec<UnwindDataPidMapping>> =
-        HashMap::new();
+    let mut mappings_by_pid: HashMap<pid_t, Vec<UnwindDataPidMapping>> = HashMap::new();
     for (path, m) in mounted_modules_by_path {
         if m.unwind_data.is_none() {
             continue;
         }
         let key = &path_to_key[path];
         for (&pid, pm) in &m.process_mounted_module {
             if let Some(ref pud) = pm.process_unwind_data {
-                unwind_data_pid_mappings_by_pid
+                mappings_by_pid
                     .entry(pid)
                     .or_default()
                     .push(UnwindDataPidMapping {
@@ -173,9 +181,9 @@ pub fn save_artifacts(
     for (&pid, jit_entries) in jit_unwind_data_by_pid {
         for (unwind_data, process_unwind_data) in jit_entries {
             let jit_path = PathBuf::from(&unwind_data.path);
-            let key = get_or_insert_key(&mut path_to_key, &jit_path);
+            let key = get_or_insert_key(path_to_key, &jit_path);
             unwind_data.save_to(profile_folder, &key).unwrap();
-            unwind_data_pid_mappings_by_pid
+            mappings_by_pid
                 .entry(pid)
                 .or_default()
                 .push(UnwindDataPidMapping {
@@ -187,27 +195,11 @@ pub fn save_artifacts(
         }
     }
 
-    for mappings in unwind_data_pid_mappings_by_pid.values_mut() {
+    for mappings in mappings_by_pid.values_mut() {
         mappings.sort_by(|a, b| a.unwind_data_key.cmp(&b.unwind_data_key));
     }
 
-    // --- 4. Collect ignored modules ---
-    let ignored_modules = collect_ignored_modules(mounted_modules_by_path);
-
-    // --- 5. Build key_to_path (inverted) ---
-    let key_to_path = path_to_key
-        .into_iter()
-        .map(|(path, key)| (key, path))
-        .collect();
-
-    SavedArtifacts {
-        symbol_pid_mappings_by_pid,
-        debug_info,
-        debug_info_pid_mappings_by_pid,
-        unwind_data_pid_mappings_by_pid,
-        ignored_modules,
-        key_to_path,
-    }
+    mappings_by_pid
 }
 
 /// Collect ignored modules by finding known-ignored and python modules in the mounted modules.
@@ -221,7 +213,9 @@ fn collect_ignored_modules(
     for (path, m) in mounted_modules_by_path {
         let path_str = path.to_string_lossy();
 
-        let is_ignored = ignore_paths.iter().any(|ip| path_str.as_ref() == ip.as_str());
+        let is_ignored = ignore_paths
+            .iter()
+            .any(|ip| path_str.as_ref() == ip.as_str());
         let is_python = path
             .file_name()
             .map(|name| name.to_string_lossy().starts_with("python"))
@@ -231,7 +225,6 @@ fn collect_ignored_modules(
             continue;
         }
 
-        // Collect address range across all pids for this module
         for pm in m.process_mounted_module.values() {
             if let Some(ref pud) = pm.process_unwind_data {
                 to_ignore.push((
@@ -245,3 +238,43 @@ fn collect_ignored_modules(
 
     to_ignore
 }
+
+/// Save all artifacts (symbols, debug info, unwind data) from mounted modules and JIT data.
+pub fn save_artifacts(
+    profile_folder: &Path,
+    mounted_modules_by_path: &HashMap<PathBuf, MountedModule>,
+    jit_unwind_data_by_pid: &HashMap<pid_t, Vec<(UnwindData, ProcessUnwindData)>>,
+) -> SavedArtifacts {
+    let mut path_to_key = HashMap::<PathBuf, String>::new();
+
+    register_paths(&mut path_to_key, mounted_modules_by_path);
+
+    let symbol_pid_mappings_by_pid =
+        save_symbols(profile_folder, mounted_modules_by_path, &path_to_key);
+
+    let (debug_info, debug_info_pid_mappings_by_pid) =
+        save_debug_info(mounted_modules_by_path, &mut path_to_key);
+
+    let unwind_data_pid_mappings_by_pid = save_unwind_data(
+        profile_folder,
+        mounted_modules_by_path,
+        jit_unwind_data_by_pid,
+        &mut path_to_key,
+    );
+
+    let ignored_modules = collect_ignored_modules(mounted_modules_by_path);
+
+    let key_to_path = path_to_key
+        .into_iter()
+        .map(|(path, key)| (key, path))
+        .collect();
+
+    SavedArtifacts {
+        symbol_pid_mappings_by_pid,
+        debug_info,
+        debug_info_pid_mappings_by_pid,
+        unwind_data_pid_mappings_by_pid,
+        ignored_modules,
+        key_to_path,
+    }
+}

src/executor/wall_time/perf/snapshots/codspeed_runner__executor__wall_time__perf__perf_map__tests__cpp_symbols.snap

@@ -3,7 +3,6 @@ source: src/executor/wall_time/perf/perf_map.rs
 expression: module_symbols
 ---
 ModuleSymbols {
-    load_bias: 0,
     symbols: [
         Symbol { offset: 0, size: 4002f8, name: __gmon_start__ },
         Symbol { offset: 4002f8, size: 20, name: __abi_tag },

src/executor/wall_time/perf/snapshots/codspeed_runner__executor__wall_time__perf__perf_map__tests__golang_symbols.snap

@@ -3,7 +3,6 @@ source: src/executor/wall_time/perf/perf_map.rs
 expression: module_symbols
 ---
 ModuleSymbols {
-    load_bias: 0,
     symbols: [
         Symbol { offset: 0, size: 8, name: runtime.tlsg },
         Symbol { offset: 0, size: 402000, name: seteuid },

src/executor/wall_time/perf/snapshots/codspeed_runner__executor__wall_time__perf__perf_map__tests__ruff_symbols.snap

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:b66645f8ee52271ff108bc73b69558cac3ed642c4ab9e101f99c4354b762f803
-size 5033521
+oid sha256:f65f0c08c8ee61c6fd4423c236845585d9ed6319a69beb3ad100a9f0537c8575
+size 5033490