refactor: add helper functions for artifacts saving

Author: GuillaumeLagrange

src/executor/wall_time/perf/mod.rs

@@ -10,33 +10,27 @@ use crate::executor::helpers::run_with_sudo::run_with_sudo;
 use crate::executor::helpers::run_with_sudo::wrap_with_sudo;
 use crate::executor::shared::fifo::FifoBenchmarkData;
 use crate::executor::shared::fifo::RunnerFifo;
-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::jit_dump::harvest_perf_jit_for_pids;
 use crate::executor::wall_time::perf::perf_executable::get_working_perf_executable;
 use crate::prelude::*;
 use anyhow::Context;
 use fifo::PerfFifo;
-use libc::pid_t;
 use parse_perf_file::MemmapRecordsOutput;
 use perf_executable::get_compression_flags;
 use perf_executable::get_event_flags;
-use rayon::prelude::*;
 use runner_shared::artifacts::ArtifactExt;
 use runner_shared::artifacts::ExecutionTimestamps;
-use runner_shared::debug_info::DebugInfoPidMapping;
 use runner_shared::fifo::Command as FifoCommand;
 use runner_shared::fifo::IntegrationMode;
 use runner_shared::metadata::PerfMetadata;
-use runner_shared::perf_map::SymbolPidMapping;
-use runner_shared::unwind_data::UnwindDataPidMapping;
 use std::path::Path;
 use std::path::PathBuf;
-use std::{cell::OnceCell, collections::HashMap, process::ExitStatus};
+use std::{cell::OnceCell, process::ExitStatus};
 
 mod jit_dump;
 mod naming;
 mod parse_perf_file;
+mod save_artifacts;
 mod setup;
 
 pub mod debug_info;
@@ -337,112 +331,19 @@ impl BenchmarkData {
                 .extend(mappings);
         }
 
-        // Assign a semantic key to each unique symbol path for on-disk filenames
-        debug!("Saving symbols ({} unique entries)", symbols_by_path.len());
-        let symbol_path_to_key: HashMap<&Path, String> = symbols_by_path
-            .keys()
-            .map(|path| {
-                (
-                    path.as_path(),
-                    naming::path_to_semantic_key(&path.to_string_lossy()),
-                )
-            })
-            .collect();
-
-        symbols_by_path.par_iter().for_each(|(path, module)| {
-            let key = &symbol_path_to_key[path.as_path()];
-            module.save_to(path_ref, key).unwrap();
-        });
-
-        let symbol_pid_mappings_by_pid: HashMap<i32, Vec<SymbolPidMapping>> =
-            pid_symbol_mappings_by_pid
-                .iter()
-                .map(|(pid, mappings)| {
-                    let converted = mappings
-                        .iter()
-                        .filter_map(|m| {
-                            Some(SymbolPidMapping {
-                                perf_map_key: symbol_path_to_key.get(m.path.as_path())?.clone(),
-                                load_bias: m.load_bias,
-                            })
-                        })
-                        .sorted_by_key(|m| m.perf_map_key.clone())
-                        .collect();
-                    (*pid, converted)
-                })
-                .collect();
-
-        // Collect debug info once per unique ELF path (deduplicated)
-        debug!("Saving debug_info");
-        let debug_info_by_elf_path = debug_info_by_path(&symbols_by_path);
-        let (debug_info, debug_info_path_to_key): (HashMap<String, _>, HashMap<PathBuf, String>) = {
-            let entries: Vec<_> = debug_info_by_elf_path.into_iter().collect();
-            let key_map: HashMap<PathBuf, String> = entries
-                .iter()
-                .map(|(path, _)| {
-                    (
-                        path.clone(),
-                        naming::path_to_semantic_key(&path.to_string_lossy()),
-                    )
-                })
-                .collect();
-            let infos: HashMap<String, _> = entries
-                .into_iter()
-                .map(|(path, info)| (naming::path_to_semantic_key(&path.to_string_lossy()), info))
-                .collect();
-            (infos, key_map)
-        };
-        let debug_info_pid_mappings_by_pid: HashMap<i32, Vec<DebugInfoPidMapping>> =
-            pid_symbol_mappings_by_pid
-                .iter()
-                .map(|(pid, mappings)| {
-                    let converted = mappings
-                        .iter()
-                        .filter_map(|m| {
-                            Some(DebugInfoPidMapping {
-                                debug_info_key: debug_info_path_to_key.get(&m.path)?.clone(),
-                                load_bias: m.load_bias,
-                            })
-                        })
-                        .sorted_by_key(|m| m.debug_info_key.clone())
-                        .collect();
-                    (*pid, converted)
-                })
-                .collect();
-
-        // Assign a semantic key to each unique path for on-disk filenames,
-        // then build the per-pid metadata referencing those keys.
-        debug!(
-            "Saving unwind data ({} unique entries)",
-            unwind_data_by_path.len()
-        );
-        let unwind_path_to_key: HashMap<&str, String> = unwind_data_by_path
-            .keys()
-            .map(|path| (path.as_str(), naming::path_to_semantic_key(path)))
-            .collect();
+        let symbol_pid_mappings_by_pid =
+            save_artifacts::save_symbols(path_ref, &symbols_by_path, &pid_symbol_mappings_by_pid);
 
