Skip to content

Commit 79b3ae1

Browse files
committed
fix: run perf with sudo; support systemd-run for non-perf walltime
1 parent 94cc035 commit 79b3ae1

2 files changed

Lines changed: 50 additions & 19 deletions

File tree

src/run/runner/wall_time/executor.rs

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use crate::run::runner::executor::Executor;
55
use crate::run::runner::helpers::env::get_base_injected_env;
66
use crate::run::runner::helpers::get_bench_command::get_bench_command;
77
use crate::run::runner::helpers::run_command_with_log_pipe::run_command_with_log_pipe;
8-
use crate::run::runner::{ExecutorName, RunData, RunnerMode};
8+
use crate::run::runner::{ExecutorName, RunData};
9+
use crate::run::RunnerMode;
910
use crate::run::{check_system::SystemInfo, config::Config};
1011
use async_trait::async_trait;
1112
use std::fs::canonicalize;
@@ -28,6 +29,18 @@ impl WallTimeExecutor {
2829
perf: use_perf.then(PerfRunner::new),
2930
}
3031
}
32+
33+
fn walltime_bench_cmd(config: &Config, run_data: &RunData) -> Result<String> {
34+
let bench_cmd = get_bench_command(config)?;
35+
36+
let setenv = get_base_injected_env(RunnerMode::Walltime, &run_data.profile_folder)
37+
.into_iter()
38+
.map(|(env, value)| format!("--setenv={env}={value}"))
39+
.join(" ");
40+
let uid = nix::unistd::Uid::current().as_raw();
41+
let gid = nix::unistd::Gid::current().as_raw();
42+
Ok(format!("systemd-run --scope --slice=codspeed.slice --same-dir --uid={uid} --gid={gid} {setenv} -- {bench_cmd}"))
43+
}
3144
}
3245

3346
#[async_trait(?Send)]
@@ -51,25 +64,18 @@ impl Executor for WallTimeExecutor {
5164
run_data: &RunData,
5265
_mongo_tracer: &Option<MongoTracer>,
5366
) -> Result<()> {
54-
// IMPORTANT: Don't use `sh` here! We will use this pid to send signals to the
55-
// spawned child process which won't work if we use a different shell.
56-
let mut cmd = Command::new("bash");
57-
58-
cmd.envs(get_base_injected_env(
59-
RunnerMode::Walltime,
60-
&run_data.profile_folder,
61-
));
67+
let mut cmd = Command::new("sh");
6268

6369
if let Some(cwd) = &config.working_directory {
6470
let abs_cwd = canonicalize(cwd)?;
6571
cmd.current_dir(abs_cwd);
6672
}
6773

68-
let bench_cmd = get_bench_command(config)?;
74+
let bench_cmd = Self::walltime_bench_cmd(config, run_data)?;
6975
let status = if let Some(perf) = &self.perf {
7076
perf.run(cmd, &bench_cmd).await
7177
} else {
72-
cmd.args(["-c", &bench_cmd]);
78+
cmd.args(["-c", &format!("sudo {bench_cmd}")]);
7379
debug!("cmd: {:?}", cmd);
7480

7581
run_command_with_log_pipe(cmd).await

src/run/runner/wall_time/perf/mod.rs

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,22 +108,20 @@ impl PerfRunner {
108108
""
109109
}
110110
};
111-
let user = nix::unistd::User::from_uid(nix::unistd::Uid::current())?.unwrap();
111+
112112
cmd.args([
113113
"-c",
114114
&format!(
115-
"perf record {quiet_flag} --user-callchains --freq=999 --switch-output --control=fifo:{},{} --delay=-1 -g --call-graph={cg_mode} --output={} -- \
116-
sudo systemd-run --scope --slice=codspeed.slice --same-dir -- runuser -u {} -- {bench_cmd}",
115+
"sudo perf record {quiet_flag} --user-callchains --freq=999 --switch-output --control=fifo:{},{} --delay=-1 -g --call-graph={cg_mode} --output={} -- {bench_cmd}",
117116
perf_fifo.ctl_fifo_path.to_string_lossy(),
118117
perf_fifo.ack_fifo_path.to_string_lossy(),
119118
perf_file.path().to_string_lossy(),
120-
user.name
121119
),
122120
]);
123121
debug!("cmd: {:?}", cmd);
124122

125-
let on_process_started = async |pid: u32| -> anyhow::Result<()> {
126-
let data = Self::handle_fifo(pid, runner_fifo, perf_fifo).await?;
123+
let on_process_started = async |_| -> anyhow::Result<()> {
124+
let data = Self::handle_fifo(runner_fifo, perf_fifo).await?;
127125
let _ = self.benchmark_data.set(data);
128126

129127
Ok(())
@@ -134,6 +132,18 @@ impl PerfRunner {
134132
pub async fn save_files_to(&self, profile_folder: &PathBuf) -> anyhow::Result<()> {
135133
let start = std::time::Instant::now();
136134

135+
// We ran perf with sudo, so we have to change the ownership of the perf data files
136+
run_with_sudo(&[
137+
"chown",
138+
"-R",
139+
&format!(
140+
"{}:{}",
141+
nix::unistd::Uid::current(),
142+
nix::unistd::Gid::current()
143+
),
144+
self.perf_dir.path().to_str().unwrap(),
145+
])?;
146+
137147
// Copy the perf data files to the profile folder
138148
let copy_tasks = std::fs::read_dir(&self.perf_dir)?
139149
.filter_map(|entry| entry.ok())
@@ -194,7 +204,6 @@ impl PerfRunner {
194204
}
195205

196206
async fn handle_fifo(
197-
perf_pid: u32,
198207
mut runner_fifo: RunnerFifo,
199208
mut perf_fifo: PerfFifo,
200209
) -> anyhow::Result<BenchmarkData> {
@@ -203,6 +212,7 @@ impl PerfRunner {
203212
let mut unwind_data_by_pid = HashMap::<u32, Vec<UnwindData>>::new();
204213
let mut integration = None;
205214

215+
let perf_pid = OnceCell::new();
206216
loop {
207217
let perf_ping = tokio::time::timeout(Duration::from_secs(1), perf_fifo.ping()).await;
208218
if let Ok(Err(_)) | Err(_) = perf_ping {
@@ -276,7 +286,22 @@ impl PerfRunner {
276286
runner_fifo.send_cmd(FifoCommand::Ack).await?;
277287
}
278288
FifoCommand::StartBenchmark => {
279-
unsafe { libc::kill(perf_pid as i32, libc::SIGUSR2) };
289+
let perf_pid = perf_pid.get_or_init(|| {
290+
let output = std::process::Command::new("sh")
291+
.arg("-c")
292+
.arg("pidof -s perf")
293+
.output()
294+
.expect("Failed to run pidof command");
295+
296+
String::from_utf8_lossy(&output.stdout)
297+
.trim()
298+
.parse::<u32>()
299+
.expect("Failed to parse perf pid")
300+
});
301+
302+
// Split the perf.data file
303+
run_with_sudo(&["kill", "-USR2", &perf_pid.to_string()])?;
304+
280305
perf_fifo.start_events().await?;
281306
runner_fifo.send_cmd(FifoCommand::Ack).await?;
282307
}

0 commit comments

Comments
 (0)