Skip to content

fix(skills): misleading DCE advice in setup-harness#350

Open
SuperMuel wants to merge 4 commits into
mainfrom
fix/skill-pedantic-not-for-dce
Open

fix(skills): misleading DCE advice in setup-harness#350
SuperMuel wants to merge 4 commits into
mainfrom
fix/skill-pedantic-not-for-dce

Conversation

@SuperMuel
Copy link
Copy Markdown

@SuperMuel SuperMuel commented May 14, 2026

The skill told users to use benchmark.pedantic() in Python to prevent dead code elimination, like black_box() in Rust. Two problems with that.

benchmark.pedantic does not prevent dead code elimination. It just lets you set rounds, iterations, and a setup function for the benchmark.

CPython does not delete unused function calls. It can't know if a function has side effects, so it always runs it. Pure-Python benchmarks do not need a black_box.

The new wording only mentions compiled languages and uses the right name for each one.

`benchmark.pedantic` is not a dead-code-elimination tool; it controls
the measurement loop. CPython doesn't perform the kind of DCE that
`black_box` guards against in compiled languages, so pure-Python
benchmarks don't need an equivalent. List the actual primitives for
the languages that do (C++: DoNotOptimize, JMH: Blackhole.consume).
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 14, 2026

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.

✅ 7 untouched benchmarks


Comparing fix/skill-pedantic-not-for-dce (d395403) with main (149a618)

Open in CodSpeed

Caught by the trailing-whitespace pre-commit hook on PR #350.
@SuperMuel SuperMuel requested a review from art049 May 14, 2026 21:29
SuperMuel added 2 commits May 15, 2026 16:43
`rustup show` stopped auto-installing the active toolchain in rustup
1.28 (March 2025). The change in 9afaef7 happened to keep working
because the macOS-15-arm64 runner image at the time still shipped an
older rustup; once GitHub rolled out image 20260511.0048 with rustup
>= 1.28, /Users/runner/.cargo/bin/cargo stayed as the rustup-init shim
and `cargo fmt`/`cargo clippy` started failing in lint (macos-latest)
with `unexpected argument 'fmt' found`.

Per the rustup 1.28 release notes, the explicit replacement is
`rustup toolchain install`, which is what this action used before.
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.

1 participant