Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ pub async fn run(args: RunArgs, api_client: &CodSpeedAPIClient) -> Result<()> {
let system_info = SystemInfo::new()?;
check_system::check_system(&system_info)?;

let executor = runner::get_executor()?;
let mode = runner::get_mode()?;
let executor = runner::get_executor_from_mode(mode);

let run_data = get_run_data()?;

Expand Down
16 changes: 0 additions & 16 deletions src/run/runner/executor.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
use super::helpers::env::BASE_INJECTED_ENV;
use super::interfaces::{ExecutorName, RunData};
use crate::prelude::*;
use crate::run::instruments::mongo_tracer::MongoTracer;
use crate::run::{check_system::SystemInfo, config::Config};
Comment on lines 2 to 4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: import grouping

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can cargo fmt handle this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind it's unstable and therefore not able to be put in rustfmt.toml, it needs to be set at IDE level 😢

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:(

use async_trait::async_trait;
use std::collections::HashMap;
use std::path::Path;

#[async_trait(?Send)]
pub trait Executor {
Expand Down Expand Up @@ -34,17 +31,4 @@ pub trait Executor {
system_info: &SystemInfo,
run_data: &RunData,
) -> Result<()>;

/// Gets the base environment for the command
///
/// Later on, we will want to refactor this and create the cmd directly in a trait function
fn get_cmd_base_envs(&self, profile_folder: &Path) -> HashMap<&str, String> {
let mut hashmap = BASE_INJECTED_ENV.clone();
hashmap.insert("CODSPEED_RUNNER_MODE", self.name().to_string());
hashmap.insert(
"CODSPEED_PROFILE_FOLDER",
profile_folder.to_str().unwrap().to_string(),
);
hashmap
}
}
26 changes: 16 additions & 10 deletions src/run/runner/helpers/env.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
use std::{collections::HashMap, env::consts::ARCH};
use std::{collections::HashMap, env::consts::ARCH, path::Path};

use lazy_static::lazy_static;
use crate::run::runner::RunnerMode;

lazy_static! {
pub static ref BASE_INJECTED_ENV: HashMap<&'static str, String> = {
HashMap::from([
("PYTHONHASHSEED", "0".into()),
("ARCH", ARCH.into()),
("CODSPEED_ENV", "runner".into()),
])
};
pub fn get_base_injected_env(
mode: RunnerMode,
profile_folder: &Path,
) -> HashMap<&'static str, String> {
HashMap::from([
("PYTHONHASHSEED", "0".into()),
("ARCH", ARCH.into()),
("CODSPEED_ENV", "runner".into()),
("CODSPEED_RUNNER_MODE", mode.to_string()),
(
"CODSPEED_PROFILE_FOLDER",
profile_folder.to_string_lossy().to_string(),
),
])
}
51 changes: 40 additions & 11 deletions src/run/runner/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::env;
use std::{env, fmt::Display};

use crate::prelude::*;

Expand All @@ -12,22 +12,51 @@ use anyhow::bail;
use executor::Executor;
use helpers::profile_folder::create_profile_folder;
pub use interfaces::{ExecutorName, RunData};
use valgrind::executor::{ValgrindExecutor, INSTRUMENTATION_RUNNER_MODE};
use wall_time::executor::{WallTimeExecutor, WALL_TIME_RUNNER_MODE};
use valgrind::executor::ValgrindExecutor;
use wall_time::executor::WallTimeExecutor;

pub enum RunnerMode {
Instrumentation,
WallTime,
}

impl Display for RunnerMode {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
RunnerMode::Instrumentation => write!(f, "instrumentation"),
RunnerMode::WallTime => write!(f, "walltime"),
}
}
}

impl TryFrom<&str> for RunnerMode {
type Error = anyhow::Error;

fn try_from(value: &str) -> Result<Self> {
match value {
"instrumentation" => Ok(RunnerMode::Instrumentation),
"walltime" => Ok(RunnerMode::WallTime),
_ => bail!("Unknown runner mode: {}", value),
}
}
}

pub const EXECUTOR_TARGET: &str = "executor";

