Skip to content

Keep the simulated caches warm, and choose defaults in Configuration - #4

Merged
ondrejmirtes merged 4 commits into
1.xfrom
tally-config-defaults
Nov 14, 2026
Merged

ondrejmirtes merged 4 commits into
1.xfrom
tally-config-defaults

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Two changes, one commit each.

Keep the simulated caches warm from one span to the next. Callgrind empties its simulated caches whenever instrumentation starts. Tally used to start it for every span, as codspeed run does, so every cache miss inside a span was a first-touch miss. As a result, the cycles of a benchmark of a few thousand instructions moved by 10–20 % whenever the code just before it touched different cache lines. That is what made phpstan-src's tiny benchmarks fail when Tally switched from instrument-hooks to tally_hooks. The counter now starts instrumentation once, after the #[Before] methods, and only toggles collection for each span, so the #[Warmup] calls fill the caches the counted calls use. Instruction counts stay the same as CodSpeed's.

I measured this locally on PHP 8.5.12 with phpstan-src's 44 smallest benchmarks, adding a few dozen unrelated instructions to the hook path. The mean cycle shift fell from 10.3 % to 3.5 %, and the worst from 22 % to 14 %. Instructions didn't change. WarmupBench::benchWarm went from 13.4K to 2.1K cycles, with 887 instructions under both designs.

Choose the default tolerance in Configuration, tuned for microbenchmarks. In phpstan-src CI, the same code measures within 0.53 % on instructions and 1.6 % on cycles from run to run, so ±10 % on every metric was far too loose. The new default is MetricTolerance(instructions: 0.02, cycles: 0.05, memory: 0.05, cyclesAtLeast: 2000). Configuration::withTolerance() replaces it. CaseExpander::DEFAULT_TOLERANCE is gone, so every default now lives in Configuration.

Configuration::withNamedArguments(true) makes tally accept, and the patch it prints for new expectations, write #[ExpectedFor(dataSet: '…')].

The cycles of Tally's own bench/ and examples/ will change, and the tighter default may fail some of them. Their numbers have to come from this PR's Benchmarks job.

Callgrind empties its simulated caches whenever the instrumentation starts.
Starting it for every span, as codspeed run does, left every miss of a span
a compulsory one, so the cycles of a benchmark of a few thousand instructions
moved by 10-20 % when the code that ran just before it touched other cache
lines, such as when Tally moved from instrument-hooks over FFI to tally_hooks.

The counter now starts the instrumentation once, after the before methods,
and only toggles collecting for each span, so the warm-up calls fill the
caches the counted calls find. Instructions stay those of CodSpeed; the
cycles of small benchmarks drop, and shift by a third as much when unrelated
code changes.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
±10 % of every metric was far wider than what varies between runs of the same
code, a few dozen instructions, yet too narrow for the cycles of a benchmark of
a few thousand instructions, which gains or loses a dozen cache misses when
unrelated code changes. The default is now MetricTolerance: ±2 % instructions,
±5 % memory, and ±5 % cycles but at least 2,000 cycles. withTolerance()
replaces it, so every default lives in Configuration and is passed down from
there.

withNamedArguments(true) makes tally accept and the patch of new expectations
write #[ExpectedFor(dataSet: '…')], for projects whose benchmarks name it.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
A complexity class bounds how the instructions grow, and the code around the
part that grows makes the counts differ from the formula by a few percent: up
to 5 % in the examples and 9 % in PHPStan's benchmarks. ±2 % of instructions
failed those, so cases with a complexity class and no policy of their own get
withGrowthTolerance(), ±10 % by default.

Tally's own benchmarks of the report and sharding build their cases with a
fixed ±10 % again, so their input does not change with the default.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
The cycles of most benchmarks dropped now that the warm-up calls fill the
simulated caches. CaseExpanderBench was already 7 % above its expectation on
1.x, which the old ±10 % default hid.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
@ondrejmirtes
ondrejmirtes merged commit d64aac2 into 1.x Nov 14, 2026
27 checks passed
@ondrejmirtes
ondrejmirtes deleted the tally-config-defaults branch November 14, 2026 10:08
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