-        unwind_data_by_path.par_iter().for_each(|(path, entry)| {
-            let key = &unwind_path_to_key[path.as_str()];
-            entry.save_to(path_ref, key).unwrap();
-        });
+        let (debug_info, debug_info_pid_mappings_by_pid) =
+            save_artifacts::save_debug_info(&symbols_by_path, &pid_symbol_mappings_by_pid);
 
-        let unwind_data_pid_mappings_by_pid: HashMap<pid_t, Vec<UnwindDataPidMapping>> =
-            pid_unwind_data_mappings_by_pid
-                .into_iter()
-                .map(|(pid, mappings)| {
-                    let converted = mappings
-                        .into_iter()
-                        .map(|m| UnwindDataPidMapping {
-                            unwind_data_key: unwind_path_to_key[m.path.as_str()].clone(),
-                            timestamp: m.timestamp,
-                            avma_range: m.avma_range,
-                            base_avma: m.base_avma,
-                        })
-                        .sorted_by_key(|m| m.unwind_data_key.clone())
-                        .collect();
-                    (pid, converted)
-                })
-                .collect();
+        let unwind_data_pid_mappings_by_pid = save_artifacts::save_unwind_data(
+            path_ref,
+            &unwind_data_by_path,
+            pid_unwind_data_mappings_by_pid,
+        );
+
+        let ignored_modules = save_artifacts::collect_ignored_modules(&pid_symbol_mappings_by_pid);
 
         debug!("Saving metadata");
         #[allow(deprecated)]
@@ -454,49 +355,7 @@ impl BenchmarkData {
                 .clone()
                 .ok_or(BenchmarkDataSaveError::MissingIntegration)?,
             uri_by_ts: self.marker_result.uri_by_ts.clone(),
-            ignored_modules: {
-                let mut to_ignore = vec![];
-
-                // Check if any of the ignored modules has been loaded in the process
-                for ignore_path in get_objects_path_to_ignore() {
-                    let ignore_pathbuf = PathBuf::from(&ignore_path);
-                    for pid_mappings in pid_symbol_mappings_by_pid.values() {
-                        if let Some(m) = pid_mappings.iter().find(|m| m.path == ignore_pathbuf) {
-                            let (Some(&(base_addr, _)), Some(&(_, end_addr))) = (
-                                m.mappings.iter().min_by_key(|(base, _)| base),
-                                m.mappings.iter().max_by_key(|(_, end)| end),
-                            ) else {
-                                continue;
-                            };
-
-                            to_ignore.push((ignore_path.clone(), base_addr, end_addr));
-                        }
-                    }
-                }
-
-                // When python is statically linked, we'll not find it in the ignored modules. Add it manually:
-                for pid_mappings in pid_symbol_mappings_by_pid.values() {
-                    for m in pid_mappings {
-                        let is_python = m
-                            .path
-                            .file_name()
-                            .map(|name| name.to_string_lossy().starts_with("python"))
-                            .unwrap_or(false);
-                        if !is_python {
-                            continue;
-                        }
-                        let (Some(&(base_addr, _)), Some(&(_, end_addr))) = (
-                            m.mappings.iter().min_by_key(|(base, _)| base),
-                            m.mappings.iter().max_by_key(|(_, end)| end),
-                        ) else {
-                            continue;
-                        };
-                        to_ignore.push((m.path.to_string_lossy().into(), base_addr, end_addr));
-                    }
-                }
-
-                to_ignore
-            },
+            ignored_modules,
             markers: self.marker_result.markers.clone(),
             debug_info,
             debug_info_pid_mappings_by_pid,