Skip to content

feat(cli): add walltime unwinding mode and stack sample size options #586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
not-matthias wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(cli): add walltime unwinding mode and stack sample size options #586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e2212c
feat(cli): add walltime unwinding mode and stack sample size options
not-matthias Oct 13, 2026
3b356c9
fix(samply): pass stack sample size as --unwinding-stack-size
not-matthias Oct 13, 2026
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
11 changes: 10 additions & 1 deletion src/cli/exec/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,16 @@ fn build_orchestrator_config(
targets: vec![target],
modes,
instruments: Instruments { mongodb: None }, // exec doesn't support MongoDB
perf_unwinding_mode: args.shared.profiler_run_args.perf.perf_unwinding_mode,
walltime_unwinding_mode: args
.shared
.profiler_run_args
.walltime
.resolve_unwinding_mode(),
walltime_stack_sample_size: args
.shared
.profiler_run_args
.walltime
.walltime_stack_sample_size,
enable_profiler: args.shared.profiler_run_args.resolve_enable_profiler(),
walltime_profiler: args.shared.walltime_profiler,
simulation_tool: args.shared.simulation_tool.unwrap_or_default(),
Expand Down
17 changes: 14 additions & 3 deletions src/cli/run/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl RunArgs {
/// Constructs a new `RunArgs` with default values for testing purposes
pub fn test() -> Self {
use super::experimental::ExperimentalArgs;
use super::{PerfRunArgs, ProfilerRunArgs};
use super::{ProfilerRunArgs, WalltimeRunArgs};
use crate::RunnerMode;

Self {
Expand Down Expand Up @@ -77,8 +77,10 @@ impl RunArgs {
profiler_run_args: ProfilerRunArgs {
enable_profiler: false,
enable_perf: None,
perf: PerfRunArgs {
walltime: WalltimeRunArgs {
walltime_unwinding_mode: None,
perf_unwinding_mode: None,
walltime_stack_sample_size: None,
},
},
experimental: ExperimentalArgs {
Expand Down Expand Up @@ -127,7 +129,16 @@ fn build_orchestrator_config(
targets,
modes,
instruments,
perf_unwinding_mode: args.shared.profiler_run_args.perf.perf_unwinding_mode,
walltime_unwinding_mode: args
.shared
.profiler_run_args
.walltime
.resolve_unwinding_mode(),
walltime_stack_sample_size: args
.shared
.profiler_run_args
.walltime
.walltime_stack_sample_size,
enable_profiler: args.shared.profiler_run_args.resolve_enable_profiler(),
walltime_profiler: args.shared.walltime_profiler,
simulation_tool: args.shared.simulation_tool.unwrap_or_default(),
Expand Down
32 changes: 28 additions & 4 deletions src/cli/shared.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,38 @@ pub struct ProfilerRunArgs {
pub enable_perf: Option<bool>,

#[command(flatten)]
pub perf: PerfRunArgs,
pub walltime: WalltimeRunArgs,
}

#[derive(Args, Debug, Clone)]
pub struct PerfRunArgs {
/// The unwinding mode that should be used with perf to collect the call stack.
#[arg(long, env = "CODSPEED_PERF_UNWINDING_MODE")]
pub struct WalltimeRunArgs {
/// The unwinding mode that the walltime profiler should use to collect the call stack.
#[arg(long, env = "CODSPEED_WALLTIME_UNWINDING_MODE")]
pub walltime_unwinding_mode: Option<UnwindingMode>,

/// Deprecated alias for --walltime-unwinding-mode / CODSPEED_WALLTIME_UNWINDING_MODE.
#[arg(long, env = "CODSPEED_PERF_UNWINDING_MODE", hide = true)]
pub perf_unwinding_mode: Option<UnwindingMode>,

/// Size in bytes of the user stack copied with each sample for DWARF unwinding.
/// Larger values produce deeper call stacks but bigger profiles.
#[arg(long, env = "CODSPEED_WALLTIME_STACK_SAMPLE_SIZE")]
pub walltime_stack_sample_size: Option<u32>,
}

impl WalltimeRunArgs {
/// Resolves the effective unwinding mode, honoring the deprecated
/// `--perf-unwinding-mode` / `CODSPEED_PERF_UNWINDING_MODE` flag with a warning.
/// The new flag takes precedence when both are set.
pub fn resolve_unwinding_mode(&self) -> Option<UnwindingMode> {
let Some(legacy) = self.perf_unwinding_mode else {
return self.walltime_unwinding_mode;
};
log::warn!(
"CODSPEED_PERF_UNWINDING_MODE / --perf-unwinding-mode is deprecated; use CODSPEED_WALLTIME_UNWINDING_MODE / --walltime-unwinding-mode instead."
);
Comment on lines +258 to +260

@greptile-apps greptile-apps Bot Oct 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Deprecation warning never appears

Both codspeed run and codspeed exec call resolve_unwinding_mode() while building the config, before Orchestrator::new() installs the logger. This drops the warning, so users of --perf-unwinding-mode or CODSPEED_PERF_UNWINDING_MODE never learn about the replacement. Emit it after logger setup or use the existing startup-warning approach.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/cli/shared.rs
Line: 258-260

Comment:
**Deprecation warning never appears**

Both `codspeed run` and `codspeed exec` call `resolve_unwinding_mode()` while building the config, before `Orchestrator::new()` installs the logger. This drops the warning, so users of `--perf-unwinding-mode` or `CODSPEED_PERF_UNWINDING_MODE` never learn about the replacement. Emit it after logger setup or use the existing startup-warning approach.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

Some(self.walltime_unwinding_mode.unwrap_or(legacy))
Comment on lines +254 to +261

@greptile-apps greptile-apps Bot Oct 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Option rules lack tests

The new precedence and stack-size overrides have no tests. Add focused cases for the old option alone, both options together, flag/environment combinations, and an explicit size overriding both the default and Python's inferred size. These are the main promises of this change; tests would catch later edits that silently send the wrong value to the profiler.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/cli/shared.rs
Line: 254-261

Comment:
**Option rules lack tests**

The new precedence and stack-size overrides have no tests. Add focused cases for the old option alone, both options together, flag/environment combinations, and an explicit size overriding both the default and Python's inferred size. These are the main promises of this change; tests would catch later edits that silently send the wrong value to the profiler.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

}
}

impl ProfilerRunArgs {
Expand Down
18 changes: 12 additions & 6 deletions src/executor/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ pub struct OrchestratorConfig {
pub modes: Vec<RunnerMode>,
pub instruments: Instruments,
pub enable_profiler: bool,
/// Stack unwinding mode for perf (if enabled)
pub perf_unwinding_mode: Option<UnwindingMode>,
/// Stack unwinding mode for the walltime profiler (if None, inferred from the command)
pub walltime_unwinding_mode: Option<UnwindingMode>,
/// User stack size in bytes copied per sample for DWARF unwinding (if None, profiler default)
pub walltime_stack_sample_size: Option<u32>,
/// Profiler override for walltime mode (if None, selected based on the platform)
pub walltime_profiler: Option<WalltimeProfiler>,

Expand Down Expand Up @@ -120,8 +122,10 @@ pub struct ExecutorConfig {

pub instruments: Instruments,
pub enable_profiler: bool,
/// Stack unwinding mode for perf (if enabled)
pub perf_unwinding_mode: Option<UnwindingMode>,
/// Stack unwinding mode for the walltime profiler (if None, inferred from the command)
pub walltime_unwinding_mode: Option<UnwindingMode>,
/// User stack size in bytes copied per sample for DWARF unwinding (if None, profiler default)
pub walltime_stack_sample_size: Option<u32>,

pub simulation_tool: SimulationTool,

Expand Down Expand Up @@ -218,7 +222,8 @@ impl OrchestratorConfig {
command,
instruments: self.instruments.clone(),
enable_profiler: self.enable_profiler,
perf_unwinding_mode: self.perf_unwinding_mode,
walltime_unwinding_mode: self.walltime_unwinding_mode,
walltime_stack_sample_size: self.walltime_stack_sample_size,
simulation_tool: self.simulation_tool,
skip_run: self.skip_run,
skip_setup: self.skip_setup,
Expand Down Expand Up @@ -251,7 +256,8 @@ impl OrchestratorConfig {
}],
modes: vec![RunnerMode::Simulation],
instruments: Instruments::test(),
perf_unwinding_mode: None,
walltime_unwinding_mode: None,
walltime_stack_sample_size: None,
walltime_profiler: None,
enable_profiler: false,
simulation_tool: SimulationTool::default(),
Expand Down
4 changes: 2 additions & 2 deletions src/executor/helpers/introspected_golang/go.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ if [ $# -eq 0 ]; then
exit $?
fi

# On arm64, warn about setting CODSPEED_PERF_UNWINDING_MODE to "fp" for correct flamegraphs
# On arm64, warn about setting CODSPEED_WALLTIME_UNWINDING_MODE to "fp" for correct flamegraphs
if [ "$(uname -m)" = "aarch64" ] && [ "${CODSPEED_GO_SUPPRESS_PERF_UNWINDING_MODE_WARNING:-}" != "true" ]; then
echo "::warning::Go profiling on arm64 require frame pointer unwinding. Set CODSPEED_PERF_UNWINDING_MODE=fp for better profiling." >&2
echo "::warning::Go profiling on arm64 require frame pointer unwinding. Set CODSPEED_WALLTIME_UNWINDING_MODE=fp for better profiling." >&2
fi

# Route command based on first argument
Expand Down
10 changes: 8 additions & 2 deletions src/executor/wall_time/profiler/perf/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl Profiler for PerfProfiler {
let perf_file_path = profile_folder.join(PERF_PIPEDATA_FILE_NAME);

// Infer the unwinding mode from the benchmark cmd
let (cg_mode, stack_size) = if let Some(mode) = config.perf_unwinding_mode {
let (cg_mode, stack_size) = if let Some(mode) = config.walltime_unwinding_mode {
(mode, None)
} else if command_has_executable(
&config.command,
Expand All @@ -117,7 +117,13 @@ impl Profiler for PerfProfiler {
suppress_go_perf_unwinding_warning();
"fp"
}
UnwindingMode::Dwarf => &format!("dwarf,{}", stack_size.unwrap_or(8192)),
UnwindingMode::Dwarf => &format!(
"dwarf,{}",
config
.walltime_stack_sample_size
.or(stack_size)
.unwrap_or(8192)
),
};
debug!("Using call graph mode: {cg_mode:?}");

Expand Down
46 changes: 31 additions & 15 deletions src/executor/wall_time/profiler/samply/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Samply profiler integration.

use crate::cli::InternalCommands;
#[cfg(target_os = "linux")]
use crate::cli::UnwindingMode;
use crate::cli::samply::SamplyArgs;
use crate::executor::ExecutorConfig;
use crate::executor::helpers::command::CommandBuilder;
Expand Down Expand Up @@ -81,28 +83,42 @@ impl Profiler for SamplyProfiler {
async fn wrap_command(
&mut self,
mut cmd_builder: CommandBuilder,
_config: &ExecutorConfig,
config: &ExecutorConfig,
profile_folder: &Path,
requires_sudo: bool,
) -> anyhow::Result<CommandBuilder> {
let output_path = profile_folder.join(SAMPLY_OUTPUT_FILE_NAME);

// samply is bundled into this binary as the `samply` subcommand;
// re-exec ourselves so we don't depend on a system install.
let samply_builder = InternalCommands::Samply(SamplyArgs {
args: vec![
"record".into(),
"--presymbolicate".into(),
"--no-open".into(),
"--save-only".into(),
"--rate".into(),
SAMPLING_RATE_HZ.to_string().into(),
"-o".into(),
output_path.clone().into(),
"--".into(),
],
})
.get_command_builder()?;
let mut args: Vec<std::ffi::OsString> = vec![
"record".into(),
"--presymbolicate".into(),
"--no-open".into(),
"--save-only".into(),
"--rate".into(),
SAMPLING_RATE_HZ.to_string().into(),
"-o".into(),
output_path.clone().into(),
];
// Unwinding knobs are Linux-only in samply.
#[cfg(target_os = "linux")]
{
if let Some(mode) = config.walltime_unwinding_mode {
let mode = match mode {
UnwindingMode::FramePointer => "fp",
UnwindingMode::Dwarf => "dwarf",
};
args.extend(["--unwinding-mode".into(), mode.into()]);
}
if let Some(size) = config.walltime_stack_sample_size {
args.extend(["--unwinding-stack-size".into(), size.to_string().into()]);
}
}
#[cfg(not(target_os = "linux"))]
let _ = config;
args.push("--".into());
let samply_builder = InternalCommands::Samply(SamplyArgs { args }).get_command_builder()?;

cmd_builder.wrap_with(samply_builder);

Expand Down
Loading