mgradwohl / mgradwohl/tasksmack

[Perf][Tooling] Several C++ benchmarks measure the wrong thing (size not payload, foldable constants, timed input generation)

Open
#877 0 comments 0 reactions 0 assignees View on GitHub
performance testing tooling
Dominant language
C++
Stars
1
Forks
0
Avg merge
3h 4m
Merged PRs (30d)
115

Description

## Problem

Several existing benchmarks have measurement-validity issues (Medium priority, from #843's
tooling-trust review):

- `benchmarks/bench_History.cpp` often observes ring-buffer *size* rather than the actual stored
payload cost.
- `benchmarks/bench_Numeric.cpp:160-167` permits the compiler to fold a constant-overflow case
away entirely, potentially measuring nothing.
- Several format benchmarks include random-input generation *inside* the timed loop, inflating
measured cost with unrelated RNG overhead.
- Ring-buffer capacity claimed as "parameterized" in some benchmark names isn't actually varied.

## Recommendation

Make the relevant payloads/runtime inputs observable to the optimizer (e.g. via
`benchmark::DoNotOptimize`/`ClobberMemory` as appropriate) so they can't be folded away.
Pre-generate input pools outside the timed region. Actually parameterize ring capacity where a
benchmark's name/intent claims it does. Inspect generated assembly for at least one representative
case to confirm the intended code path is actually being measured. Rebaseline
`perf-data/linux-ci-baseline.json` (see #683) if any of these fixes change a benchmark's
semantics enough to invalidate the existing baseline comparison for it.

## Acceptance criteria

- `bench_History.cpp`'s relevant benchmarks measure payload cost, not just buffer size.
- `bench_Numeric.cpp`'s constant-overflow case cannot be folded away by the optimizer (verified
via generated assembly or a `DoNotOptimize` fix).
- Format benchmarks generate random input outside the timed loop.
- Any benchmark whose semantics change as part of this fix has its baseline entry refreshed.

## Additional Context

From #843's Phase 0 tooling-trust review addendum ("Concrete performance-tooling work" table,
Medium-priority benchmark-measurement row).

Contributor guide

Open the contributing guide

Research direction

Start by inspecting benchmarks/bench_History.cpp and benchmarks/bench_Numeric.cpp:160-167, then locate the format benchmarks and run representative cases. Check generated assembly for the constant-overflow benchmark and review the existing entries in perf-data/linux-ci-baseline.json. Done means payload cost is measured, random input is generated outside timed loops, claimed ring capacities vary, and affected baselines are refreshed.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
performance, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.