Repository navigation
Do not fail the time gate on a benchmark too small to time - #1044
Merged
Merged
Conversation
EvalEasy went 0.98 ns -> 3.52 ns on #1027, a branch that changes exception message strings and a documentation file, and failed the gate at 3.58x. Its allocation moved 0.0%, and so did almost everything else's: every one of the seventeen benchmarks came out slower, between 1.30x and 3.58x, because the baseline was taken on an 8-core runner and pull requests run on a 4-core one. The inflation tracks how small the benchmark is, which is what says it is overhead rather than code: EvalEasy 0.98 ns 3.58x ParseEasy 6,919 ns 1.59x RunEasy 23 ns 1.81x Derivate 17,215 ns 1.52x RunHard 346 ns 1.74x SimplifyHard 2.16e9 ns 1.30x RunMedium 203 ns 1.57x Allocation already has this guard -- AllocationFloorBytes exists so that a benchmark allocating almost nothing cannot fail on a percentage of almost nothing -- and the same argument applies to time with no floor at all. Below roughly a microsecond a ratio measures the runner: scheduling, cache state, one fewer core. So time under 1,000 ns/op is reported and never fails. The gate still catches what it was built for. SimplifyHard at 2.16 seconds would have to reach 6.5 seconds, and allocation is untouched -- it remains the metric that fails, at 3%, on every benchmark regardless of size. A gate that goes red without a defect is a gate people learn to ignore, and this one is a day old.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The performance gate added in #1014 failed on #1027 — a branch that changes exception message
strings and a documentation file:
It is not a regression. Allocation moved
0.0%there and on nearly every other row, and everyone of the seventeen benchmarks came out slower — between 1.30× and 3.58× — because the baseline
was captured on an 8-core runner and pull requests run on a 4-core one.
The inflation tracks how small the benchmark is
Which is what says it is overhead rather than code:
EvalEasyParseEasyRunEasyDerivateRunHardSimplifyHardRunMediumFixed per-call cost — scheduling, cache state, one fewer core — is a large fraction of a
nanosecond-scale measurement and a rounding error at second scale.
The fix is the guard allocation already has
AllocationFloorBytesexists so that "a benchmark allocating almost nothing does not fail on apercentage of almost nothing". The same argument applies to time, and time had no floor at all.
So: a baseline under 1,000 ns/op is reported and never fails on time. The constant carries the
measurement above as its justification, not an assertion.
What the gate still catches
metric that fails. That is the point of the gate and Goal: Math OS — a ten-year vision for AngouriMath as an open mathematical reasoning platform #746 item 80.
SimplifyHardat 2.16 s would have to reach6.5 s.
even though it does not fail the build.
Also worth recording
The baseline and the pull-request runners are different machine classes, which the gate prints
but does not otherwise account for. That is a separate question — re-baselining on the class actually
used would be the fuller answer — and it is not this change.
A gate that goes red without a defect is a gate people learn to ignore, and this one is a day old.