pub fn get_executor() -> Result<Box<dyn Executor>> {
pub fn get_mode() -> Result<RunnerMode> {
if let Ok(runner_mode) = env::var("CODSPEED_RUNNER_MODE") {
debug!("CODSPEED_RUNNER_MODE is set to {}", runner_mode);
match runner_mode.as_str() {
INSTRUMENTATION_RUNNER_MODE => Ok(Box::new(ValgrindExecutor)),
WALL_TIME_RUNNER_MODE => Ok(Box::new(WallTimeExecutor)),
_ => bail!("Unknown codspeed runner mode"),
}
RunnerMode::try_from(runner_mode.as_str())
} else {
debug!("CODSPEED_RUNNER_MODE is not set, using valgrind");
Ok(Box::new(ValgrindExecutor))
debug!("CODSPEED_RUNNER_MODE is not set, using instrumentation");
Ok(RunnerMode::Instrumentation)
}
}

pub fn get_executor_from_mode(mode: RunnerMode) -> Box<dyn Executor> {
match mode {
RunnerMode::Instrumentation => Box::new(ValgrindExecutor),
RunnerMode::WallTime => Box::new(WallTimeExecutor),
}
}

Expand Down
6 changes: 1 addition & 5 deletions src/run/runner/valgrind/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ use crate::run::{check_system::SystemInfo, config::Config};

use super::{helpers::perf_maps::harvest_perf_maps, measure, setup::setup};

pub const INSTRUMENTATION_RUNNER_MODE: &str = "instrumentation";

pub struct ValgrindExecutor;

#[async_trait(?Send)]
Expand All @@ -36,10 +34,8 @@ impl Executor for ValgrindExecutor {
run_data: &RunData,
mongo_tracer: &Option<MongoTracer>,
) -> Result<()> {
let base_env = self.get_cmd_base_envs(&run_data.profile_folder);

//TODO: add valgrind version check
measure::measure(&base_env, config, &run_data.profile_folder, mongo_tracer)?;
measure::measure(config, &run_data.profile_folder, mongo_tracer)?;

Ok(())
}
Expand Down
15 changes: 9 additions & 6 deletions src/run/runner/valgrind/measure.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use crate::prelude::*;
use crate::run::runner::helpers::env::get_base_injected_env;
use crate::run::runner::helpers::get_bench_command::get_bench_command;
use crate::run::runner::helpers::run_command_with_log_pipe::run_command_with_log_pipe;
use crate::run::runner::valgrind::helpers::ignored_objects_path::get_objects_path_to_ignore;
use crate::run::runner::valgrind::helpers::introspected_nodejs::setup_introspected_nodejs;
use crate::run::runner::RunnerMode;
use crate::run::{config::Config, instruments::mongo_tracer::MongoTracer};
use lazy_static::lazy_static;
use std::collections::HashMap;
use std::env;
use std::fs::canonicalize;
use std::path::Path;
Expand Down Expand Up @@ -42,17 +43,20 @@ lazy_static! {
}

pub fn measure(
base_env: &HashMap<&str, String>,
config: &Config,
profile_folder: &Path,
mongo_tracer: &Option<MongoTracer>,
) -> Result<()> {
// Create the command
let mut cmd = Command::new("setarch");
cmd.envs(base_env);
cmd.arg(ARCH).arg("-R");
// Configure the environment
cmd.env(
cmd.envs(get_base_injected_env(
RunnerMode::Instrumentation,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have the executor define its RunnerMode?

So that we do not have to hardcode it here, and instead have it as a method of the Executor trait

Copy link
Member Author

@art049 art049 Feb 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not useful with the valgrind executor since it needs refactoring to be properly implemented. We could drill the mode into measure but that would be pointless.

profile_folder,
))
.env("PYTHONMALLOC", "malloc")
.env(
"PATH",
format!(
"{}:{}",
Expand All @@ -62,8 +66,7 @@ pub fn measure(
.unwrap(),
env::var("PATH").unwrap_or_default(),
),
)
.env("PYTHONMALLOC", "malloc");
);

if let Some(cwd) = &config.working_directory {
let abs_cwd = canonicalize(cwd)?;
Expand Down
11 changes: 7 additions & 4 deletions src/run/runner/wall_time/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@ use crate::prelude::*;

use crate::run::instruments::mongo_tracer::MongoTracer;
use crate::run::runner::executor::Executor;
use crate::run::runner::helpers::env::get_base_injected_env;
use crate::run::runner::helpers::get_bench_command::get_bench_command;
use crate::run::runner::helpers::run_command_with_log_pipe::run_command_with_log_pipe;
use crate::run::runner::{ExecutorName, RunData};
use crate::run::runner::{ExecutorName, RunData, RunnerMode};
use crate::run::{check_system::SystemInfo, config::Config};
use async_trait::async_trait;
use std::fs::canonicalize;
use std::process::Command;

pub const WALL_TIME_RUNNER_MODE: &str = "walltime";

pub struct WallTimeExecutor;

#[async_trait(?Send)]
Expand All @@ -37,7 +36,11 @@ impl Executor for WallTimeExecutor {
_mongo_tracer: &Option<MongoTracer>,
) -> Result<()> {
let mut cmd = Command::new("sh");
cmd.envs(self.get_cmd_base_envs(&run_data.profile_folder));

cmd.envs(get_base_injected_env(
RunnerMode::WallTime,
&run_data.profile_folder,
));

if let Some(cwd) = &config.working_directory {
let abs_cwd = canonicalize(cwd)?;
Expand Down