From a8e339a7b088e7cc4db30f560616af5533dd3f30 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sun, 29 Mar 2026 21:31:50 +0200 Subject: [PATCH 1/7] feat: search LLVM ORC JIT jitdump paths The jitdump search was hardcoded to /tmp/jit-{pid}.dump, which misses jitdumps from LLVM ORC JIT (e.g. revmc, Julia) that write to {JITDUMPDIR|HOME}/.debug/jit/*/jit-{pid}.dump per JITLoaderPerf.cpp. --- src/executor/wall_time/perf/jit_dump.rs | 84 ++++++++++++++++++------- 1 file changed, 62 insertions(+), 22 deletions(-) diff --git a/src/executor/wall_time/perf/jit_dump.rs b/src/executor/wall_time/perf/jit_dump.rs index f306de08d..c8502d3a5 100644 --- a/src/executor/wall_time/perf/jit_dump.rs +++ b/src/executor/wall_time/perf/jit_dump.rs @@ -108,6 +108,43 @@ impl JitDump { } } +/// Finds all jitdump file paths for a given PID. +/// +/// Searches in order: +/// 1. `/tmp/jit-{pid}.dump` (standard perf jitdump location) +/// 2. `{jitdumpdir}/.debug/jit/*/jit-{pid}.dump` where `jitdumpdir` is `$JITDUMPDIR` or `$HOME` +/// (LLVM ORC JIT / PerfSupportPlugin location) +fn find_jit_dumps_for_pid(pid: libc::pid_t) -> Vec { + let name = format!("jit-{pid}.dump"); + let mut paths = Vec::new(); + + // Standard perf location. + let tmp_path = PathBuf::from("/tmp").join(&name); + if tmp_path.exists() { + paths.push(tmp_path); + } + + // LLVM ORC JIT location: {base}/.debug/jit/*/jit-{pid}.dump + // See LLVM's JITLoaderPerf.cpp for the path construction logic. + let base_dir = std::env::var("JITDUMPDIR") + .or_else(|_| std::env::var("HOME")) + .ok() + .map(PathBuf::from); + if let Some(base) = base_dir { + let jit_dir = base.join(".debug/jit"); + if let Ok(entries) = std::fs::read_dir(&jit_dir) { + for entry in entries.filter_map(|e| e.ok()) { + let candidate = entry.path().join(&name); + if candidate.exists() { + paths.push(candidate); + } + } + } + } + + paths +} + /// Converts all the `jit-.dump` into a perf-.map with symbols, and collects the unwind data /// /// # Symbols @@ -123,34 +160,37 @@ pub async fn save_symbols_and_harvest_unwind_data_for_pids( let mut jit_unwind_data_by_path = HashMap::new(); for pid in pids { - let name = format!("jit-{pid}.dump"); - let path = PathBuf::from("/tmp").join(&name); - - if !path.exists() { + let paths = find_jit_dumps_for_pid(*pid); + if paths.is_empty() { continue; } - debug!("Found JIT dump file: {path:?}"); - let symbols = match JitDump::new(path.clone()).into_perf_map() { - Ok(symbols) => symbols, - Err(error) => { - warn!("Failed to convert jit dump into perf map: {error:?}"); - continue; - } - }; + for path in paths { + debug!("Found JIT dump file: {path:?}"); - // Also write to perf-.map for harvested Python perf maps compatibility - symbols.append_to_file(profile_folder.join(format!("perf-{pid}.map")))?; + let symbols = match JitDump::new(path.clone()).into_perf_map() { + Ok(symbols) => symbols, + Err(error) => { + warn!("Failed to convert jit dump into perf map: {error:?}"); + continue; + } + }; - let jit_unwind_data = match JitDump::new(path).into_unwind_data() { - Ok(data) => data, - Err(error) => { - warn!("Failed to convert jit dump into unwind data: {error:?}"); - continue; - } - }; + symbols.append_to_file(profile_folder.join(format!("perf-{pid}.map")))?; - jit_unwind_data_by_path.insert(*pid, jit_unwind_data); + let jit_unwind_data = match JitDump::new(path).into_unwind_data() { + Ok(data) => data, + Err(error) => { + warn!("Failed to convert jit dump into unwind data: {error:?}"); + continue; + } + }; + + jit_unwind_data_by_path + .entry(*pid) + .or_insert_with(Vec::new) + .extend(jit_unwind_data); + } } Ok(jit_unwind_data_by_path) From eaf949b810a7b1c95589ed334025e601d53e9486 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sun, 29 Mar 2026 22:12:30 +0200 Subject: [PATCH 2/7] refactor: discover jitdump paths from MMAP2 records Instead of searching hardcoded filesystem paths, extract jitdump file paths from MMAP2 records in the perf data. This is how perf inject finds them and works for any jitdump location. --- src/executor/wall_time/perf/jit_dump.rs | 59 ++++--------------- src/executor/wall_time/perf/mod.rs | 17 +++--- .../wall_time/perf/parse_perf_file.rs | 19 ++++++ 3 files changed, 39 insertions(+), 56 deletions(-) diff --git a/src/executor/wall_time/perf/jit_dump.rs b/src/executor/wall_time/perf/jit_dump.rs index c8502d3a5..273850062 100644 --- a/src/executor/wall_time/perf/jit_dump.rs +++ b/src/executor/wall_time/perf/jit_dump.rs @@ -5,7 +5,7 @@ use crate::{ use linux_perf_data::jitdump::{JitDumpReader, JitDumpRecord}; use runner_shared::unwind_data::{ProcessUnwindData, UnwindData}; use std::{ - collections::{HashMap, HashSet}, + collections::HashMap, path::{Path, PathBuf}, }; @@ -108,44 +108,10 @@ impl JitDump { } } -/// Finds all jitdump file paths for a given PID. +/// Converts all the `jit-.dump` into a perf-.map with symbols, and collects the unwind data. /// -/// Searches in order: -/// 1. `/tmp/jit-{pid}.dump` (standard perf jitdump location) -/// 2. `{jitdumpdir}/.debug/jit/*/jit-{pid}.dump` where `jitdumpdir` is `$JITDUMPDIR` or `$HOME` -/// (LLVM ORC JIT / PerfSupportPlugin location) -fn find_jit_dumps_for_pid(pid: libc::pid_t) -> Vec { - let name = format!("jit-{pid}.dump"); - let mut paths = Vec::new(); - - // Standard perf location. - let tmp_path = PathBuf::from("/tmp").join(&name); - if tmp_path.exists() { - paths.push(tmp_path); - } - - // LLVM ORC JIT location: {base}/.debug/jit/*/jit-{pid}.dump - // See LLVM's JITLoaderPerf.cpp for the path construction logic. - let base_dir = std::env::var("JITDUMPDIR") - .or_else(|_| std::env::var("HOME")) - .ok() - .map(PathBuf::from); - if let Some(base) = base_dir { - let jit_dir = base.join(".debug/jit"); - if let Ok(entries) = std::fs::read_dir(&jit_dir) { - for entry in entries.filter_map(|e| e.ok()) { - let candidate = entry.path().join(&name); - if candidate.exists() { - paths.push(candidate); - } - } - } - } - - paths -} - -/// Converts all the `jit-.dump` into a perf-.map with symbols, and collects the unwind data +/// Jitdump file paths are discovered from MMAP2 records in the perf data, since JIT runtimes +/// mmap the jitdump file and perf records the mapping with the actual path on disk. /// /// # Symbols /// Since a jit dump is by definition specific to a single pid, we append the harvested symbols @@ -155,16 +121,11 @@ fn find_jit_dumps_for_pid(pid: libc::pid_t) -> Vec { /// Unwind data is generated as a list pub async fn save_symbols_and_harvest_unwind_data_for_pids( profile_folder: &Path, - pids: &HashSet, + jit_dump_paths_by_pid: &HashMap>, ) -> Result>> { - let mut jit_unwind_data_by_path = HashMap::new(); - - for pid in pids { - let paths = find_jit_dumps_for_pid(*pid); - if paths.is_empty() { - continue; - } + let mut jit_unwind_data_by_pid = HashMap::new(); + for (pid, paths) in jit_dump_paths_by_pid { for path in paths { debug!("Found JIT dump file: {path:?}"); @@ -178,7 +139,7 @@ pub async fn save_symbols_and_harvest_unwind_data_for_pids( symbols.append_to_file(profile_folder.join(format!("perf-{pid}.map")))?; - let jit_unwind_data = match JitDump::new(path).into_unwind_data() { + let jit_unwind_data = match JitDump::new(path.clone()).into_unwind_data() { Ok(data) => data, Err(error) => { warn!("Failed to convert jit dump into unwind data: {error:?}"); @@ -186,12 +147,12 @@ pub async fn save_symbols_and_harvest_unwind_data_for_pids( } }; - jit_unwind_data_by_path + jit_unwind_data_by_pid .entry(*pid) .or_insert_with(Vec::new) .extend(jit_unwind_data); } } - Ok(jit_unwind_data_by_path) + Ok(jit_unwind_data_by_pid) } diff --git a/src/executor/wall_time/perf/mod.rs b/src/executor/wall_time/perf/mod.rs index c854d49ae..ab8674ae1 100644 --- a/src/executor/wall_time/perf/mod.rs +++ b/src/executor/wall_time/perf/mod.rs @@ -300,6 +300,7 @@ impl BenchmarkData { let MemmapRecordsOutput { loaded_modules_by_path, tracked_pids, + jit_dump_paths_by_pid, } = { parse_perf_file::parse_for_memmap2(perf_file_path, pid_filter).map_err(|e| { error!("Failed to parse perf file: {e}"); @@ -317,13 +318,15 @@ impl BenchmarkData { error!("Failed to harvest perf maps: {e}"); BenchmarkDataSaveError::FailedToHarvestPerfMaps })?; - let jit_unwind_data_by_pid = - jit_dump::save_symbols_and_harvest_unwind_data_for_pids(path_ref, &tracked_pids) - .await - .map_err(|e| { - error!("Failed to harvest jit dumps: {e}"); - BenchmarkDataSaveError::FailedToHarvestJitDumps - })?; + let jit_unwind_data_by_pid = jit_dump::save_symbols_and_harvest_unwind_data_for_pids( + path_ref, + &jit_dump_paths_by_pid, + ) + .await + .map_err(|e| { + error!("Failed to harvest jit dumps: {e}"); + BenchmarkDataSaveError::FailedToHarvestJitDumps + })?; let artifacts = save_artifacts::save_artifacts( path_ref, diff --git a/src/executor/wall_time/perf/parse_perf_file.rs b/src/executor/wall_time/perf/parse_perf_file.rs index 9f5b0fd8d..2378167dc 100644 --- a/src/executor/wall_time/perf/parse_perf_file.rs +++ b/src/executor/wall_time/perf/parse_perf_file.rs @@ -42,6 +42,8 @@ pub struct MemmapRecordsOutput { /// Module symbols and the computed load bias for each pid that maps the ELF path. pub loaded_modules_by_path: HashMap, pub tracked_pids: HashSet, + /// Jitdump file paths discovered from MMAP2 records, keyed by PID. + pub jit_dump_paths_by_pid: HashMap>, } /// Parse the perf file at `perf_file_path` and look for MMAP2 records for the given `pids`. @@ -53,6 +55,7 @@ pub fn parse_for_memmap2>( mut pid_filter: PidFilter, ) -> Result { let mut loaded_modules_by_path = HashMap::::new(); + let mut jit_dump_paths_by_pid = HashMap::>::new(); // 1MiB buffer let reader = std::io::BufReader::with_capacity( @@ -105,6 +108,21 @@ pub fn parse_for_memmap2>( continue; } + // Collect jitdump file paths before the PROT_EXEC filter in process_mmap2_record + // skips them. JIT runtimes mmap the jitdump file so perf records it. + if mmap2_record.path.as_slice().ends_with(b".dump") { + let path = PathBuf::from( + String::from_utf8_lossy(&mmap2_record.path.as_slice()).into_owned(), + ); + if path.exists() { + debug!("Found jitdump path from MMAP2 record: {path:?}"); + jit_dump_paths_by_pid + .entry(mmap2_record.pid) + .or_default() + .push(path); + } + } + process_mmap2_record(mmap2_record, &mut loaded_modules_by_path); } _ => continue, @@ -123,6 +141,7 @@ pub fn parse_for_memmap2>( Ok(MemmapRecordsOutput { loaded_modules_by_path, tracked_pids, + jit_dump_paths_by_pid, }) } From fc2b18e91c3059253eac70fcf9ffda40fc058197 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sun, 29 Mar 2026 22:14:27 +0200 Subject: [PATCH 3/7] fix: match perf's jit_detect() pattern for jitdump paths Only match basenames of the form jit-.dump instead of any .dump suffix. This matches the validation in perf's jit_detect(). --- .../wall_time/perf/parse_perf_file.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/executor/wall_time/perf/parse_perf_file.rs b/src/executor/wall_time/perf/parse_perf_file.rs index 2378167dc..94615f224 100644 --- a/src/executor/wall_time/perf/parse_perf_file.rs +++ b/src/executor/wall_time/perf/parse_perf_file.rs @@ -110,7 +110,8 @@ pub fn parse_for_memmap2>( // Collect jitdump file paths before the PROT_EXEC filter in process_mmap2_record // skips them. JIT runtimes mmap the jitdump file so perf records it. - if mmap2_record.path.as_slice().ends_with(b".dump") { + // Match perf's jit_detect(): basename must be `jit-.dump`. + if is_jit_dump_path(&mmap2_record.path.as_slice()) { let path = PathBuf::from( String::from_utf8_lossy(&mmap2_record.path.as_slice()).into_owned(), ); @@ -179,6 +180,23 @@ impl PidFilter { } } +/// Returns true if the path basename matches perf's jitdump pattern: `jit-.dump`. +fn is_jit_dump_path(path: &[u8]) -> bool { + let basename = match path.iter().rposition(|&b| b == b'/') { + Some(pos) => &path[pos + 1..], + None => return false, + }; + let rest = match basename.strip_prefix(b"jit-") { + Some(rest) => rest, + None => return false, + }; + let rest = match rest.strip_suffix(b".dump") { + Some(rest) => rest, + None => return false, + }; + !rest.is_empty() && rest.iter().all(|b| b.is_ascii_digit()) +} + /// Process a single MMAP2 record and add it to the symbols and unwind data maps fn process_mmap2_record( record: linux_perf_data::linux_perf_event_reader::Mmap2Record, From 92ff4b6100899b8ef4507e04428c051a1743f7a2 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sun, 29 Mar 2026 22:29:49 +0200 Subject: [PATCH 4/7] fix: validate jitdump filename PID matches MMAP2 record PID Match perf's jit_detect() behavior: the PID embedded in the jitdump filename must match the MMAP2 record's PID. Also fixes a bug where rposition returned a usize that was incorrectly used as a byte slice. --- .../wall_time/perf/parse_perf_file.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/executor/wall_time/perf/parse_perf_file.rs b/src/executor/wall_time/perf/parse_perf_file.rs index 94615f224..2e0195a11 100644 --- a/src/executor/wall_time/perf/parse_perf_file.rs +++ b/src/executor/wall_time/perf/parse_perf_file.rs @@ -111,7 +111,7 @@ pub fn parse_for_memmap2>( // Collect jitdump file paths before the PROT_EXEC filter in process_mmap2_record // skips them. JIT runtimes mmap the jitdump file so perf records it. // Match perf's jit_detect(): basename must be `jit-.dump`. - if is_jit_dump_path(&mmap2_record.path.as_slice()) { + if is_jit_dump_path(&mmap2_record.path.as_slice(), mmap2_record.pid) { let path = PathBuf::from( String::from_utf8_lossy(&mmap2_record.path.as_slice()).into_owned(), ); @@ -180,21 +180,15 @@ impl PidFilter { } } -/// Returns true if the path basename matches perf's jitdump pattern: `jit-.dump`. -fn is_jit_dump_path(path: &[u8]) -> bool { - let basename = match path.iter().rposition(|&b| b == b'/') { - Some(pos) => &path[pos + 1..], - None => return false, +/// Returns true if the path basename matches perf's `jit_detect()` pattern: `jit-.dump`, +/// where `` must match the MMAP2 record's PID. +fn is_jit_dump_path(path: &[u8], pid: pid_t) -> bool { + let Some(pos) = path.iter().rposition(|&b| b == b'/') else { + return false; }; - let rest = match basename.strip_prefix(b"jit-") { - Some(rest) => rest, - None => return false, - }; - let rest = match rest.strip_suffix(b".dump") { - Some(rest) => rest, - None => return false, - }; - !rest.is_empty() && rest.iter().all(|b| b.is_ascii_digit()) + let basename = &path[pos + 1..]; + let expected = format!("jit-{pid}.dump"); + basename == expected.as_bytes() } /// Process a single MMAP2 record and add it to the symbols and unwind data maps From 26d6cc0e5db6f02dd32e860bd3fb0ef182ea48e5 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sat, 27 Jun 2026 10:34:05 +0200 Subject: [PATCH 5/7] chore: format perf profiler --- src/executor/wall_time/profiler/perf/mod.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/executor/wall_time/profiler/perf/mod.rs b/src/executor/wall_time/profiler/perf/mod.rs index 6846e8eda..884a45279 100644 --- a/src/executor/wall_time/profiler/perf/mod.rs +++ b/src/executor/wall_time/profiler/perf/mod.rs @@ -286,14 +286,13 @@ impl BenchmarkData<'_> { error!("Failed to harvest perf maps: {e}"); BenchmarkDataSaveError::FailedToHarvestPerfMaps })?; - let jit_unwind_data_by_pid = jit_dump::save_symbols_and_harvest_unwind_data_for_pids(path, - &jit_dump_paths_by_pid, - ) - .await - .map_err(|e| { - error!("Failed to harvest jit dumps: {e}"); - BenchmarkDataSaveError::FailedToHarvestJitDumps - })?; + let jit_unwind_data_by_pid = + jit_dump::save_symbols_and_harvest_unwind_data_for_pids(path, &jit_dump_paths_by_pid) + .await + .map_err(|e| { + error!("Failed to harvest jit dumps: {e}"); + BenchmarkDataSaveError::FailedToHarvestJitDumps + })?; let artifacts = save_artifacts::save_artifacts(path, &loaded_modules_by_path, &jit_unwind_data_by_pid); From 546d8c098500ccadf3f24b3b8ce36379b6b2ca30 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sat, 27 Jun 2026 10:58:28 +0200 Subject: [PATCH 6/7] fix: dedupe jitdump paths Use an IndexSet for discovered jitdump paths so repeated MMAP2 records do not append duplicate symbols or unwind data. Also keep the legacy /tmp/jit-.dump fallback for tracked PIDs when perf starts too late to capture the jitdump marker mapping. --- Cargo.lock | 1 + Cargo.toml | 1 + .../wall_time/profiler/perf/jit_dump.rs | 55 ++++++++++++++++++- src/executor/wall_time/profiler/perf/mod.rs | 3 +- .../profiler/perf/parse_perf_file.rs | 50 ++++++++++++----- 5 files changed, 93 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index daf2000ef..0fca01738 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -712,6 +712,7 @@ dependencies = [ "gimli", "git2", "gql_client", + "indexmap", "indicatif", "insta", "instrument-hooks-bindings", diff --git a/Cargo.toml b/Cargo.toml index a2703b51f..c10c40493 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ gql_client = { git = "https://github.com/CodSpeedHQ/gql-client-rs", rev = "c2691 serde_yaml = "0.9.34" sysinfo = { version = "0.37", features = ["serde"] } indicatif = "0.18" +indexmap = "2.14" console = "0.16" async-trait = "0.1.89" libc = { workspace = true } diff --git a/src/executor/wall_time/profiler/perf/jit_dump.rs b/src/executor/wall_time/profiler/perf/jit_dump.rs index 6c317b00e..686c061e2 100644 --- a/src/executor/wall_time/profiler/perf/jit_dump.rs +++ b/src/executor/wall_time/profiler/perf/jit_dump.rs @@ -1,12 +1,15 @@ use super::module_symbols::{ModuleSymbols, Symbol}; use crate::prelude::*; +use indexmap::IndexSet; use linux_perf_data::jitdump::{JitDumpReader, JitDumpRecord}; use runner_shared::unwind_data::{ProcessUnwindData, UnwindData}; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, path::{Path, PathBuf}, }; +pub(super) type JitDumpPathsByPid = HashMap>; + struct JitDump { path: PathBuf, } @@ -106,6 +109,26 @@ impl JitDump { } } +pub(super) fn insert_jit_dump_path( + jit_dump_paths_by_pid: &mut JitDumpPathsByPid, + pid: libc::pid_t, + path: PathBuf, +) -> bool { + jit_dump_paths_by_pid.entry(pid).or_default().insert(path) +} + +pub(super) fn add_legacy_tmp_jit_dump_paths_for_pids( + jit_dump_paths_by_pid: &mut JitDumpPathsByPid, + pids: &HashSet, +) { + for pid in pids { + let path = PathBuf::from("/tmp").join(format!("jit-{pid}.dump")); + if path.exists() && insert_jit_dump_path(jit_dump_paths_by_pid, *pid, path.clone()) { + debug!("Found legacy JIT dump file: {path:?}"); + } + } +} + /// Converts all the `jit-.dump` into a perf-.map with symbols, and collects the unwind data. /// /// Jitdump file paths are discovered from MMAP2 records in the perf data, since JIT runtimes @@ -119,7 +142,7 @@ impl JitDump { /// Unwind data is generated as a list pub async fn save_symbols_and_harvest_unwind_data_for_pids( profile_folder: &Path, - jit_dump_paths_by_pid: &HashMap>, + jit_dump_paths_by_pid: &JitDumpPathsByPid, ) -> Result>> { let mut jit_unwind_data_by_pid = HashMap::new(); @@ -154,3 +177,31 @@ pub async fn save_symbols_and_harvest_unwind_data_for_pids( Ok(jit_unwind_data_by_pid) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn insert_jit_dump_path_dedupes_per_pid() { + let mut paths = HashMap::new(); + let path = PathBuf::from("/tmp/jit-123.dump"); + + assert!(insert_jit_dump_path(&mut paths, 123, path.clone())); + assert!(!insert_jit_dump_path(&mut paths, 123, path.clone())); + + assert_eq!(paths[&123].iter().collect::>(), vec![&path]); + } + + #[test] + fn insert_jit_dump_path_keeps_same_path_for_different_pids() { + let mut paths = HashMap::new(); + let path = PathBuf::from("/tmp/jit-123.dump"); + + assert!(insert_jit_dump_path(&mut paths, 123, path.clone())); + assert!(insert_jit_dump_path(&mut paths, 456, path.clone())); + + assert_eq!(paths[&123].iter().collect::>(), vec![&path]); + assert_eq!(paths[&456].iter().collect::>(), vec![&path]); + } +} diff --git a/src/executor/wall_time/profiler/perf/mod.rs b/src/executor/wall_time/profiler/perf/mod.rs index 884a45279..ce7531199 100644 --- a/src/executor/wall_time/profiler/perf/mod.rs +++ b/src/executor/wall_time/profiler/perf/mod.rs @@ -270,7 +270,7 @@ impl BenchmarkData<'_> { let MemmapRecordsOutput { loaded_modules_by_path, tracked_pids, - jit_dump_paths_by_pid, + mut jit_dump_paths_by_pid, } = parse_perf_file::parse_for_memmap2(perf_file_path, pid_filter).map_err(|e| { error!("Failed to parse perf file: {e}"); BenchmarkDataSaveError::FailedToParsePerfFile @@ -286,6 +286,7 @@ impl BenchmarkData<'_> { error!("Failed to harvest perf maps: {e}"); BenchmarkDataSaveError::FailedToHarvestPerfMaps })?; + jit_dump::add_legacy_tmp_jit_dump_paths_for_pids(&mut jit_dump_paths_by_pid, &tracked_pids); let jit_unwind_data_by_pid = jit_dump::save_symbols_and_harvest_unwind_data_for_pids(path, &jit_dump_paths_by_pid) .await diff --git a/src/executor/wall_time/profiler/perf/parse_perf_file.rs b/src/executor/wall_time/profiler/perf/parse_perf_file.rs index 327eb57a0..3af6beba8 100644 --- a/src/executor/wall_time/profiler/perf/parse_perf_file.rs +++ b/src/executor/wall_time/profiler/perf/parse_perf_file.rs @@ -1,3 +1,4 @@ +use super::jit_dump::{JitDumpPathsByPid, insert_jit_dump_path}; use super::loaded_module::{LoadedModule, ProcessLoadedModule}; use super::module_symbols::ModuleSymbols; use super::unwind_data::unwind_data_from_elf; @@ -17,7 +18,7 @@ pub struct MemmapRecordsOutput { pub loaded_modules_by_path: HashMap, pub tracked_pids: HashSet, /// Jitdump file paths discovered from MMAP2 records, keyed by PID. - pub jit_dump_paths_by_pid: HashMap>, + pub jit_dump_paths_by_pid: JitDumpPathsByPid, } /// Parse the perf file at `perf_file_path` and look for MMAP2 records for the given `pids`. @@ -29,7 +30,7 @@ pub fn parse_for_memmap2>( mut pid_filter: PidFilter, ) -> Result { let mut loaded_modules_by_path = HashMap::::new(); - let mut jit_dump_paths_by_pid = HashMap::>::new(); + let mut jit_dump_paths_by_pid = JitDumpPathsByPid::new(); // 1MiB buffer let reader = std::io::BufReader::with_capacity( @@ -120,17 +121,19 @@ pub fn parse_for_memmap2>( // Collect jitdump file paths before the PROT_EXEC filter in process_mmap2_record // skips them. JIT runtimes mmap the jitdump file so perf records it. - // Match perf's jit_detect(): basename must be `jit-.dump`. - if is_jit_dump_path(&mmap2_record.path.as_slice(), mmap2_record.pid) { + // Match perf's jit_detect(): basename must be `jit-.dump`. + if is_jit_dump_path(&mmap2_record.path.as_slice()) { let path = PathBuf::from( String::from_utf8_lossy(&mmap2_record.path.as_slice()).into_owned(), ); - if path.exists() { + if path.exists() + && insert_jit_dump_path( + &mut jit_dump_paths_by_pid, + mmap2_record.pid, + path.clone(), + ) + { debug!("Found jitdump path from MMAP2 record: {path:?}"); - jit_dump_paths_by_pid - .entry(mmap2_record.pid) - .or_default() - .push(path); } } @@ -228,15 +231,19 @@ fn purge_process_mappings(loaded_modules_by_path: &mut HashMap.dump`, -/// where `` must match the MMAP2 record's PID. -fn is_jit_dump_path(path: &[u8], pid: pid_t) -> bool { +/// Returns true if the path basename matches perf's `jit_detect()` pattern: `jit-.dump`. +fn is_jit_dump_path(path: &[u8]) -> bool { let Some(pos) = path.iter().rposition(|&b| b == b'/') else { return false; }; let basename = &path[pos + 1..]; - let expected = format!("jit-{pid}.dump"); - basename == expected.as_bytes() + let Some(pid) = basename + .strip_prefix(b"jit-") + .and_then(|path| path.strip_suffix(b".dump")) + else { + return false; + }; + !pid.is_empty() && pid.iter().all(|b| b.is_ascii_digit()) } /// Process a single MMAP2 record and add it to the symbols and unwind data maps @@ -429,4 +436,19 @@ mod tests { Some(0xaaaaaaaa0000) ); } + + #[test] + fn is_jit_dump_path_accepts_numeric_pid() { + assert!(is_jit_dump_path(b"/tmp/jit-123.dump")); + assert!(is_jit_dump_path(b"/home/user/.debug/jit/abc/jit-123.dump")); + } + + #[test] + fn is_jit_dump_path_rejects_non_perf_jitdump_names() { + assert!(!is_jit_dump_path(b"jit-123.dump")); + assert!(!is_jit_dump_path(b"/tmp/jit-.dump")); + assert!(!is_jit_dump_path(b"/tmp/jit-abc.dump")); + assert!(!is_jit_dump_path(b"/tmp/not-jit-123.dump")); + assert!(!is_jit_dump_path(b"/tmp/jit-123.dump.old")); + } } From 8dfd9c32d776c2f9295cbb6641327f143edabdba Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:20:19 +0200 Subject: [PATCH 7/7] fix: harvest jitdumps for valgrind --- Cargo.lock | 1 - Cargo.toml | 1 - src/executor/valgrind/executor.rs | 105 ++++++++++++++++- src/executor/valgrind/helpers/perf_maps.rs | 6 +- .../wall_time/profiler/perf/jit_dump.rs | 106 +++++------------- src/executor/wall_time/profiler/perf/mod.rs | 4 +- .../profiler/perf/parse_perf_file.rs | 53 --------- 7 files changed, 135 insertions(+), 141 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 559737d4a..3e5a52330 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -713,7 +713,6 @@ dependencies = [ "gimli", "git2", "gql_client", - "indexmap", "indicatif", "insta", "instrument-hooks-bindings", diff --git a/Cargo.toml b/Cargo.toml index 607418d06..8055882e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,6 @@ gql_client = { git = "https://github.com/CodSpeedHQ/gql-client-rs", rev = "c2691 serde_yaml = "0.9.34" sysinfo = { version = "0.37", features = ["serde"] } indicatif = "0.18" -indexmap = "2.14" console = "0.16" async-trait = "0.1.89" libc = { workspace = true } diff --git a/src/executor/valgrind/executor.rs b/src/executor/valgrind/executor.rs index accd5b34a..f1651c3f3 100644 --- a/src/executor/valgrind/executor.rs +++ b/src/executor/valgrind/executor.rs @@ -1,5 +1,7 @@ use async_trait::async_trait; -use std::path::Path; +use std::collections::HashSet; +use std::fs; +use std::path::{Path, PathBuf}; use crate::executor::Executor; use crate::executor::ToolStatus; @@ -15,6 +17,47 @@ use super::{helpers::perf_maps::harvest_perf_maps, helpers::venv_compat, measure pub struct ValgrindExecutor; +fn harvest_jit_dumps( + profile_folder: &Path, + pids: &HashSet, + tmp_dir: &Path, + jit_dump_base_dir: Option<&Path>, +) -> Result<()> { + let mut jit_dump_paths = pids + .iter() + .map(|pid| tmp_dir.join(format!("jit-{pid}.dump"))) + .filter(|path| path.exists()) + .collect::>(); + + if let Some(base_dir) = jit_dump_base_dir { + let jit_dir = base_dir.join(".debug/jit"); + for entry in fs::read_dir(jit_dir).into_iter().flatten().flatten() { + for pid in pids { + let path = entry.path().join(format!("jit-{pid}.dump")); + if path.exists() { + jit_dump_paths.push(path); + } + } + } + } + + debug!("Found {} jit dumps", jit_dump_paths.len()); + for path in jit_dump_paths { + let Some(file_name) = path.file_name() else { + continue; + }; + fs::copy(&path, profile_folder.join(file_name)).with_context(|| { + format!( + "Failed to copy jit dump file: {:?} to {}", + file_name, + profile_folder.display() + ) + })?; + } + + Ok(()) +} + #[async_trait(?Send)] impl Executor for ValgrindExecutor { fn name(&self) -> ExecutorName { @@ -66,7 +109,27 @@ impl Executor for ValgrindExecutor { } async fn teardown(&self, execution_context: &ExecutionContext) -> Result<()> { - harvest_perf_maps(&execution_context.profile_folder).await?; + let pids = harvest_perf_maps(&execution_context.profile_folder).await?; + let jit_dump_base_dir = execution_context + .config + .extra_env + .get("JITDUMPDIR") + .map(PathBuf::from) + .or_else(|| std::env::var_os("JITDUMPDIR").map(PathBuf::from)) + .or_else(|| { + execution_context + .config + .extra_env + .get("HOME") + .map(PathBuf::from) + }) + .or_else(|| std::env::var_os("HOME").map(PathBuf::from)); + harvest_jit_dumps( + &execution_context.profile_folder, + &pids, + Path::new("/tmp"), + jit_dump_base_dir.as_deref(), + )?; // No matter the command in input, at this point valgrind will have been run and have produced output files. // @@ -78,3 +141,41 @@ impl Executor for ValgrindExecutor { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn harvests_jit_dumps_for_profile_pids() { + let temp_dir = tempfile::tempdir().unwrap(); + let profile_folder = temp_dir.path().join("profile"); + let tmp_dir = temp_dir.path().join("tmp"); + let jit_dump_base_dir = temp_dir.path().join("home"); + let llvm_jit_dir = jit_dump_base_dir.join(".debug/jit/llvm"); + fs::create_dir_all(&profile_folder).unwrap(); + fs::create_dir_all(&tmp_dir).unwrap(); + fs::create_dir_all(&llvm_jit_dir).unwrap(); + fs::write(tmp_dir.join("jit-123.dump"), b"tmp").unwrap(); + fs::write(llvm_jit_dir.join("jit-456.dump"), b"llvm").unwrap(); + fs::write(llvm_jit_dir.join("jit-789.dump"), b"untracked").unwrap(); + + harvest_jit_dumps( + &profile_folder, + &HashSet::from([123, 456]), + &tmp_dir, + Some(&jit_dump_base_dir), + ) + .unwrap(); + + assert_eq!( + fs::read(profile_folder.join("jit-123.dump")).unwrap(), + b"tmp" + ); + assert_eq!( + fs::read(profile_folder.join("jit-456.dump")).unwrap(), + b"llvm" + ); + assert!(!profile_folder.join("jit-789.dump").exists()); + } +} diff --git a/src/executor/valgrind/helpers/perf_maps.rs b/src/executor/valgrind/helpers/perf_maps.rs index 583ca2f95..f46a19433 100644 --- a/src/executor/valgrind/helpers/perf_maps.rs +++ b/src/executor/valgrind/helpers/perf_maps.rs @@ -15,12 +15,14 @@ fn extract_pid_from_profile_file(path: &Path) -> Option { } } -pub async fn harvest_perf_maps(profile_folder: &Path) -> Result<()> { +pub async fn harvest_perf_maps(profile_folder: &Path) -> Result> { let pids = fs::read_dir(profile_folder)? .filter_map(|entry| entry.ok()) .map(|entry| entry.path()) .filter_map(|path| extract_pid_from_profile_file(&path)) .collect::>(); - harvest_perf_maps_for_pids(profile_folder, &pids).await + harvest_perf_maps_for_pids(profile_folder, &pids).await?; + + Ok(pids) } diff --git a/src/executor/wall_time/profiler/perf/jit_dump.rs b/src/executor/wall_time/profiler/perf/jit_dump.rs index 686c061e2..fd5fad056 100644 --- a/src/executor/wall_time/profiler/perf/jit_dump.rs +++ b/src/executor/wall_time/profiler/perf/jit_dump.rs @@ -1,6 +1,5 @@ use super::module_symbols::{ModuleSymbols, Symbol}; use crate::prelude::*; -use indexmap::IndexSet; use linux_perf_data::jitdump::{JitDumpReader, JitDumpRecord}; use runner_shared::unwind_data::{ProcessUnwindData, UnwindData}; use std::{ @@ -8,8 +7,6 @@ use std::{ path::{Path, PathBuf}, }; -pub(super) type JitDumpPathsByPid = HashMap>; - struct JitDump { path: PathBuf, } @@ -109,30 +106,7 @@ impl JitDump { } } -pub(super) fn insert_jit_dump_path( - jit_dump_paths_by_pid: &mut JitDumpPathsByPid, - pid: libc::pid_t, - path: PathBuf, -) -> bool { - jit_dump_paths_by_pid.entry(pid).or_default().insert(path) -} - -pub(super) fn add_legacy_tmp_jit_dump_paths_for_pids( - jit_dump_paths_by_pid: &mut JitDumpPathsByPid, - pids: &HashSet, -) { - for pid in pids { - let path = PathBuf::from("/tmp").join(format!("jit-{pid}.dump")); - if path.exists() && insert_jit_dump_path(jit_dump_paths_by_pid, *pid, path.clone()) { - debug!("Found legacy JIT dump file: {path:?}"); - } - } -} - -/// Converts all the `jit-.dump` into a perf-.map with symbols, and collects the unwind data. -/// -/// Jitdump file paths are discovered from MMAP2 records in the perf data, since JIT runtimes -/// mmap the jitdump file and perf records the mapping with the actual path on disk. +/// Converts all the `jit-.dump` into a perf-.map with symbols, and collects the unwind data /// /// # Symbols /// Since a jit dump is by definition specific to a single pid, we append the harvested symbols @@ -142,66 +116,40 @@ pub(super) fn add_legacy_tmp_jit_dump_paths_for_pids( /// Unwind data is generated as a list pub async fn save_symbols_and_harvest_unwind_data_for_pids( profile_folder: &Path, - jit_dump_paths_by_pid: &JitDumpPathsByPid, + pids: &HashSet, ) -> Result>> { - let mut jit_unwind_data_by_pid = HashMap::new(); - - for (pid, paths) in jit_dump_paths_by_pid { - for path in paths { - debug!("Found JIT dump file: {path:?}"); - - let symbols = match JitDump::new(path.clone()).into_perf_map() { - Ok(symbols) => symbols, - Err(error) => { - warn!("Failed to convert jit dump into perf map: {error:?}"); - continue; - } - }; - - symbols.append_to_file(profile_folder.join(format!("perf-{pid}.map")))?; + let mut jit_unwind_data_by_path = HashMap::new(); - let jit_unwind_data = match JitDump::new(path.clone()).into_unwind_data() { - Ok(data) => data, - Err(error) => { - warn!("Failed to convert jit dump into unwind data: {error:?}"); - continue; - } - }; + for pid in pids { + let name = format!("jit-{pid}.dump"); + let path = PathBuf::from("/tmp").join(&name); - jit_unwind_data_by_pid - .entry(*pid) - .or_insert_with(Vec::new) - .extend(jit_unwind_data); + if !path.exists() { + continue; } - } - - Ok(jit_unwind_data_by_pid) -} + debug!("Found JIT dump file: {path:?}"); -#[cfg(test)] -mod tests { - use super::*; + let symbols = match JitDump::new(path.clone()).into_perf_map() { + Ok(symbols) => symbols, + Err(error) => { + warn!("Failed to convert jit dump into perf map: {error:?}"); + continue; + } + }; - #[test] - fn insert_jit_dump_path_dedupes_per_pid() { - let mut paths = HashMap::new(); - let path = PathBuf::from("/tmp/jit-123.dump"); + // Also write to perf-.map for harvested Python perf maps compatibility + symbols.append_to_file(profile_folder.join(format!("perf-{pid}.map")))?; - assert!(insert_jit_dump_path(&mut paths, 123, path.clone())); - assert!(!insert_jit_dump_path(&mut paths, 123, path.clone())); + let jit_unwind_data = match JitDump::new(path).into_unwind_data() { + Ok(data) => data, + Err(error) => { + warn!("Failed to convert jit dump into unwind data: {error:?}"); + continue; + } + }; - assert_eq!(paths[&123].iter().collect::>(), vec![&path]); + jit_unwind_data_by_path.insert(*pid, jit_unwind_data); } - #[test] - fn insert_jit_dump_path_keeps_same_path_for_different_pids() { - let mut paths = HashMap::new(); - let path = PathBuf::from("/tmp/jit-123.dump"); - - assert!(insert_jit_dump_path(&mut paths, 123, path.clone())); - assert!(insert_jit_dump_path(&mut paths, 456, path.clone())); - - assert_eq!(paths[&123].iter().collect::>(), vec![&path]); - assert_eq!(paths[&456].iter().collect::>(), vec![&path]); - } + Ok(jit_unwind_data_by_path) } diff --git a/src/executor/wall_time/profiler/perf/mod.rs b/src/executor/wall_time/profiler/perf/mod.rs index 4c0475ab6..2c5514d24 100644 --- a/src/executor/wall_time/profiler/perf/mod.rs +++ b/src/executor/wall_time/profiler/perf/mod.rs @@ -269,7 +269,6 @@ impl BenchmarkData<'_> { let MemmapRecordsOutput { loaded_modules_by_path, tracked_pids, - mut jit_dump_paths_by_pid, } = parse_perf_file::parse_for_memmap2(perf_file_path, pid_filter).map_err(|e| { error!("Failed to parse perf file: {e}"); BenchmarkDataSaveError::FailedToParsePerfFile @@ -285,9 +284,8 @@ impl BenchmarkData<'_> { error!("Failed to harvest perf maps: {e}"); BenchmarkDataSaveError::FailedToHarvestPerfMaps })?; - jit_dump::add_legacy_tmp_jit_dump_paths_for_pids(&mut jit_dump_paths_by_pid, &tracked_pids); let jit_unwind_data_by_pid = - jit_dump::save_symbols_and_harvest_unwind_data_for_pids(path, &jit_dump_paths_by_pid) + jit_dump::save_symbols_and_harvest_unwind_data_for_pids(path, &tracked_pids) .await .map_err(|e| { error!("Failed to harvest jit dumps: {e}"); diff --git a/src/executor/wall_time/profiler/perf/parse_perf_file.rs b/src/executor/wall_time/profiler/perf/parse_perf_file.rs index 3af6beba8..151b54945 100644 --- a/src/executor/wall_time/profiler/perf/parse_perf_file.rs +++ b/src/executor/wall_time/profiler/perf/parse_perf_file.rs @@ -1,4 +1,3 @@ -use super::jit_dump::{JitDumpPathsByPid, insert_jit_dump_path}; use super::loaded_module::{LoadedModule, ProcessLoadedModule}; use super::module_symbols::ModuleSymbols; use super::unwind_data::unwind_data_from_elf; @@ -17,8 +16,6 @@ pub struct MemmapRecordsOutput { /// Module symbols and the computed load bias for each pid that maps the ELF path. pub loaded_modules_by_path: HashMap, pub tracked_pids: HashSet, - /// Jitdump file paths discovered from MMAP2 records, keyed by PID. - pub jit_dump_paths_by_pid: JitDumpPathsByPid, } /// Parse the perf file at `perf_file_path` and look for MMAP2 records for the given `pids`. @@ -30,7 +27,6 @@ pub fn parse_for_memmap2>( mut pid_filter: PidFilter, ) -> Result { let mut loaded_modules_by_path = HashMap::::new(); - let mut jit_dump_paths_by_pid = JitDumpPathsByPid::new(); // 1MiB buffer let reader = std::io::BufReader::with_capacity( @@ -119,24 +115,6 @@ pub fn parse_for_memmap2>( continue; } - // Collect jitdump file paths before the PROT_EXEC filter in process_mmap2_record - // skips them. JIT runtimes mmap the jitdump file so perf records it. - // Match perf's jit_detect(): basename must be `jit-.dump`. - if is_jit_dump_path(&mmap2_record.path.as_slice()) { - let path = PathBuf::from( - String::from_utf8_lossy(&mmap2_record.path.as_slice()).into_owned(), - ); - if path.exists() - && insert_jit_dump_path( - &mut jit_dump_paths_by_pid, - mmap2_record.pid, - path.clone(), - ) - { - debug!("Found jitdump path from MMAP2 record: {path:?}"); - } - } - process_mmap2_record(mmap2_record, &mut loaded_modules_by_path); } _ => continue, @@ -155,7 +133,6 @@ pub fn parse_for_memmap2>( Ok(MemmapRecordsOutput { loaded_modules_by_path, tracked_pids, - jit_dump_paths_by_pid, }) } @@ -231,21 +208,6 @@ fn purge_process_mappings(loaded_modules_by_path: &mut HashMap.dump`. -fn is_jit_dump_path(path: &[u8]) -> bool { - let Some(pos) = path.iter().rposition(|&b| b == b'/') else { - return false; - }; - let basename = &path[pos + 1..]; - let Some(pid) = basename - .strip_prefix(b"jit-") - .and_then(|path| path.strip_suffix(b".dump")) - else { - return false; - }; - !pid.is_empty() && pid.iter().all(|b| b.is_ascii_digit()) -} - /// Process a single MMAP2 record and add it to the symbols and unwind data maps fn process_mmap2_record( record: linux_perf_data::linux_perf_event_reader::Mmap2Record, @@ -436,19 +398,4 @@ mod tests { Some(0xaaaaaaaa0000) ); } - - #[test] - fn is_jit_dump_path_accepts_numeric_pid() { - assert!(is_jit_dump_path(b"/tmp/jit-123.dump")); - assert!(is_jit_dump_path(b"/home/user/.debug/jit/abc/jit-123.dump")); - } - - #[test] - fn is_jit_dump_path_rejects_non_perf_jitdump_names() { - assert!(!is_jit_dump_path(b"jit-123.dump")); - assert!(!is_jit_dump_path(b"/tmp/jit-.dump")); - assert!(!is_jit_dump_path(b"/tmp/jit-abc.dump")); - assert!(!is_jit_dump_path(b"/tmp/not-jit-123.dump")); - assert!(!is_jit_dump_path(b"/tmp/jit-123.dump.old")); - } }