feat: get rid of path hashing in favor of an index-based shared map

Author: GuillaumeLagrange

crates/runner-shared/src/metadata.rs

@@ -3,6 +3,7 @@ use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::io::BufWriter;
 use std::path::Path;
+use std::path::PathBuf;
 
 use crate::debug_info::{DebugInfoPidMapping, ModuleDebugInfo};
 use crate::fifo::MarkerType;
@@ -48,6 +49,11 @@ pub struct PerfMetadata {
     /// Per-pid symbol references, mapping PID to list of perf map index + load bias
     #[serde(default, skip_serializing_if = "HashMap::is_empty")]
     pub symbol_pid_mappings_by_pid: HashMap<i32, Vec<SymbolPidMapping>>,
+
+    /// Mapping from semantic key to original binary path
+    /// Kept for traceability, and if we ever need to reconstruct the original paths from the keys
+    #[serde(default, skip_serializing_if = "HashMap::is_empty")]
+    pub key_to_path: HashMap<String, PathBuf>,
 }
 
 impl PerfMetadata {

src/executor/wall_time/perf/jit_dump.rs

@@ -53,7 +53,7 @@ impl JitDump {
     pub fn into_unwind_data(
         self,
     ) -> Result<(
-        HashMap<String, UnwindData>,
+        HashMap<PathBuf, UnwindData>,
         Vec<UnwindDataPidMappingWithFullPath>,
     )> {
         let file = std::fs::File::open(self.path)?;
@@ -80,11 +80,12 @@ impl JitDump {
                         continue;
                     };
 
-                    let path = format!("jit_{name}");
+                    let path_string = format!("jit_{name}");
+                    let path = PathBuf::from(&path_string);
                     unwind_data_by_path.insert(
                         path.clone(),
                         UnwindData {
-                            path: path.clone(),
+                            path: path_string,
                             base_svma: 0,
                             eh_frame_hdr,
                             eh_frame_hdr_svma: 0..0,
@@ -129,7 +130,7 @@ pub async fn harvest_perf_jit_for_pids(
 ) -> Result<(
     HashMap<PathBuf, ModuleSymbols>,
     HashMap<i32, Vec<SymbolPidMappingWithFullPath>>,
-    HashMap<String, UnwindData>,
+    HashMap<PathBuf, UnwindData>,
     HashMap<i32, Vec<UnwindDataPidMappingWithFullPath>>,
 )> {
     let mut all_symbols_by_path = HashMap::new();

src/executor/wall_time/perf/mod.rs

@@ -331,16 +331,26 @@ impl BenchmarkData {
                 .extend(mappings);
         }
 
-        let symbol_pid_mappings_by_pid =
-            save_artifacts::save_symbols(path_ref, &symbols_by_path, &pid_symbol_mappings_by_pid);
+        let mut path_to_key = std::collections::HashMap::<PathBuf, String>::new();
 
-        let (debug_info, debug_info_pid_mappings_by_pid) =
-            save_artifacts::save_debug_info(&symbols_by_path, &pid_symbol_mappings_by_pid);
+        let symbol_pid_mappings_by_pid = save_artifacts::save_symbols(
+            path_ref,
+            &symbols_by_path,
+            &pid_symbol_mappings_by_pid,
+            &mut path_to_key,
+        );
+
+        let (debug_info, debug_info_pid_mappings_by_pid) = save_artifacts::save_debug_info(
+            &symbols_by_path,
+            &pid_symbol_mappings_by_pid,
+            &mut path_to_key,
+        );
 
         let unwind_data_pid_mappings_by_pid = save_artifacts::save_unwind_data(
             path_ref,
             &unwind_data_by_path,
             pid_unwind_data_mappings_by_pid,
+            &mut path_to_key,
         );
 
         let ignored_modules = save_artifacts::collect_ignored_modules(&pid_symbol_mappings_by_pid);
@@ -361,6 +371,10 @@ impl BenchmarkData {
             debug_info_pid_mappings_by_pid,
             unwind_data_pid_mappings_by_pid,
             symbol_pid_mappings_by_pid,
+            key_to_path: path_to_key
+                .into_iter()
+                .map(|(path, key)| (key, path))
+                .collect(),
             debug_info_by_pid: Default::default(), // No longer used
         };
         metadata.save_to(&path).unwrap();

src/executor/wall_time/perf/naming.rs

@@ -1,17 +1,14 @@
-use std::hash::{DefaultHasher, Hash, Hasher};
+use std::path::Path;
 
-/// Convert a full path to a semantic key suitable for filenames.
+/// Build a semantic key from a global index and a path.
 ///
-/// The key is `{basename}-{hash:08x}` where `basename` is the last component
-/// of the path and `hash` is a truncated hash of the full path. This gives
-/// human-readable filenames while still being unique when multiple libraries
-/// share the same basename.
-pub fn path_to_semantic_key(path: &str) -> String {
-    let basename = path.rsplit('/').next().unwrap_or(path);
-    let mut hasher = DefaultHasher::new();
-    path.hash(&mut hasher);
-    let hash = hasher.finish() as u32;
-    format!("{basename}-{hash:08x}")
+/// The key is `{index}__{basename}` where `basename` is the last component
+/// of the path. The index ensures uniqueness across all artifact types.
+pub fn indexed_semantic_key(index: usize, path: &Path) -> String {
+    format!(
+        "{index}__{}",
+        path.file_name().unwrap_or_default().to_string_lossy()
+    )
 }
 
 #[cfg(test)]
@@ -20,39 +17,26 @@ mod tests {
 
     #[test]
     fn test_normal_path() {
-        let key = path_to_semantic_key("/usr/lib/libc.so.6");
-        assert!(key.starts_with("libc.so.6-"));
-        assert_eq!(key.len(), "libc.so.6-".len() + 8);
+        let key = indexed_semantic_key(0, Path::new("/usr/lib/libc.so.6"));
+        assert_eq!(key, "0__libc.so.6");
     }
 
     #[test]
     fn test_jit_path() {
-        let key = path_to_semantic_key("/tmp/jit-12345.so");
-        assert!(key.starts_with("jit-12345.so-"));
-        assert_eq!(key.len(), "jit-12345.so-".len() + 8);
+        let key = indexed_semantic_key(5, Path::new("/tmp/jit-12345.so"));
+        assert_eq!(key, "5__jit-12345.so");
     }
 
     #[test]
     fn test_same_basename_different_paths() {
-        let key1 = path_to_semantic_key("/usr/lib/libc.so.6");
-        let key2 = path_to_semantic_key("/opt/lib/libc.so.6");
-        // Both start with the same basename
-        assert!(key1.starts_with("libc.so.6-"));
-        assert!(key2.starts_with("libc.so.6-"));
-        // But have different hashes
+        let key1 = indexed_semantic_key(0, Path::new("/usr/lib/libc.so.6"));
+        let key2 = indexed_semantic_key(1, Path::new("/opt/lib/libc.so.6"));
         assert_ne!(key1, key2);
     }
 
     #[test]
     fn test_bare_filename() {
-        let key = path_to_semantic_key("libfoo.so");
-        assert!(key.starts_with("libfoo.so-"));
-    }
-
-    #[test]
-    fn test_deterministic() {
-        let key1 = path_to_semantic_key("/usr/lib/libc.so.6");
-        let key2 = path_to_semantic_key("/usr/lib/libc.so.6");
-        assert_eq!(key1, key2);
+        let key = indexed_semantic_key(3, Path::new("libfoo.so"));
+        assert_eq!(key, "3__libfoo.so");
     }
 }

src/executor/wall_time/perf/parse_perf_file.rs

@@ -15,7 +15,7 @@ use std::path::PathBuf;
 /// Per-pid mounting info for a specific ELF, referencing it by path.
 #[derive(Debug)]
 pub struct UnwindDataPidMappingWithFullPath {
-    pub path: String,
+    pub path: PathBuf,
     pub timestamp: Option<u64>,
     pub avma_range: std::ops::Range<u64>,
     pub base_avma: u64,
@@ -35,7 +35,7 @@ pub struct MemmapRecordsOutput {
     /// Per-pid mounting info referencing symbols by path
     pub pid_symbol_mappings_by_pid: HashMap<pid_t, Vec<SymbolPidMappingWithFullPath>>,
     /// Deduplicated unwind data keyed by ELF path
-    pub unwind_data_by_path: HashMap<String, UnwindData>,
+    pub unwind_data_by_path: HashMap<PathBuf, UnwindData>,
     /// Per-pid mounting info referencing unwind data by path
     pub pid_unwind_data_mappings_by_pid: HashMap<pid_t, Vec<UnwindDataPidMappingWithFullPath>>,
     pub tracked_pids: HashSet<pid_t>,
@@ -51,7 +51,7 @@ pub fn parse_for_memmap2<P: AsRef<Path>>(
 ) -> Result<MemmapRecordsOutput> {
     let mut symbols_by_path = HashMap::<PathBuf, ModuleSymbols>::new();
     let mut symbol_mappings_by_pid = HashMap::<pid_t, Vec<SymbolPidMappingWithFullPath>>::new();
-    let mut unwind_data_by_path = HashMap::<String, UnwindData>::new();
+    let mut unwind_data_by_path = HashMap::<PathBuf, UnwindData>::new();
     let mut unwind_mappings_by_pid = HashMap::<pid_t, Vec<UnwindDataPidMappingWithFullPath>>::new();
 
     // 1MiB buffer
@@ -170,7 +170,7 @@ fn process_mmap2_record(
     record: linux_perf_data::linux_perf_event_reader::Mmap2Record,
     symbols_by_path: &mut HashMap<PathBuf, ModuleSymbols>,
     symbol_mappings_by_pid: &mut HashMap<pid_t, Vec<SymbolPidMappingWithFullPath>>,
-    unwind_data_by_path: &mut HashMap<String, UnwindData>,
+    unwind_data_by_path: &mut HashMap<PathBuf, UnwindData>,
     unwind_mappings_by_pid: &mut HashMap<pid_t, Vec<UnwindDataPidMappingWithFullPath>>,
 ) {
     // Check PROT_EXEC early to avoid string allocation for non-executable mappings
@@ -257,9 +257,7 @@ fn process_mmap2_record(
     ) {
         Ok((v3, mapping)) => {
             // Store pid-agnostic data once per path
-            unwind_data_by_path
-                .entry(record_path_string.clone())
-                .or_insert(v3);
+            unwind_data_by_path.entry(record_path.clone()).or_insert(v3);
 
             // Always store per-pid mounting info
             unwind_mappings_by_pid

src/executor/wall_time/perf/save_artifacts.rs

@@ -12,23 +12,38 @@ use std::path::{Path, PathBuf};
 
 use super::parse_perf_file::{SymbolPidMappingWithFullPath, UnwindDataPidMappingWithFullPath};
 
+/// Register a path in the map if absent, assigning a new unique key.
+/// Returns the assigned key.
+fn get_or_insert_key(path_to_key: &mut HashMap<PathBuf, String>, path: &Path) -> String {
+    if let Some(key) = path_to_key.get(path) {
+        return key.clone();
+    }
+    let key = naming::indexed_semantic_key(path_to_key.len(), path);
+    path_to_key.insert(path.to_owned(), key.clone());
+    key
+}
+
 /// Save deduplicated symbol files to disk and build per-pid mappings using semantic keys.
 pub fn save_symbols(
     profile_folder: &Path,
     symbols_by_path: &HashMap<PathBuf, ModuleSymbols>,
     pid_symbol_mappings_by_pid: &HashMap<i32, Vec<SymbolPidMappingWithFullPath>>,
+    path_to_key: &mut HashMap<PathBuf, String>,
 ) -> HashMap<i32, Vec<SymbolPidMapping>> {
     debug!("Saving symbols ({} unique entries)", symbols_by_path.len());
 
-    let path_to_key = build_path_to_key_map(symbols_by_path.keys().map(|p| p.to_string_lossy()));
+    // Pre-register all paths so the parallel section can use immutable borrows
+    for path in symbols_by_path.keys() {
+        get_or_insert_key(path_to_key, path);
+    }
 
     symbols_by_path.par_iter().for_each(|(path, module)| {
-        let key = &path_to_key[&*path.to_string_lossy()];
+        let key = &path_to_key[path];
         module.save_to(profile_folder, key).unwrap();
     });
 
     convert_pid_mappings(pid_symbol_mappings_by_pid, |m| {
-        let key = path_to_key.get(&*m.path.to_string_lossy())?.clone();
+        let key = path_to_key.get(&m.path)?.clone();
         Some((
             key.clone(),
             SymbolPidMapping {
@@ -43,6 +58,7 @@ pub fn save_symbols(
 pub fn save_debug_info(
     symbols_by_path: &HashMap<PathBuf, ModuleSymbols>,
     pid_symbol_mappings_by_pid: &HashMap<i32, Vec<SymbolPidMappingWithFullPath>>,
+    path_to_key: &mut HashMap<PathBuf, String>,
 ) -> (
     HashMap<String, ModuleDebugInfo>,
     HashMap<i32, Vec<DebugInfoPidMapping>>,
@@ -51,16 +67,21 @@ pub fn save_debug_info(
 
     let debug_info_by_elf_path = debug_info_by_path(symbols_by_path);
 
-    let path_to_key =
-        build_path_to_key_map(debug_info_by_elf_path.keys().map(|p| p.to_string_lossy()));
+    // Pre-register all paths so the closures below can use immutable borrows
+    for path in debug_info_by_elf_path.keys() {
+        get_or_insert_key(path_to_key, path);
+    }
 
     let debug_info: HashMap<String, ModuleDebugInfo> = debug_info_by_elf_path
         .into_iter()
-        .map(|(path, info)| (path_to_key[&*path.to_string_lossy()].clone(), info))
+        .filter_map(|(path, info)| {
+            let key = path_to_key.get(&path)?.clone();
+            Some((key, info))
+        })
         .collect();
 
     let pid_mappings = convert_pid_mappings(pid_symbol_mappings_by_pid, |m| {
-        let key = path_to_key.get(&*m.path.to_string_lossy())?.clone();
+        let key = path_to_key.get(&m.path)?.clone();
         Some((
             key.clone(),
             DebugInfoPidMapping {
@@ -76,18 +97,22 @@ pub fn save_debug_info(
 /// Save deduplicated unwind data files to disk and build per-pid mappings using semantic keys.
 pub fn save_unwind_data(
     profile_folder: &Path,
-    unwind_data_by_path: &HashMap<String, UnwindData>,
+    unwind_data_by_path: &HashMap<PathBuf, UnwindData>,
     pid_unwind_data_mappings_by_pid: HashMap<i32, Vec<UnwindDataPidMappingWithFullPath>>,
+    path_to_key: &mut HashMap<PathBuf, String>,
 ) -> HashMap<i32, Vec<UnwindDataPidMapping>> {
     debug!(
         "Saving unwind data ({} unique entries)",
         unwind_data_by_path.len()
     );
 
-    let path_to_key = build_path_to_key_map(unwind_data_by_path.keys().map(String::as_str));
+    // Pre-register all paths so the parallel section can use immutable borrows
+    for path in unwind_data_by_path.keys() {
+        get_or_insert_key(path_to_key, path);
+    }
 
     unwind_data_by_path.par_iter().for_each(|(path, entry)| {
-        let key = &path_to_key[path.as_str()];
+        let key = &path_to_key[path];
         entry.save_to(profile_folder, key).unwrap();
     });
 
@@ -97,7 +122,7 @@ pub fn save_unwind_data(
             let converted = mappings
                 .into_iter()
                 .map(|m| UnwindDataPidMapping {
-                    unwind_data_key: path_to_key[m.path.as_str()].clone(),
+                    unwind_data_key: path_to_key[&m.path].clone(),
                     timestamp: m.timestamp,
                     avma_range: m.avma_range,
                     base_avma: m.base_avma,
@@ -173,18 +198,3 @@ fn convert_pid_mappings<T, K: Ord>(
         })
         .collect()
 }
-
-/// Build a `path_string → semantic_key` map from an iterator of path-like items.
-fn build_path_to_key_map<I, S>(paths: I) -> HashMap<String, String>
-where
-    I: Iterator<Item = S>,
-    S: AsRef<str>,
-{
-    paths
-        .map(|p| {
-            let s = p.as_ref().to_owned();
-            let key = naming::path_to_semantic_key(&s);
-            (s, key)
-        })
-        .collect()
-}

src/executor/wall_time/perf/unwind_data.rs

@@ -7,6 +7,7 @@ use object::Object;
 use object::ObjectSection;
 use runner_shared::unwind_data::UnwindData;
 use std::ops::Range;
+use std::path::PathBuf;
 
 use super::parse_perf_file::UnwindDataPidMappingWithFullPath;
 
@@ -84,7 +85,7 @@ pub fn unwind_data_from_elf(
     };
 
     let mapping = UnwindDataPidMappingWithFullPath {
-        path,
+        path: PathBuf::from(&path),
         timestamp: None,
         avma_range,
         base_avma,