From 7dd0821620c130e327a61723a062332618f941f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kostrubiec?= Date: Mon, 10 Feb 2025 18:03:39 +0100 Subject: [PATCH 1/2] Added support for Performance / Efficency cores. --- collector/src/compile/execute/mod.rs | 72 ++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 3 deletions(-) diff --git a/collector/src/compile/execute/mod.rs b/collector/src/compile/execute/mod.rs index 144c508c0..392d51df1 100644 --- a/collector/src/compile/execute/mod.rs +++ b/collector/src/compile/execute/mod.rs @@ -130,7 +130,64 @@ pub struct CargoProcess<'a> { pub touch_file: Option, pub jobserver: Option, } - +/// Returns an optional list of Performance CPU cores, if the system has P and E cores. +/// This list *should* be in a format suitable for the `taskset` command. +#[cfg(target_os = "linux")] +fn performance_cores() -> Option<&'static String> { + use std::sync::LazyLock; + static PERFORMANCE_CORES: LazyLock> = LazyLock::new(|| { + if std::fs::exists("/sys/devices/cpu").expect("Could not check the CPU architecture details: could not check if `/sys/devices/cpu` exists!"){ + // If /sys/devices/cpu exists, then this is not a "Performance-hybrid" CPU. + None + } + else if std::fs::exists("/sys/devices/cpu_core").expect("Could not check the CPU architecture detali: could not check if `/sys/devices/cpu_core` exists!") { + // If /sys/devices/cpu_core exists, then this is a "Performance-hybrid" CPU. + eprintln!("WARNING: Performance-Hybrid CPU detected. `rustc-perf` can't run properly on Efficency cores: test suite will only use Performance cores!"); + Some(std::fs::read_to_string("/sys/devices/cpu_core/cpus").unwrap().trim().to_string()) + }else{ + // If neither dir exists, then something is wrong - `/sys/devices/cpu` has been in Linux for over a decade. + eprintln!("WARNING: neither `/sys/devices/cpu` nor `/sys/devices/cpu_core` present, unable to determine if this CPU has a Performance-Hybrid architecture."); + None + } + }); + (*PERFORMANCE_CORES).as_ref() +} +#[cfg(not(target_os = "linux"))] +// Modify this stub if you want to add support for P/E cores on more OSs +fn performance_cores() -> Option<&'static String> { + None +} +#[cfg(target_os = "linux")] +/// Makes the benchmark run only on Performance cores. +fn run_on_p_cores(path: &Path, cpu_list: &str) -> Command { + // Parse CPU list to extract the number of P cores! + // This assumes the P core id's are countinus, in format `fisrt_id-last_id` + let (core_start, core_end) = cpu_list + .split_once("-") + .unwrap_or_else(|| panic!("Unsuported P core list format: {cpu_list:?}.")); + let core_start: u32 = core_start + .parse() + .expect("Expected a number when parsing the start of the P core list!"); + let core_end: u32 = core_end + .parse() + .expect("Expected a number when parsing the end of the P core list!"); + let core_count = core_end - core_start; + let mut cmd = Command::new("taskset"); + // Set job count to P core count - this is done for 2 reasons: + // 1. The instruction count info for E core is often very incompleate - a substantial chunk of events is lost. + // 2. The performance charcteristics of E cores are less reliable, so excluding them from the benchmark makes things easier. + cmd.env("CARGO_BUILD_JOBS", format!("{core_count}")); + // pass the P core list to taskset to pin task to the P core. + cmd.arg("--cpu-list"); + cmd.arg(cpu_list); + cmd.arg(path); + cmd +} +#[cfg(not(target_os = "linux"))] +// Modify this stub if you want to add support for P/E cores on more OSs +fn run_on_p_cores(_path: &Path, _cpu_list: &str) -> Command { + todo!("Can't run comamnds on the P cores on this platform") +} impl<'a> CargoProcess<'a> { pub fn incremental(mut self, incremental: bool) -> Self { self.incremental = incremental; @@ -149,7 +206,12 @@ impl<'a> CargoProcess<'a> { } fn base_command(&self, cwd: &Path, subcommand: &str) -> Command { - let mut cmd = Command::new(Path::new(&self.toolchain.components.cargo)); + // Processors with P and E cores require special handling + let mut cmd = if let Some(p_cores) = performance_cores() { + run_on_p_cores(Path::new(&self.toolchain.components.cargo), p_cores) + } else { + Command::new(Path::new(&self.toolchain.components.cargo)) + }; cmd // Not all cargo invocations (e.g. `cargo clean`) need all of these // env vars set, but it doesn't hurt to have them. @@ -550,7 +612,11 @@ fn process_stat_output( let mut parts = line.split(';').map(|s| s.trim()); let cnt = get!(parts.next()); let _unit = get!(parts.next()); - let name = get!(parts.next()); + let mut name = get!(parts.next()); + // Map P-core events to normal events + if name == "cpu_core/instructions:u/" { + name = "instructions:u"; + } let _time = get!(parts.next()); let pct = get!(parts.next()); if cnt == "" || cnt == "" || cnt.is_empty() { From 89efd0ed89a10d8e528d4f426098dc0413f8431f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 13 Feb 2025 17:42:23 +0100 Subject: [PATCH 2/2] Manually format code since rustfmt chokes on it --- collector/src/compile/execute/mod.rs | 30 ++++++++++++++++------------ 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/collector/src/compile/execute/mod.rs b/collector/src/compile/execute/mod.rs index 392d51df1..c43fb02d6 100644 --- a/collector/src/compile/execute/mod.rs +++ b/collector/src/compile/execute/mod.rs @@ -136,27 +136,29 @@ pub struct CargoProcess<'a> { fn performance_cores() -> Option<&'static String> { use std::sync::LazyLock; static PERFORMANCE_CORES: LazyLock> = LazyLock::new(|| { - if std::fs::exists("/sys/devices/cpu").expect("Could not check the CPU architecture details: could not check if `/sys/devices/cpu` exists!"){ + if std::fs::exists("/sys/devices/cpu").expect("Could not check the CPU architecture details: could not check if `/sys/devices/cpu` exists!") { // If /sys/devices/cpu exists, then this is not a "Performance-hybrid" CPU. - None - } - else if std::fs::exists("/sys/devices/cpu_core").expect("Could not check the CPU architecture detali: could not check if `/sys/devices/cpu_core` exists!") { - // If /sys/devices/cpu_core exists, then this is a "Performance-hybrid" CPU. - eprintln!("WARNING: Performance-Hybrid CPU detected. `rustc-perf` can't run properly on Efficency cores: test suite will only use Performance cores!"); - Some(std::fs::read_to_string("/sys/devices/cpu_core/cpus").unwrap().trim().to_string()) - }else{ - // If neither dir exists, then something is wrong - `/sys/devices/cpu` has been in Linux for over a decade. - eprintln!("WARNING: neither `/sys/devices/cpu` nor `/sys/devices/cpu_core` present, unable to determine if this CPU has a Performance-Hybrid architecture."); - None - } + None + } + else if std::fs::exists("/sys/devices/cpu_core").expect("Could not check the CPU architecture detali: could not check if `/sys/devices/cpu_core` exists!") { + // If /sys/devices/cpu_core exists, then this is a "Performance-hybrid" CPU. + eprintln!("WARNING: Performance-Hybrid CPU detected. `rustc-perf` can't run properly on Efficency cores: test suite will only use Performance cores!"); + Some(std::fs::read_to_string("/sys/devices/cpu_core/cpus").unwrap().trim().to_string()) + } else { + // If neither dir exists, then something is wrong - `/sys/devices/cpu` has been in Linux for over a decade. + eprintln!("WARNING: neither `/sys/devices/cpu` nor `/sys/devices/cpu_core` present, unable to determine if this CPU has a Performance-Hybrid architecture."); + None + } }); (*PERFORMANCE_CORES).as_ref() } + #[cfg(not(target_os = "linux"))] // Modify this stub if you want to add support for P/E cores on more OSs fn performance_cores() -> Option<&'static String> { None } + #[cfg(target_os = "linux")] /// Makes the benchmark run only on Performance cores. fn run_on_p_cores(path: &Path, cpu_list: &str) -> Command { @@ -183,11 +185,13 @@ fn run_on_p_cores(path: &Path, cpu_list: &str) -> Command { cmd.arg(path); cmd } + #[cfg(not(target_os = "linux"))] // Modify this stub if you want to add support for P/E cores on more OSs fn run_on_p_cores(_path: &Path, _cpu_list: &str) -> Command { - todo!("Can't run comamnds on the P cores on this platform") + todo!("Can't run commands on the P cores on this platform"); } + impl<'a> CargoProcess<'a> { pub fn incremental(mut self, incremental: bool) -> Self { self.incremental = incremental;