Skip to content

feat: make samply profiler default for all OSes (#472) - #475

Merged
GuillaumeLagrange merged 1 commit into
mainfrom
cod-2796-switch-to-samply-profiler-by-default
Jul 27, 2026
Merged

feat: make samply profiler default for all OSes (#472)#475
GuillaumeLagrange merged 1 commit into
mainfrom
cod-2796-switch-to-samply-profiler-by-default

Conversation

@GuillaumeLagrange

Copy link
Copy Markdown
Contributor

Previously merged with #472, but it got "reverted" because of force push

@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown

Greptile Summary

Makes Samply the default wall-time profiler on every supported OS.

  • Adds an internal executable override so profiler subprocesses can re-enter the CodSpeed CLI reliably.
  • Runs wall-time and memory executor tests without the previous CI-only gate and builds required workspace binaries on demand.
  • Updates the Samply submodule and removes redundant CI installation and build-script logic.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/executor/wall_time/executor.rs Changes the implicit wall-time profiler from platform-specific selection to Samply on all operating systems.
src/cli/mod.rs Adds an environment-variable override for the executable used to dispatch internal Samply subcommands.
src/executor/tests.rs Makes executor tests self-contained by building and explicitly selecting the required workspace binaries.
Cargo.toml Adds Escargot as a development dependency for locating binaries built during tests.
.github/workflows/ci.yml Removes the now-redundant explicit exec-harness installation step.

Sequence Diagram

sequenceDiagram
    participant Executor as WallTimeExecutor
    participant Profiler as SamplyProfiler
    participant CLI as CodSpeed internal CLI
    participant Benchmark
    Executor->>Profiler: Select default profiler
    Profiler->>CLI: Re-exec CODSPEED_SELF_EXE samply
    CLI->>Benchmark: Record benchmark process
    Benchmark-->>Profiler: Profiling data
    Profiler-->>Executor: Finalized wall-time artifacts
Loading

Reviews (2): Last reviewed commit: "feat: make samply profiler default for a..." | Re-trigger Greptile

@GuillaumeLagrange
GuillaumeLagrange force-pushed the cod-2796-switch-to-samply-profiler-by-default branch from 2b5b079 to 228b86f Compare July 27, 2026 15:19
@codspeed-hq

codspeed-hq Bot commented Jul 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 17 untouched benchmarks


Comparing cod-2796-switch-to-samply-profiler-by-default (266f658) with main (97a789a)

Open in CodSpeed

* feat(samply-codspeed): bump submodule to forward signal exit code

* chore: fix self-exec failures in tests and ungate executor tests

Tests no longer require sudo, or at least they dont after the kernel
parameters are set, and the memory executor tests now run in a
reasonable time thanks to the attach on demand, we dont need to gate
anymore.

Also add a way for the test executable to find the actual codspeed
binary in order to have the self exec into samply work properly. It used
to re-exec the test executable, which is not the behavior we want here.

* feat: make samply profiler default for all OSes
@GuillaumeLagrange
GuillaumeLagrange force-pushed the cod-2796-switch-to-samply-profiler-by-default branch from 228b86f to 266f658 Compare July 27, 2026 16:20
@GuillaumeLagrange

Copy link
Copy Markdown
Contributor Author

@greptileai review

@GuillaumeLagrange
GuillaumeLagrange merged commit 266f658 into main Jul 27, 2026
23 checks passed
@GuillaumeLagrange
GuillaumeLagrange deleted the cod-2796-switch-to-samply-profiler-by-default branch July 27, 2026 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants