Skip to content

ci: remove samply walltime profiler and pinned runner version#18

Merged
not-matthias merged 5 commits into
masterfrom
fix/remove-wip-commit
Jun 22, 2026
Merged

ci: remove samply walltime profiler and pinned runner version#18
not-matthias merged 5 commits into
masterfrom
fix/remove-wip-commit

Conversation

@not-matthias

@not-matthias not-matthias commented Jun 22, 2026

Copy link
Copy Markdown
Member

Summary

  • Reverts the WIP commit that pinned the runner to 4.17.7-alpha.1 to test samply
  • Removes the CODSPEED_WALLTIME_PROFILER: samply env var from the benchmarks workflow step

The CI workflow returns to using CodSpeedHQ/action@main with the default runner version and default walltime profiler.

Test plan

  • CI runs the benchmarks step successfully

@greptile-apps

greptile-apps Bot commented Jun 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR cleans up a WIP experiment: it removes the CODSPEED_WALLTIME_PROFILER: samply env var and the pinned runner-version: 4.17.7-alpha.1 from the benchmarks workflow step, returning to the default runner and walltime profiler via CodSpeedHQ/action@main. It also extends the cache to include the .capstone directory (with a bumped v2 key to bust stale caches) and reduces the job timeout from 30 to 15 minutes.

  • Two stress-ng CPU-stress commands are removed from bench/generate_config.py, consistent with the samply experiment being rolled back.
  • The .capstone cache addition and v2 key bump ensure cache hits no longer break the make install step that needs the Capstone headers.

Confidence Score: 5/5

Safe to merge — this reverts a WIP experiment and restores the CI workflow to its previous stable state.

All changes are revertive: the samply env var and pinned alpha runner version are removed, returning the workflow to its default configuration. The only additive changes (.capstone cache path, v2 key bump, timeout reduction) are well-reasoned and consistent with the diff's intent. No logic changes were made to benchmark generation beyond removing the two stress-ng commands that were only relevant to the samply experiment.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/codspeed.yml Reverts samply profiler env var and pinned runner version; adds .capstone to the cache path with a bumped v2 key; reduces timeout from 30 to 15 minutes; all changes are intentional and consistent with the PR description.
bench/generate_config.py Removes two stress-ng benchmark commands that were added for samply profiler testing; the remaining command list is clean, though stress-ng is still installed in the workflow step.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant GH as GitHub Actions
    participant Cache as actions/cache
    participant Build as Build Steps
    participant CodSpeed as CodSpeedHQ/action@main

    GH->>Cache: "Restore valgrind-{ver}-{os}-v2-{hash}"
    note over Cache: Paths: /tmp/valgrind-build + .capstone

    alt Cache miss
        Cache-->>Build: No hit
        Build->>Build: apt-get install deps
        Build->>Build: Build Capstone (local only)
        Build->>Build: "just build {valgrind}"
        Build->>Cache: Save cache (both paths)
    else Cache hit
        Cache-->>Build: Restored /tmp/valgrind-build + .capstone
    end

    Build->>Build: "just install {valgrind}"
    Build->>Build: "generate_config.py -> codspeed.yml"
    note over Build: 3 commands x 5 configs (minus requires_codspeed skips)

    Build->>CodSpeed: Run benchmarks (mode: walltime, default runner)
    note over CodSpeed: No CODSPEED_WALLTIME_PROFILER env var
    CodSpeed-->>GH: Results uploaded
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant GH as GitHub Actions
    participant Cache as actions/cache
    participant Build as Build Steps
    participant CodSpeed as CodSpeedHQ/action@main

    GH->>Cache: "Restore valgrind-{ver}-{os}-v2-{hash}"
    note over Cache: Paths: /tmp/valgrind-build + .capstone

    alt Cache miss
        Cache-->>Build: No hit
        Build->>Build: apt-get install deps
        Build->>Build: Build Capstone (local only)
        Build->>Build: "just build {valgrind}"
        Build->>Cache: Save cache (both paths)
    else Cache hit
        Cache-->>Build: Restored /tmp/valgrind-build + .capstone
    end

    Build->>Build: "just install {valgrind}"
    Build->>Build: "generate_config.py -> codspeed.yml"
    note over Build: 3 commands x 5 configs (minus requires_codspeed skips)

    Build->>CodSpeed: Run benchmarks (mode: walltime, default runner)
    note over CodSpeed: No CODSPEED_WALLTIME_PROFILER env var
    CodSpeed-->>GH: Results uploaded
Loading

Reviews (7): Last reviewed commit: "fix(bench): drop stress-ng benchmarks" | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Jun 22, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 48.6%

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 40 untouched benchmarks
⏩ 88 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_valgrind[valgrind-3.25.1, echo Hello, World!, full-no-inline] 567.6 ms 3,101.9 ms -81.7%
test_valgrind[valgrind-3.26.0, python3 testdata/test.py, full-no-inline] 10.3 s 7.1 s +44.38%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing fix/remove-wip-commit (e0e62db) with master (2bc9a1c)

Open in CodSpeed

Footnotes

  1. 88 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@not-matthias not-matthias force-pushed the fix/remove-wip-commit branch from 9c0f6d1 to d4fc94b Compare June 22, 2026 15:35
On a cache hit the Build Capstone step is skipped, so .capstone is
absent and 'just install local' fails when make recompiles
cycledecode.o (capstone/capstone.h not found). Cache .capstone
alongside the build tree and bump the cache key to repopulate.
The local 'CodSpeed Benchmarks' job kept hitting its job timeout on the
stress-ng rows. stress-ng is the only forking workload in the suite and
deadlocks under 'callgrind --trace-children=yes' on a lost pause() wakeup.

Root cause is an application-level race in stress-ng, not our code and not
a Valgrind signal-delivery bug. Its termination wait

    while (stress_continue(args))
        (void)shim_pause();

can miss the SIGALRM that clears the continue flag if the signal lands
between the flag check and pause(), so pause() blocks on a signal that
already arrived. stress-ng's alarm(1) re-alarm mitigation fails here because
alarm() is per-process (a forked worker that already lost its SIGALRM has no
self-armed alarm) and Valgrind serializes all guest threads onto one
scheduler lock, widening the check->pause() window from nanoseconds to
milliseconds. It reproduces on stock upstream Valgrind 3.26.0/3.25.1 too; the
only thing special about 'local' is that it is slower and runs extra configs,
so it trips the race far more often.

In practice only the full-* configs (--trace-children + cache-sim, the
slowest) hang; take_strings/echo/python3 are unaffected. stress-ng adds
little value as a callgrind throughput benchmark, so remove it rather than
work around an upstream bug. The 'timeout --kill-after=10s 120s' wrapper from
the previous commit stays as a backstop. (--fair-sched=yes was tried and
regressed the hang onto take_strings full-with-inline, so it was reverted.)
@not-matthias not-matthias merged commit e0e62db into master Jun 22, 2026
8 of 9 checks passed
@not-matthias not-matthias deleted the fix/remove-wip-commit branch June 22, 2026 16:14
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