Skip to content

Commit 61422a1

Browse files
feat: bypass systemd-run usage on macos
1 parent 19ea85c commit 61422a1

3 files changed

Lines changed: 49 additions & 22 deletions

File tree

src/executor/wall_time/executor.rs

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
use super::helpers::validate_walltime_results;
2+
use super::isolation::wrap_with_isolation;
23
use super::perf::PerfRunner;
34
use crate::executor::Executor;
45
use crate::executor::ExecutorConfig;
56
use crate::executor::ToolStatus;
67
use crate::executor::helpers::command::CommandBuilder;
7-
use crate::executor::helpers::env::{
8-
build_path_env, get_base_injected_env, is_codspeed_debug_enabled,
9-
};
8+
use crate::executor::helpers::env::{build_path_env, get_base_injected_env};
109
use crate::executor::helpers::get_bench_command::get_bench_command;
1110
use crate::executor::helpers::run_command_with_log_pipe::run_command_with_log_pipe;
1211
use crate::executor::helpers::run_with_env::wrap_with_env;
@@ -105,28 +104,12 @@ impl WallTimeExecutor {
105104
bench_cmd.arg(script_file.path());
106105
let (mut bench_cmd, env_file) = wrap_with_env(bench_cmd, &extra_env)?;
107106

108-
let mut cmd_builder = CommandBuilder::new("systemd-run");
109107
if let Some(cwd) = &config.working_directory {
110108
let abs_cwd = canonicalize(cwd)?;
111-
cmd_builder.current_dir(abs_cwd);
109+
bench_cmd.current_dir(abs_cwd);
112110
}
113-
if !is_codspeed_debug_enabled() {
114-
cmd_builder.arg("--quiet");
115-
}
116-
// Remarks:
117-
// - We're using --scope so that perf is able to capture the events of the benchmark process.
118-
// - We can't user `--user` here because we need to run in `codspeed.slice`, otherwise we'd run in
119-
// user.slice` (which is isolated). We can use `--gid` and `--uid` to run the command as the current user.
120-
// - We must use `bash` here instead of `sh` since `source` isn't available when symlinked to `dash`.
121-
// - We have to pass the environment variables because `--scope` only inherits the system and not the user environment variables.
122-
cmd_builder.arg("--slice=codspeed.slice");
123-
cmd_builder.arg("--scope");
124-
cmd_builder.arg("--same-dir");
125-
cmd_builder.arg(format!("--uid={}", nix::unistd::Uid::current().as_raw()));
126-
cmd_builder.arg(format!("--gid={}", nix::unistd::Gid::current().as_raw()));
127-
cmd_builder.args(["--"]);
128-
129-
bench_cmd.wrap_with(cmd_builder);
111+
112+
let bench_cmd = wrap_with_isolation(bench_cmd)?;
130113

131114
Ok((env_file, script_file, bench_cmd))
132115
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
use crate::executor::helpers::command::CommandBuilder;
2+
use crate::prelude::*;
3+
4+
/// Run the benchmark command in an isolated process scope.
5+
///
6+
/// On Linux, the command is wrapped with `systemd-run --scope` so it runs inside the
7+
/// `codspeed.slice` cgroup (required for perf to capture the full process tree).
8+
///
9+
/// Remarks:
10+
/// - We're using `--scope` so that perf is able to capture the events of the benchmark process.
11+
/// - We can't use `--user` here because we need to run in `codspeed.slice`, otherwise we'd run in
12+
/// `user.slice` (which is isolated). We use `--uid` and `--gid` to keep running as the current
13+
/// user.
14+
/// - `--scope` only inherits the system environment, so the caller is expected to have already
15+
/// forwarded the relevant variables (via `wrap_with_env`).
16+
/// - The caller is expected to have already set the working directory on `bench_cmd`; it will be
17+
/// propagated to `systemd-run` via [`CommandBuilder::wrap_with`], and `--same-dir` makes the
18+
/// spawned scope inherit it.
19+
#[cfg(target_os = "linux")]
20+
pub fn wrap_with_isolation(mut bench_cmd: CommandBuilder) -> Result<CommandBuilder> {
21+
use crate::executor::helpers::env::is_codspeed_debug_enabled;
22+
23+
let mut cmd_builder = CommandBuilder::new("systemd-run");
24+
if !is_codspeed_debug_enabled() {
25+
cmd_builder.arg("--quiet");
26+
}
27+
cmd_builder.arg("--slice=codspeed.slice");
28+
cmd_builder.arg("--scope");
29+
cmd_builder.arg("--same-dir");
30+
cmd_builder.arg(format!("--uid={}", nix::unistd::Uid::current().as_raw()));
31+
cmd_builder.arg(format!("--gid={}", nix::unistd::Gid::current().as_raw()));
32+
cmd_builder.args(["--"]);
33+
34+
bench_cmd.wrap_with(cmd_builder);
35+
Ok(bench_cmd)
36+
}
37+
38+
/// Dummy implementation on non-Linux platforms: the benchmark command is returned as-is.
39+
// TODO(COD-2513): implement an equivalent process-isolation mechanism on macOS
40+
#[cfg(not(target_os = "linux"))]
41+
pub fn wrap_with_isolation(bench_cmd: CommandBuilder) -> Result<CommandBuilder> {
42+
Ok(bench_cmd)
43+
}

src/executor/wall_time/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
pub mod executor;
22
pub mod helpers;
3+
pub mod isolation;
34
pub mod perf;

0 commit comments

Comments
 (0)