Skip to content

Commit cb76ebf

Browse files
committed
fix(perf): run with --scope to allow perf to trace the benchmark process
1 parent 2631a25 commit cb76ebf

1 file changed

Lines changed: 49 additions & 15 deletions

File tree

src/run/runner/wall_time/executor.rs

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,25 @@ impl Drop for HookScriptsGuard {
6363
}
6464
}
6565

66+
/// Returns a list of exported environment variables which can be loaded with `source` in a shell.
67+
///
68+
/// Example: `declare -x outputs="out"`
69+
fn get_exported_system_env() -> Result<String> {
70+
let output = Command::new("bash")
71+
.arg("-c")
72+
.arg("export")
73+
.output()
74+
.context("Failed to run `export`")?;
75+
if !output.status.success() {
76+
bail!(
77+
"Failed to get system environment variables: {}",
78+
String::from_utf8_lossy(&output.stderr)
79+
);
80+
}
81+
82+
String::from_utf8(output.stdout).context("Failed to parse export output as UTF-8")
83+
}
84+
6685
pub struct WallTimeExecutor {
6786
perf: Option<PerfRunner>,
6887
}
@@ -74,36 +93,51 @@ impl WallTimeExecutor {
7493
}
7594
}
7695

77-
fn walltime_bench_cmd(config: &Config, run_data: &RunData) -> Result<(NamedTempFile, String)> {
96+
fn walltime_bench_cmd(
97+
config: &Config,
98+
run_data: &RunData,
99+
) -> Result<(NamedTempFile, NamedTempFile, String)> {
78100
let bench_cmd = get_bench_command(config)?;
79101

80-
let combined_env = std::env::vars()
81-
.chain(
82-
get_base_injected_env(RunnerMode::Walltime, &run_data.profile_folder)
83-
.into_iter()
84-
.map(|(k, v)| (k.into(), v)),
85-
)
86-
.map(|(env, value)| format!("{env}={value}"))
87-
.collect::<Vec<_>>()
88-
.join("\n");
102+
let system_env = get_exported_system_env()?;
103+
let base_injected_env =
104+
get_base_injected_env(RunnerMode::Walltime, &run_data.profile_folder)
105+
.into_iter()
106+
.map(|(k, v)| format!("export {k}={v}",))
107+
.collect::<Vec<_>>()
108+
.join("\n");
109+
let combined_env = format!("{system_env}\n{base_injected_env}");
89110

90111
let mut env_file = NamedTempFile::new()?;
91112
env_file.write_all(combined_env.as_bytes())?;
92113

114+
let script_file = {
115+
let mut file = NamedTempFile::new()?;
116+
let bash_command = format!("source {} && {}", env_file.path().display(), bench_cmd);
117+
debug!("Bash command: {bash_command}");
118+
file.write_all(bash_command.as_bytes())?;
119+
file
120+
};
121+
93122
let quiet_flag = if is_codspeed_debug_enabled() {
94123
"--quiet"
95124
} else {
96125
""
97126
};
98127

128+
// Remarks:
129+
// - We're using --scope so that perf is able to capture the events of the benchmark process.
130+
// - We can't user `--user` here because we need to run in `codspeed.slice`, otherwise we'd run in
131+
// user.slice` (which is isolated). We can use `--gid` and `--uid` to run the command as the current user.
132+
// - We must use `bash` here instead of `sh` since `source` isn't available when symlinked to `dash`.
133+
// - We have to pass the environment variables because `--scope` only inherits the system and not the user environment variables.
99134
let uid = nix::unistd::Uid::current().as_raw();
100135
let gid = nix::unistd::Gid::current().as_raw();
101136
let cmd = format!(
102-
"systemd-run {quiet_flag} --pipe --collect --wait --slice=codspeed.slice --same-dir --uid={uid} --gid={gid} --property=EnvironmentFile={} -- sh -c '{}'",
103-
env_file.path().display(),
104-
bench_cmd.replace("'", "\"")
137+
"systemd-run {quiet_flag} --scope --slice=codspeed.slice --same-dir --uid={uid} --gid={gid} -- bash {}",
138+
script_file.path().display()
105139
);
106-
Ok((env_file, cmd))
140+
Ok((env_file, script_file, cmd))
107141
}
108142
}
109143

@@ -138,7 +172,7 @@ impl Executor for WallTimeExecutor {
138172
let status = {
139173
let _guard = HookScriptsGuard::setup();
140174

141-
let (_env_file, bench_cmd) = Self::walltime_bench_cmd(config, run_data)?;
175+
let (_env_file, _script_file, bench_cmd) = Self::walltime_bench_cmd(config, run_data)?;
142176
if let Some(perf) = &self.perf
143177
&& config.enable_perf
144178
{

0 commit comments

Comments
 (0)