Make benchmark history points comparable with each other - #5404
Draft
Timothee Guerin (timotheeguerin) wants to merge 10 commits into
Draft
Make benchmark history points comparable with each other#5404Timothee Guerin (timotheeguerin) wants to merge 10 commits into
Timothee Guerin (timotheeguerin) wants to merge 10 commits into
Conversation
A chart line implies every point was measured the same way, but this series spans single-iteration laptop runs, several Node versions and two runner images -- shifts that move the numbers further than most real regressions do. The result files knew all of this; the history step threw it away. history.json now carries a schema version, the runner behind each point, and a quality summary flagging the points that are not comparable with their neighbors: too few iterations, a foreign platform, or an isolated spike. On the current data that marks 37 laptop runs and the five ~9x spikes, and leaves genuine step changes alone. Rebuilding the history also stopped spawning a git process per result file, which was most of its cost at 453 files: 67s and 198MB, now 3s and 31MB.
Points measured with different iteration counts and noise gates cannot be
compared, so making the series uniform means measuring all of it again. That
was impossible: backfill skipped commits that already had results, wrote to a
hardcoded results/ directory regardless of --results-dir, and did not forward
the noise-gate flags, so a backfilled point was produced differently from a
live one.
gh workflow run benchmark.yml \
-f backfill_from=<oldest-sha> -f backfill_to=<sha> -f backfill_reset=true
--reset re-measures every commit in range and drops the stored results, and
backfill_to splits a long range into chunks that each fit in a job, so the
whole series can be rebuilt from Actions without a laptop in the loop.
Contributor
|
No changes needing a change description found. |
Contributor
📦 Package size report1 package changed size, +9.95 KB (+0.1%) packed overall.
12 package(s) with no notable change
Packed = gzipped |
Contributor
|
You can try these changes here
|
A CI checkout only creates the branch it was asked for, so `git log main` fails on every branch except main -- including whichever branch a change to the backfill is being tested on. Fall back to origin's copy, fetching it when the checkout left it out.
Backfill pointed at a log file it never wrote -- the benchmark's output went to /dev/null -- and a run where every commit failed still exited 0, so CI reported green having measured nothing. Keep the output, print the tail of it next to the failure, and fail the run when nothing was measured.
The specs emit with the Python, JS, TS and Java client emitters, but backfill only ever built the core and Azure libraries, so every compile failed with "Emitter not found" and no commit could be measured. Build those emitters and link them where the specs resolve from.
A CI checkout has no git identity, so backfill's commit failed, and because that commit was silent the branch was simply never created -- surfacing much later as "src refspec does not match any" from the push. Configure the same identity store-results uses, and let a failed commit say so.
Filtering the build to the packages the specs use left their own workspace dependencies unbuilt, so the compiler failed to resolve tmlanguage-generator and every commit in the range died before it could be measured. Install and build output was also discarded, which reported this as a bare "build failed" that could not be diagnosed after the runner was gone.
Every commit gets its own CI job, and CI hands out whichever runner is free. Across 100 commits of main the same work varied by 63% depending on the machine: spread between machines was 13.7% against 0.9% within one, so hardware outweighed code changes 16 to 1. That noise lands directly between neighboring points, which is why the chart jumps at commits that changed nothing relevant. Machine speed scales TypeSpec workloads more or less uniformly, so it can be divided out. Each run now also compiles a frozen reference workload on the same machine, in the same job, and records how long it took. Dividing by it drops between-machine spread below 1%, taking the smallest detectable regression from roughly 41% to 3.5%. The reference deliberately does not use the compiler being benchmarked. If it moved with the repo, a real compiler regression would slow the reference by the same amount and cancel itself out. It is a fixed spec built with a pinned release from npm, materialized from constants, so it is identical for every commit ever measured, including commits that predate it. Calibration costs ~15s and never fails a run: if the pinned compiler cannot be installed the run continues and the point is flagged uncalibrated. Raw numbers are never rewritten; history stores the calibration next to them and exposes a factor to multiply by, so the correction stays visible and reversible. Also records the CPU model, which is what actually differs between runners and which nothing captured before.
Points carried only the time they were measured, which is fine while one commit is measured per push but wrong the moment history is rebuilt: a backfill covers months of commits in an afternoon, so all 100 points collapsed onto the two days the backfill ran and the chart's x-axis lost any meaning. Worse, entries were also ordered by measurement time. Re-measuring a single old commit would have moved it to the end of the series, silently reordering history around it. Record the commit's committer date and use it for both placement and ordering, falling back to measurement time for points recorded before it was captured.
The first reference imported only @typespec/compiler and compiled synthetic models. Measured across two CI machines it slowed 16% while the real specs slowed 34%, so dividing by it removed only half the machine effect and left a 15% residual at the boundary -- still far more than the regressions this is meant to expose. Hardware sensitivity depends on the kind of work. loader is a third of the real measurement and turned out to be the most sensitive phase of all (+41% between those two machines), and a spec with no libraries to load barely exercises it. A reference is only useful if it does the same mix of work. Use a frozen copy of the azure-full spec against a pinned library stack and the same linter ruleset. Measured cold, the way both are actually measured, its phase mix now tracks the real one closely: loader 34.9% against 33.6%, where before there was essentially no library loading at all. The reference stays frozen and independent of the repo, so it still cannot cancel out a real regression. Backfill carries the calibration directory alongside dist and specs, so every commit is measured against the same yardstick.
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
1 participant
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 benchmark chart draws one line through every run, which says those numbers
can be compared. They often can't, and nothing in
history.jsonsaid so.Checking the current 453 entries:
darwin-25.4.0.totalof 412 / 814 / 920 / 920 / 666 ms across those groups — bigger than any regression the chart is meant to catch.The result files knew all of this —
runner,iterations,variability.total.cv— andflattenRuntime()discarded it.What this changes
history.jsongains aversion, and each entry gains the runner it was measured on plus a quality summary:{ "commit": "…", "metrics": { "total": 6595, … }, "runner": { "os": "linux-6.11.0", "nodeVersion": "v24.15.0", "arch": "x64" }, "quality": { "iterations": 25, "cv": 0.014, // null for runs that predate the field "flags": ["outlier"] // also: "low-iterations", "foreign-runner" } }On the live data that flags 37
low-iterations, 37foreign-runnerand 5outlier— exactly the anomalies above. The outlier test compares a point against the median of its 11 neighbors excluding itself, so a change that persists moves the window with it and is never flagged; only isolated spikes and dips are.foreign-runnercompares platform and arch only, not the kernel string, so routine image bumps don't light up the whole series.Re-measuring the series
Flagging bad points is a stopgap; the fix is to measure them all the same way. That wasn't possible — backfill skipped commits that already had results, wrote to a hardcoded
results/regardless of--results-dir, and never forwarded the noise-gate flags, so a backfilled point was produced differently from a live one.backfill_resetre-measures everything in range and drops the stored results;backfill_tosplits a long range into chunks that each fit inside a job, so the whole series can be rebuilt from Actions rather than someone's laptop. Subsequent chunks run withoutresetand append.Rebuild cost
Regenerating the history spawned one
git showper result file, which is now most of the work at 453 files. Batchedgit cat-file --batchreads plus streaming the files instead of holding them all:Byte-for-byte identical output, checked against
git showacross 120 real result files.Backfill was broken
Testing this on Actions found the backfill path had not worked in some time. Four separate faults, each of which stopped a run dead:
main. A CI checkout creates just the branch it was asked for, sogit log mainfailed on every other branch — including any branch testing a change to the backfill itself.emit:with the Python, JS, TS and Java client emitters; the build filter still listed only the core and Azure libraries, so every compile died withEmitter "@azure-tools/typespec-python" not found.src refspec ... does not match any./dev/null.Verified green on Actions against a throwaway
benchmark-data-testbranch (run), which produced:One caveat for planning the real re-measure: a commit costs ~30 min, since each one rebuilds the compiler and five emitters before measuring. That is roughly a dozen commits per 6-hour job, so re-measuring the full 453 means a good number of chunked
backfill_toruns rather than one sitting.Follow-ups
The dashboard doesn't read the new flags yet — that belongs on top of #5402. Also still open:
metricsaverages dissimilar specs,stats.complexitynever reaches the history, and the 11 MB payload is ~76% repeated metric names.