Skip to content

Commit 9d9f24b

Browse files
committed
Fix some formatting and typos.
1 parent f8d2bd3 commit 9d9f24b

File tree

1 file changed

+42
-27
lines changed
  • collector/src/compile/execute

1 file changed

+42
-27
lines changed

collector/src/compile/execute/mod.rs

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -134,66 +134,81 @@ pub struct CargoProcess<'a> {
134134
pub target: Target,
135135
pub workspace_package: Option<String>,
136136
}
137-
/// Returns an optional list of Performance CPU cores, if the system has P and E cores.
137+
138+
/// Returns an optional list of P-cores, if the system has P-cores and E-cores.
138139
/// This list *should* be in a format suitable for the `taskset` command.
139140
#[cfg(target_os = "linux")]
140141
fn performance_cores() -> Option<&'static String> {
141142
use std::sync::LazyLock;
142143
static PERFORMANCE_CORES: LazyLock<Option<String>> = LazyLock::new(|| {
143-
if std::fs::exists("/sys/devices/cpu").expect("Could not check the CPU architecture details: could not check if `/sys/devices/cpu` exists!") {
144-
// If /sys/devices/cpu exists, then this is not a "Performance-hybrid" CPU.
145-
None
146-
}
147-
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!") {
148-
// If /sys/devices/cpu_core exists, then this is a "Performance-hybrid" CPU.
149-
eprintln!("WARNING: Performance-Hybrid CPU detected. `rustc-perf` can't run properly on Efficency cores: test suite will only use Performance cores!");
150-
Some(std::fs::read_to_string("/sys/devices/cpu_core/cpus").unwrap().trim().to_string())
151-
} else {
152-
// If neither dir exists, then something is wrong - `/sys/devices/cpu` has been in Linux for over a decade.
153-
eprintln!("WARNING: neither `/sys/devices/cpu` nor `/sys/devices/cpu_core` present, unable to determine if this CPU has a Performance-Hybrid architecture.");
154-
None
155-
}
144+
if std::fs::exists("/sys/devices/cpu")
145+
.expect("Could not check if `/sys/devices/cpu` exists")
146+
{
147+
// If /sys/devices/cpu exists, then this is not a hybrid CPU.
148+
None
149+
} else if std::fs::exists("/sys/devices/cpu_core")
150+
.expect("Could not check if `/sys/devices/cpu_core` exists!")
151+
{
152+
// If /sys/devices/cpu_core exists, then this is a hybrid CPU.
153+
eprintln!("WARNING: hybrid Intel CPU detected.");
154+
eprintln!("WARNING: test suite will only use P-cores, not E-cores");
155+
Some(
156+
std::fs::read_to_string("/sys/devices/cpu_core/cpus")
157+
.unwrap()
158+
.trim()
159+
.to_string(),
160+
)
161+
} else {
162+
// If neither dir exists, then something is wrong, because `/sys/devices/cpu` has been
163+
// in Linux for over a decade.
164+
eprintln!("WARNING: neither `/sys/devices/cpu` nor `/sys/devices/cpu_core` present");
165+
eprintln!("WARNING: unable to determine if CPU has a hybrid architecture");
166+
None
167+
}
156168
});
157169
(*PERFORMANCE_CORES).as_ref()
158170
}
159171

160172
#[cfg(not(target_os = "linux"))]
161-
// Modify this stub if you want to add support for P/E cores on more OSs
173+
// Modify this stub if you want to add support for P-/E-cores on more OSs
162174
fn performance_cores() -> Option<&'static String> {
163175
None
164176
}
165177

166178
#[cfg(target_os = "linux")]
167179
/// Makes the benchmark run only on Performance cores.
168180
fn run_on_p_cores(path: &Path, cpu_list: &str) -> Command {
169-
// Parse CPU list to extract the number of P cores!
170-
// This assumes the P core id's are countinus, in format `fisrt_id-last_id`
181+
// Parse CPU list to extract the number of P-cores!
182+
// This assumes the P-core id's are continuous, in format `first_id-last_id`
171183
let (core_start, core_end) = cpu_list
172184
.split_once("-")
173-
.unwrap_or_else(|| panic!("Unsuported P core list format: {cpu_list:?}."));
185+
.unwrap_or_else(|| panic!("Unsupported P-core list format: {cpu_list:?}."));
174186
let core_start: u32 = core_start
175187
.parse()
176-
.expect("Expected a number when parsing the start of the P core list!");
188+
.expect("Expected a number when parsing the start of the P-core list!");
177189
let core_end: u32 = core_end
178190
.parse()
179-
.expect("Expected a number when parsing the end of the P core list!");
191+
.expect("Expected a number when parsing the end of the P-core list!");
180192
let core_count = core_end - core_start;
181193
let mut cmd = Command::new("taskset");
182-
// Set job count to P core count - this is done for 2 reasons:
183-
// 1. The instruction count info for E core is often very incompleate - a substantial chunk of events is lost.
184-
// 2. The performance charcteristics of E cores are less reliable, so excluding them from the benchmark makes things easier.
194+
// Set job count to P-core count. This is done for 3 reasons:
195+
// 1. The instruction count info for E-cores is often incomplete, and a substantial chunk of
196+
// events is lost.
197+
// 2. The performance characteristics of E-cores are less reliable, so excluding them from the
198+
// benchmark makes things easier.
199+
// 3. An unpredictable mix of P-core and E-core execution will give inconsistent results.
185200
cmd.env("CARGO_BUILD_JOBS", format!("{core_count}"));
186-
// pass the P core list to taskset to pin task to the P core.
201+
// Pass the P-core list to taskset to pin task to the P-core.
187202
cmd.arg("--cpu-list");
188203
cmd.arg(cpu_list);
189204
cmd.arg(path);
190205
cmd
191206
}
192207

193208
#[cfg(not(target_os = "linux"))]
194-
// Modify this stub if you want to add support for P/E cores on more OSs
209+
// Modify this stub if you want to add support for P-cores/E-cores on more OSs.
195210
fn run_on_p_cores(_path: &Path, _cpu_list: &str) -> Command {
196-
todo!("Can't run commands on the P cores on this platform");
211+
todo!("Can't run commands on the P-cores on this platform");
197212
}
198213

199214
impl<'a> CargoProcess<'a> {
@@ -214,7 +229,7 @@ impl<'a> CargoProcess<'a> {
214229
}
215230

216231
fn base_command(&self, cwd: &Path, subcommand: &str) -> Command {
217-
// Processors with P and E cores require special handling
232+
// Processors with P-core and E-cores require special handling.
218233
let mut cmd = if let Some(p_cores) = performance_cores() {
219234
run_on_p_cores(Path::new(&self.toolchain.components.cargo), p_cores)
220235
} else {

0 commit comments

Comments
 (0)