[Deploy] Comparison reports - #6012
Draft
mstange wants to merge 45 commits into
Draft
Conversation
mstange
force-pushed
the
comparison-reports
branch
2 times, most recently
from
May 12, 2026 18:02
a3cc414 to
4bd0212
Compare
mstange
force-pushed
the
comparison-reports
branch
from
June 4, 2026 21:22
b345b45 to
0c8885a
Compare
mstange
force-pushed
the
comparison-reports
branch
from
July 7, 2026 21:30
0c8885a to
8600e06
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6012 +/- ##
==========================================
- Coverage 83.79% 83.18% -0.62%
==========================================
Files 350 365 +15
Lines 37583 39646 +2063
Branches 10459 11121 +662
==========================================
+ Hits 31492 32978 +1486
- Misses 5664 6237 +573
- Partials 427 431 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mstange
force-pushed
the
comparison-reports
branch
from
July 23, 2026 16:12
4d7ce7b to
0d31b7f
Compare
mstange
added a commit
to mstange/perfherder2
that referenced
this pull request
Aug 8, 2026
…nchmark A pinned comparison whose two runs each uploaded profile_<benchmark>_compact.jslb.gz gets a fourth link, into the Firefox Profiler's benchmark-comparison view. That view does for the profiles what this pane does for the scores — lines the two runs up subtest by subtest — so "it got 1.4% slower" becomes "TodoMVC-Svelte-Complex-DOM did, and here is where the time went". By hand it was: open both jobs on treeherder, find the compact profile in each artifact list, open the profiler's compare form, paste two URLs, and get base and new the right way round yourself. Eligibility is one rule — both runs uploaded an artifact with the *same* name matching that pattern — and the rest follows from it. `_compact` and not its two whole-process siblings because that is the shape the view is built for (labels inserted, main threads merged). A suffix rule rather than a list of benchmarks because the name is composed from the raptor test's own name, so an allowlist would silently withhold the feature from the next benchmark to turn profiling on. And the same name on both sides because the name carries the benchmark: a speedometer3 profile against a jetstream3 one is two unrelated sample sets in a view whose whole output is the difference between them. Two counterparts of one test on different platforms match for free. The two runs are the two the user clicked. PerfCompare has to choose a run per side — its row knows a list of job ids and nothing about which the reader means — so it preselects each side's median and offers a dialog to override. Here the selection *is* a run, and the distribution above it already shows where that run sits among its push's retriggers, so picking a different pair is clicking a different dot rather than reaching into a second picker. Two fetches per side, and only when pinned: `profileComparison` reads `comparedSelection`, which is already gated on a pinned comparison, because following the hover preview would spend a job lookup and an artifact list on every dot the pointer crosses for a link that goes away before it can be clicked. The URL points at the deploy preview of firefox-devtools/profiler#6012 until the view ships — production rejects the compare-benchmark route, so a link built against profiler.firefox.com would land on the profiler's error page. PROFILER_BENCHMARK_ORIGIN is the one constant to change.
mstange
force-pushed
the
comparison-reports
branch
from
August 13, 2026 19:14
bce3231 to
94a476f
Compare
canova
reviewed
Aug 16, 2026
| sources.filename[sourceIndex] = filenameIndex; | ||
| sources.startLine[sourceIndex] = 1; | ||
| sources.startColumn[sourceIndex] = 1; | ||
| sources.sourceMapURL[sourceIndex] = null; |
Member
There was a problem hiding this comment.
Could we maybe do a silly thing and add a filename + .map as a sourceMapURL here (I don't think we have that data here in general, right?)? This would allow us to apply source maps more easily at a later time.
We can also apply source maps in this script, but this might increase the size of the profile. In sp-exp profiles, I added sourceMapURL manually so I can apply the source map for my workload.
The tooltip uses `position: fixed` which interprets `left/top` as viewport-relative, but `ChartCanvas` was storing `event.pageX/pageY` (document-relative, including scroll offsets). In a non-scrolling page these match, but inside a scrollable container like our benchmark page they diverge by `window.scrollY`, dragging the tooltip down. Switched the state field from `pageX/pageY` to `clientX/clientY`.
This is a workaround for Firefox and Chrome using different syntax to indicate the location of a JS function. On Windows, the difference is samply's fault: The JIT ETW events already allow specifying the URL and line/col separately from the function name, but samply puts it back into the function name. And Firefox doesn't make use of those ETW events yet.
The tie-correction counts can be built directly from the two input arrays, so mannWhitneyPValue no longer needs the pre-concatenated allValues array. Every caller was passing baseIter/newIter and separately concatenating them into allValues just for this function.
The extractor previously produced a per-bucket number[] copy of the dense
bucketIterationTotals buffer, at ~200k small allocations per profile. Widen
SparseBucketEntry.iterationTotals to Float64Array | number[] and hand out
subarray() views instead. The downstream consumers (mean, mannWhitneyU,
mannWhitneyPValue) are widened to ArrayLike<number> and use indexed loops,
and buildKeyMap now borrows entries on first insert, only materialising a
fresh Float64Array on the rare key-collision case.
The CLI serialiser converts Float64Array to plain arrays via a JSON.stringify
replacer, since typed arrays stringify as {"0": ...} otherwise.
Replace the naive O(n1 * n2) nested loop with an O((n1 + n2) log(n1 + n2)) algorithm: sort a and b once each, then walk them in a single pass with two monotonic pointers into the sorted b that count b[j] < ai and b[j] <= ai respectively. At n=200 (Speedometer iterations) and ~200k MWU calls per profile pair, this drops the compare-benchmark load-time contribution of mannWhitneyU from ~30% to ~3%.
Before: https://share.firefox.dev/4azGzbu (230ms concatenation) After: https://share.firefox.dev/4vNj739 (60ms concatenation, 3.8x faster)
… sliders. Adds an expandable overall row in the benchmark-compare score table that shows buckets whose change is significant across all suites (using the geomean-normalised globalBuckets). Buckets that don't cross the threshold in any single subtest but do when aggregated globally now surface here. Replaces the categorical "confidence !== LOW && effectSize !== Negligible" filter with two numeric sliders (max p-value, min |Cliff's δ|) above the table. BucketComparison now carries the raw Mann-Whitney pValue so the filter doesn't round-trip through the confidence enum.
The bucket flame-graph pair now shows an "Open ↗" link on each side that builds a fresh profiler URL: dataSource/hash preserved, tab set to flame-graph, with marker-search transforms restricting to measured samples and (for subtest expansions) the current suite, plus a focus-self on the bucket's representative func. The p-value slider is gone; with iterationCount ≈ 200, |Cliff's δ| and the Mann-Whitney p-value track each other so tightly that the second slider was redundant. Removes the pValue field on BucketComparison too.
Row index shifts when the effect-size slider changes the filtered/sorted list, so an expanded row would appear to migrate to whichever different bucket now sits at that index. Track expansion by BucketComparison.key (the cross-profile matching key — source location for JS funcs, name otherwise) which is stable across re-filterings. Also plumbs `key` onto BucketComparison so the UI has an explicit stable identifier per bucket.
Without an explicit v param, the URL parser assumes v=0 and runs every
upgrader. The v5->v6 upgrader re-encodes `thread` from its old decimal
form to uintarray encoding, but the deep link already writes it in the
new form -- so a threadIndex of 10 ("a") is parsed as NaN and encoded
back as "0", silently reselecting thread 0.
Explores replacing hand-annotated relevantForJS buckets with buckets chosen automatically from the call tree, driven by which grouping actually resolves a statistically significant difference. The framing that makes it tractable: "bucket = deepest relevantForJS frame" is already "bucket = nearest ancestor-or-self in a set S", so automatic bucketing means choosing S automatically rather than a new mechanism -- which keeps buckets a partition, and therefore keeps the mean differences additive so the bucket list stays a budget for the score change. One criterion covers both motivating cases: split B into C and R when max(|t_C|, |t_R|) > |t_B|. Means add and variances add, so this fires both when the change lives in one part and when a part contributes variance without signal (intermittent GC-like work). The search has to be paid for. With 12 children and no real difference anywhere, splitting on any positive gain and judging at |t| > 1.96 reports a "significant" bucket in 62% of trials; permutation-calibrating the whole procedure brings that to 6% against a 5% target. Includes a seeded, self-contained prototype with six worked examples, and dump-bucket-subtree for running it against a real profile pair. Findings recorded along the way: iterations show no run-level clustering (ICC -0.08..+0.14, so 200 iterations really are ~200 replicates); projecting out the leading principal component of the node x iteration matrix cannot work and inflates the null instead; and GC / CC conflates cycle collection with garbage collection today.
Per-iteration bucket weights are small integers -- a function usually accounts for 0, 1 or 2 samples in an iteration -- so base and new tie on 13-44% of all pairs in a real Speedometer profile. A rank statistic on such data is dominated by how ties are handled rather than by the difference being measured, which is how the geomean-normalised global view came to report 75 phantom effects. Mann-Whitney U and Cliff's delta are removed rather than left available, since their failure is silent. In their place, Welch's t on the per-iteration mean difference: the quantity that adds up across a bucket partition, so it is the same number the budget column shows. The p-value comes from permuting iteration labels wherever the verdict could turn on it -- Welch p below 0.25, plus sparse buckets (zero in over half their iterations, where the t-distribution is not trustworthy however large the sample looks) up to 0.5. One set of relabellings is shared by every bucket so their p-values are comparable; the CLI marks Welch-approximated rows with "~". Sequential stopping (Besag & Clifford 1991) is what makes this affordable: draw until 20 relabellings come out at least as extreme, then report hits/draws. Without it a comparison took 37.6s; with it, 0.3s. The effect-size filter switches from Cliff's delta to Cohen's d, whose 0.2/0.5/0.8 cut points sit close to the 0.15/0.33/0.47 they replace. Both tables gain an MDE column: the smallest change a row could have shown and still been called significant. "Did not move" and "could not tell" both show no significant change today, and only the MDE separates them -- the overall score here resolves nothing below ~1.4%. Also fixes a case the previous code got right by accident: a bucket with zero spread on both sides but different means, i.e. one that appeared or disappeared, has se == 0, and delta/se guarded as "se > 0 ? delta / se : 0" reports the most clear-cut change in a profile as no change at all.
Measured on two profile pairs, one with no difference detectable at subtest level and one with a large real change. The count of global buckets passing each |Cohen's d| cutoff: min|d| no-difference pair real-change pair 0.20 248 265 0.25 48 53 0.30 7 9 0.35 1 5 0.40 0 3 Below 0.3 the two are indistinguishable: the default was admitting ~250 rows regardless of whether anything had changed, which is what made the overall section unreadable. At 0.4 the no-difference pair is empty and the real-change pair shows exactly the three buckets that moved (CanvasRenderingContext2D stroke/fill/arc, work shifting out of fill and arc into stroke). Documented at the constant, because 0.4 is not free: d is standardised by the bucket's own spread, so it discriminates against large noisy buckets where absolute impact is what matters. HTMLElement.click dropped 1.16ms in that comparison -- the largest single contributor to the score change, p=1.6e-2 -- but that is only 0.25 sd of a 21ms bucket, so this cutoff hides it.
The bucket tables were filtered by a standardised effect size -- Cohen's
d, and Cliff's delta before that -- behind a slider. That is the wrong
instrument twice over.
It does not discriminate. Below d = 0.3 the filter admitted ~250 of the
6800 global buckets whether or not the two builds differed at all (248 on
a pair with nothing detectable at subtest level, 265 on a pair with a
large real change). A filter that passes the same rows either way is not
filtering, which is why the overall section was unreadable.
And where it does discriminate, it discriminates against the wrong rows.
d divides by the bucket's own spread, so a 1.16ms drop in a 21ms bucket
scores only 0.25 -- yet that bucket was the single largest contributor to
its comparison's score change. Raising the default to 0.4 to get a clean
list hid it.
Replaced by the two questions actually being asked: did this move, and
does it matter. Significance answers the first. Impact on the overall
geomean score answers the second, and it is the right denominator because
the "Δ% overall" column already means the same thing in the overall table
as in a subtest table, so one threshold serves both.
Global buckets clearing both p <= 0.05 and a minimum score impact:
min |Δ% overall| no-difference pair real-change pair
0.010% 29 30
0.020% 5 12
0.030% 5 5
0.040% 0 5
0.050% 0 3
0.04% is where the no-difference pair empties while the real one still
shows everything in it: the three canvas buckets whose work shifted among
them, the largest absolute mover, and one more at -16%.
The slider becomes three named options, since the useful settings are few
and a continuous scale invited hunting for one that produced a pleasing
number of rows. Each carries a title saying what it admits and what it
costs; "All significant" says outright that it is uncorrected and that
about 5% of buckets clear it by chance.
Handoff notes covering what would be needed to make per-bucket p-values mean something across ~6800 buckets, written for someone starting cold. The central finding is a negative one, so it is stated up front: BH is the obvious fix and it does not work yet. It needs p <= 0.05/6798 = 7.4e-6 to reject even the best bucket, and the permutation p-value is floored at 1/2000 = 5.0e-4, about 68x too coarse. Prototyped over both profile pairs: zero survivors, including a bucket that moved +73% with a Cohen's d of 1.00. Two ways out, with the cost model for each. Also records the parts that are judgement calls rather than implementation: whether BH is even the right family given the buckets partition the same samples and so are negatively correlated by construction, what the family should be across the global and per-subtest views, and why the impact floor should not simply be deleted once error control exists. Includes both test profile pairs with ready-to-paste URLs -- one as a negative control where any finding is by definition noise, one with a real canvas change and a table of what should come out of it -- plus the backstory on why the control pair is the one to distrust results from, and the se == 0 trap for appeared/disappeared buckets.
The compare view tests ~6800 buckets in the global view and up to ~800 in each subtest view, with no multiplicity correction, so "p <= 0.05" on a single row meant very little: on a profile pair with no difference detectable at subtest level, 133 global buckets cleared it. Benjamini-Hochberg cannot fix this directly. It needs the smallest p-value under 0.05/6798 = 7.4e-6, and a permutation p-value from 1999 relabellings is floored at 5.0e-4 -- so BH rejects nothing at all, not even a bucket that moved +73% with a Cohen's d of 1.0. The floor, not the evidence, is the binding constraint, and raising the relabelling count cannot fix the sparse buckets at any price. Stop going through per-bucket p-values instead. FDR control needs E[V(c)], the number of buckets expected to clear a threshold by chance -- a property of the family, not of any one bucket. Relabelling every bucket jointly and counting exceedances estimates it directly from 13.6 million pooled null statistics, so the resolution problem disappears; and because the relabelling is joint, the buckets' dependence is reproduced in every draw rather than having to be assumed away. That is SAM (Tusher, Tibshirani & Chu 2001) with Storey & Tibshirani's q-value. The same pass yields a Westfall-Young FWER-adjusted p-value for free. Affordable because the pooled weights are stored sparsely: most buckets are zero in most iterations, so a null evaluation walks a handful of nonzeros rather than 400 entries. Measured on two Speedometer 3 profile pairs from CI. On the pair with no real difference, 133 buckets at p <= 0.05 become 0 at q <= 0.05. On the pair whose only code delta was in canvas, the three canvas buckets whose work shifted are exactly what survives, at q = 2.5e-4 to 2.0e-3. The MDE had to move with it. It is defined as the smallest change that would have been reported, so changing what "reported" means changes it; left alone it would promise a sensitivity the table no longer has. A bucket's MDE is now built on the family's critical |t|, about 2.1x larger. MIN_SCORE_IMPACT drops from 0.04% to 0.01%. It was doubling as the only defence against ~340 expected false positives, which took a threshold high enough to also hide real small changes; now it only has to answer whether a row is worth acting on.
Three loose ends from the multiple-comparisons work, all of them the same mistake in different places: expressing a result in terms that are wrong for the person reading it. The reader is a developer with a try push with and without their patch, asking whether they changed anything and whether they made anything worse. They may have no statistics background. 1. Every row now carries a verdict, and there are four rather than three. "Nothing changed" and "we could not tell" are different answers, and conflating them is how a perf tool tells someone their patch is fine when it has no idea. The separator is the MDE against a 2% tolerance. On the reference pairs this is bracing: at 20 runs x 10 iterations most subtests read "can't tell" and only the tightest can say "no change". That is the honest reading, and more useful than the old column, which labelled every subtest "Negligible" from a Cohen's d -- including Perf-Dashboard at a real -2.27%. 2. Effect size is gone from the styling for the same reason. Cohen's d divides by the row's own spread, so emphasis-by-d picked out whichever rows happened to be quiet rather than whichever moved the benchmark, and nothing tied it to significance: a bucket could render bold on a large d with its q-value at 1.0. Weight now tracks impact on the overall score and colour tracks confidence, which are the two questions being asked and are independent of each other. 3. The 20 subtest scores get plain Benjamini-Hochberg. A family of 20 is small enough for it -- rank 1 needs 0.05/20 = 2.5e-3, well above the permutation floor that made BH useless for 6798 buckets. The overall score stays uncorrected: it is the one hypothesis the developer came to ask about, and charging it for the company of 20 subtests it was not compared against would answer a question nobody asked. Their MDEs had to move with them, which is the same trap as before and it caught me twice: with the bucket MDEs corrected but the subtest ones not, Perf-Dashboard reported "no change" while showing a delta larger than the smallest change it claimed it could see. Also drops the per-bucket permutation prefilter. The family pass already relabels every bucket, so each bucket's own p-value falls out of it for one extra comparison per null value. The separate pass could not afford to relabel everything, so it spent permutations only where a verdict might change and left the rest on a Welch approximation that is untrustworthy on sparse counts; three tuning constants existed to make that choice. Every bucket now has an exact p-value and the constants are gone.
The common case is two profiles -- a try push with and without a patch -- and requiring two extract-benchmark-stats runs first, remembering where the intermediate files went, and passing the right pair of them was friction for no benefit. --base and --new now take either form, detected from the bytes rather than declared: a profile is either gzipped or JSON without a bucketNames array in it. extract-benchmark-stats stays, and its output is still accepted, because it is the expensive half: pre-extracting brings a re-run down from 3.7s to 2.3s, which is worth it when iterating on the comparison rather than on the profiles. The --max-old-space-size=8192 the docs called for turns out never to have been needed. Extracting one profile peaks at 1.3 GB and the two-profile comparison at 1.9 GB, both inside Node's ~4.3 GB default. Extraction is sequential rather than concurrent so the second profile does not have to share the heap with the first. Errors reachable from here are all about which files were passed, so the top-level handler prints the message rather than a stack through minified bundler output. --stack restores the old behaviour.
Most subtests have nothing to show under the default filter, and finding that out cost a click each. The badge is the length of exactly the list the expansion renders, so the filtering moved out of BucketTable into a shared bucketRowsForFilter(), and the two cannot disagree. A zero is a result, so it stays visible -- just unshaded, so a screen of quiet subtests reads as quiet at a glance.
computeStackTableFromRawStackTable and createThreadFromDerivedTables both want a derived FrameTable, not the raw one, and the latter grew two allocation-table parameters. Neither matters at runtime for a benchmark profile, but the file did not typecheck.
The view was built for before-patch vs after-patch, but the same comparison answers Chrome vs Firefox or Release vs Nightly just as well, and "Base"/"New" is the wrong label for those. Names now live in the URL as names[], defaulting to Baseline/New. /compare-benchmark with no profiles shows its own form rather than the generic compare form, and the same form stays available above a loaded report. The follow-up question after reading one comparison is almost always a neighbouring one -- the same pair the other way round, or one side swapped out -- and going back to a separate form page to ask it means retyping URLs you already had. A swap button does the common case in one click, and loaded profiles are cached by URL so it doesn't re-download either side.
Every string that said "Base", "New", "slower" or "faster" without saying slower than what was readable only because the two sides used to be the same build before and after a patch. For Chrome vs Firefox they are not: a column of bare "slower"s leaves the reader guessing which side moved. The names travel by context rather than by prop -- the column headers, the verdict cells, the MDE tooltips and the flame-graph panels all need them, and they sit four component layers apart.
Mixing the wins and the losses in one list ranked by size is right for "what
did my patch do", where both are the answer. It is the wrong shape for "where
is Firefox losing to Chrome": those are separate pieces of work, and reading
either one means scrolling past the other. The old comparison report kept a
separate list per direction for this reason.
The options name the slower side ("Chrome is slower" / "Firefox is slower")
rather than reading as slower/faster relative to the second profile. For two
profiles the two phrasings pick out the same sets, but only the first still
picks out anything for three: "Firefox is faster" stops identifying a subset
as soon as there is a Safari column.
The three signed numbers on the row already carry it, but their reference point
-- percent of what, and for which side -- lives in a header tooltip. The old
comparison report led with the sentence instead ("Making Firefox as fast as
Chrome on this function would reduce its time on TodoMVC by 4%"), which is the
form the resulting bug report gets written in.
Always phrased as the slower side catching up, since that is the only direction
anyone can act on, and the saving is a fraction of that side's own total rather
than of the base side's.
`buildDeepLinkUrl` rewrites the profile viewer URL's tab segment, so it needs a URL of the form `/public/<hash>/<tab>/` or `/from-url/<enc>/<tab>/` and bails on anything shorter than two path parts. A share.firefox.dev (or perfht.ml / bit.ly) shortlink has just one -- `/45r9nQE` -- so the bail fired and every Open link on the page silently disappeared. `loadOneProfile` already resolved the shortlink to fetch the profile, but that expanded URL was thrown away. Return it alongside the profile, carry it through as `baseViewerUrl` / `newViewerUrl`, and hand *that* to the deep-link builder. The `baseUrl` / `newUrl` fields keep the as-typed URL so the report header still echoes what the user pasted in.
Chrome spells the IndexedDB bindings IdbDatabase.transaction where Firefox spells them IDBDatabase.transaction. These are DOM-binding funcs with no source location, so the bucket name is the only matching key there is, and the capitalisation difference was splitting all ~28 of them into a disappeared row and an appeared row. Fold case for keys that are still the name, and leave location keys alone: filenames and URL paths are case-sensitive, so folding those could merge two genuinely different sources. Testing key-equals-name also covers stats files that predate bucketKeys, whose keys are the names, which is why this lives at comparison time rather than in the extractor -- the two sides can be a mix of old and new stats files. The reported key keeps the profile's own spelling, since it is UI-facing identity.
Opening one of the compared profiles is a routine step in reading a row: the report says where the time went, the profile says what the code was doing there. It used to take a click into the collapsed editing section first, because the only links to the two profiles lived inside it. So the header is two lines now, and only the second one collapses: the names of the two profiles, as links to them, and below that the disclosure for the form that changes them. The URL list inside the section is gone rather than duplicated -- its links are the new top line, and the full URL text is still in the form's inputs when it is opened. The links sit outside the <summary> rather than linkifying the names in it, since an anchor there both navigates and toggles the disclosure.
Comparing two Speedometer profiles is ~3s of tight numeric loops — measured on a 13-subtest pair, 1.1s for the global bucket table and 1.6s across the 13 subtest tables — and it all ran in one uninterrupted go. Nothing else could happen on the page while it did, including painting results that were already computed and following a link. So the two expensive functions become synchronous generators that yield wherever their state is consistent: after each permutation draw in computeFamilyCorrection, which is where all of the time actually goes, and every 256 rows of compareBuckets's set-up. chunked-work.ts then offers two ways to drive one — runToCompletion for the CLI and the tests, runInSlices for the UI, which hands the main thread back every 12ms and stops early if its AbortSignal fires. compareBuckets and computeFamilyCorrection keep their names, signatures and results exactly; they are now one-line wrappers that drain the generator, so there is a single copy of each computation and the existing callers and tests are untouched. Measured no throughput cost from the conversion: the per-draw arithmetic stays in an ordinary function called from the generator body rather than being inlined into it. What to yield *with* turned out to matter more than any of the above, and the answer is a MessageChannel ping. Not setTimeout(0): browsers clamp nested timeouts to 4ms after the first few, which against a 12ms slice throws away a quarter of the throughput. Not scheduler.yield(), even though it is the primitive built for this. In Gecko a yield continuation is a WebTaskMainThreadRunnable at MediumHigh priority, which outranks the ordinary task React's scheduler renders in, so a chain of them starves React for as long as the chain lasts. Profiled with that version in place: the score rows were ready at 2.0s and the first layout after them ran at 7.5s, when the last table finished and the chain stopped. The slices were all there, 12ms each, with the main thread handed back between them — and not one Styles, Reflow or SetNeedStyleFlush marker in those 5.5 seconds, because React never got a turn to touch the DOM. Vsync outranks both, so the spinner's CSS animation kept turning and the page looked alive while it was frozen. MessageChannel is what React's own scheduler posts with, so our slices and its renders land in one queue at one priority and take turns in order. It is the fairness and not the mechanism that matters: what a long computation yields with has to be no more urgent than what the page needs in order to draw.
Reading a comparison starts with the score rows and the two profile links, and none of that needed the ~3s of per-function statistics that were blocking it. The page showed a spinner for the whole run, then everything at once. Now the comparison is a sequence of snapshots rather than one answer. run-benchmark-comparison.ts takes over the loading and the staging from the component: it downloads the pair, extracts both profiles' stats, yields the score rows, and then yields again after each bucket table, one per score row, in the order the rows are listed. Each stage hands the main thread back before it starts and — via runInSlices — every 12ms while it runs, so the rows paint as they fill in and a click on a profile link is dispatched instead of queued. A row whose table has not arrived shows a spinner where its count of moved functions will go, and cannot be expanded yet. That is a third state, not a missing count: a subtest only one profile ran has no table at all, and has to say so immediately rather than spin forever, so the list of pending rows is settled up front rather than discovered as the tables come in. Two smaller things fell out of putting the table on the critical path. The flame-graph bundles are no longer built in a useMemo when results first render — deriving every table of two multi-hundred-megabyte profiles, for graphs the reader may never open — but on the first expansion that needs one. And the effect drives the run with an AbortController, so replacing either profile stops the old comparison between slices instead of letting it finish tables nobody will read. Snapshots are immutable: each one gets a fresh map and pending list, since the consumer renders from whichever it last received.
Slicing the comparison keeps the page alive but does not make it finish sooner: ~2.6s of the ~3s is arithmetic in two places, and both are embarrassingly parallel. This is the brief for doing something about it, written while the measurements and the reasons were still to hand. It covers the pipeline and its measured costs, the seam to cut (an injected table runner in compareStatsProgressively, so tests keep running in-process), the wire format and its one real trap around structured-cloning subarray views, the two phases and what each is worth, and the invariant that matters most: the q-values are load-bearing, so the result must be exactly identical however many cores ran it — which the draw-range split does permit, since the accumulators are integer counts. The estimate is a day for a worker per table and half a day again for splitting the largest one by draw range, taking ~2.6s to ~0.3s.
Slicing kept the page painting but the ~2.6s of arithmetic still all happened on the main thread. The tables are independent of each other, and within one table the permutation draws are independent too, so both levels are now used: a subtest table gets a thread to itself, and the global table -- which is the whole critical path once the subtests are spread out, and also the row a reader expands first -- is split across every thread by draw range. The seam is an injected TableRunner. compareStatsProgressively builds the job list and takes whichever table comes back first; the compare page passes the worker pool, and the default in-process runner is what the CLI and the tests still get, so no test has to spawn a worker. A factory rather than a plain function because a pool has a lifetime: spawn, hand each worker the bucket metadata once, terminate on abort or when the comparison ends. computeFamilyCorrectionInSlices splits into accumulateFamilyPartialInSlices (prepare, then a range of draws) and combineFamilyPartials, and compareBuckets is now a single shard covering every draw -- one implementation rather than a fast path and a threaded path to keep agreeing. The combination is exact, which it has to be, since the q-values decide what the report shows: the accumulators are integer counts, so they add in any order, and every shard recomputes the same observed |t|, so the thresholds are bit-identical. combineFamilyPartials throws unless the ranges tile the draws exactly once and agree about every member's |t|, and the tests assert exactness with toEqual rather than toBeCloseTo, at the family level, the table level, and across the real postMessage protocol against a fake Worker. Two departures from the brief in docs-developer, which is rewritten to describe what exists. Jobs are packed into flat typed arrays rather than sent as the subarray views they arrive as: structured clone copies a view's whole underlying buffer, which covers every bucket at every iteration (17MB per side for a Speedometer pair) where a job needs a third of it, and a split table sends the same job to every thread. And the seam is a factory, since a per-job function has nowhere to get the metadata or the pool size from. Verified byte-identical to the single-threaded path: every field of 750 rows across four tables, at full precision, diffed between builds of the statistics code from before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
perf-compare-stats.ts had grown a quarter of its length in functions reachable only from their own unit tests: a Shapiro-Wilk normality test, a bootstrap CI for the median difference, Cohen's d and its cut points, and a bitmask-DP matcher for distribution modes with the helpers that fed it -- leftovers from a version of this view that reported them. The normal quantile and CDF and the median went with them, since nothing else called those either. What is left is the path from two sets of per-iteration weights to the numbers a comparison row shows, which makes the file about the FDR construction it is mostly there for rather than a mixed bag with that construction somewhere in it. welchTTest loses sdBase and sdNew, which existed only to be squared back up by Cohen's d, and confidenceLessThan goes as a duplicate of the one in src/utils/stats.ts that benchmark-stuff.ts actually imports. Reachability checked by hand rather than by a linter: outside its own test file, perf-compare-stats.ts is imported only by compare-benchmark-stats.ts and by BenchmarkCompareViewer, for pValueToConfidence and ConfidenceRating. No doc refers to any of the deleted names. Output verified unchanged, byte for byte, over the same 750 rows as the worker change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
compareBuckets took (baseBuckets, newBuckets, baseBucketNames, newBucketNames, baseBucketFuncs, newBucketFuncs, iterationCount, excludeAppearedDisappeared, baseBucketKeys, newBucketKeys), of which six were same-typed arrays -- names, keys and funcs for each side. Transposing any two of them type-checked, ran, and compared the wrong things. It also meant two calling conventions for one computation, since the sharded path already took a BucketTableInput; now there is one. The bucket metadata is a BucketTableSide per profile, and bucketTableSideOf builds one from a stats file with the fallbacks an older file needs: keys back to names, which is matching by name, and funcs to -1, which reads as "no func to reach into the profile with". Those two fallbacks had a copy each in the CLI, in run-benchmark-comparison and in the test helpers; the CLI's four patch blocks over the loaded stats go away with them. Output unchanged, byte for byte, over the same 750 rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three functions computed the same studentised |t| and none of them mentioned the others. welchTTest takes two passes over each group, which is the better way round and the right choice for the number a row reports. The permutation test and the family pass both use the computational formula on one pass over the base group, deriving the other by subtraction, which is what makes 13.6 million of them affordable -- but they had a copy each of the clamping, the standard error and the degenerate cases, and they had drifted: one guarded a group of size one and not NaN, the other guarded NaN and not the size. Both now accumulate their sums however suits their data layout -- a dense pooled array with an index list, or one member's nonzeros with a mask -- and call absTFromGroupSums, which has the formula, both guards, and the note on why it is not welchTTest. The tolerance had a similar problem: two spellings of 1e-9, applied in three places, in two directions -- shrink the threshold here, widen the value there -- each with its own partial explanation. There is now one constant, one convention (widen the candidate, never shrink the threshold), and atLeastAsExtreme / widened / lowestReaching for the three shapes a caller needs. Flipping permutationTwoSidedP and the family-wise count to the widening form is a change in the last ULPs, where either answer was defensible; it moved nothing on the 750-row check, though it is not guaranteed to move nothing on data that lands a null value within 1e-9 of a threshold from the other side. Also written down, above the FDR combine, what an index means down there: index i is the threshold ascending[i], which rejects the memberCount - i members at or above it, while nullsClearing is a histogram over 0..memberCount of how many thresholds a null value cleared. The two ranges being one apart is the whole of the fiddliness, and re-deriving it is what splitting this across threads cost me most of. No measurable cost to the extracted kernel: 186.7ms against 186.1ms, and 184.3 against 184.6, for a 1000-bucket global table over 1999 draws. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Its only mention anywhere is a commented-out import in benchmark-stuff.ts, of a function it does not export. With it go a second ConfidenceRating, a second copy of pValueToConfidence, the confidenceLessThan that perf-compare-stats had a duplicate of, and computeComparedStats -- a Welch-ish t test whose p-value came from a closed-form approximation to the t CDF rather than from the incomplete beta that studentTTwoSidedP uses. Having a cruder second t test in the tree, one import away from being picked up by mistake, is worse than not having one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every benchmark compare worker was sent both profiles' bucketNames and bucketKeys at startup: four arrays of a few thousand strings, and bucketKeys holds source locations, so long ones. It sounded cheap -- single megabytes, against the several hundred a parsed profile weighs -- and it was not. Profiling the compare page on a Speedometer 3 pair over eight workers: reading the init clone, 96% of it readString 91ms per worker the shutdown GC, 80% of it string finalize 34ms per worker the shard itself 356ms ~14% of every worker, and the 91ms is the worst kind: it sits between the worker starting and its shard starting, so it is on the path to the first row the reader sees. Three quarters of both figures is not copying but _os_unfair_lock_lock_slow and __ulock_wait2 under moz_arena_malloc and free -- eight threads pushing tens of thousands of small char buffers through one process-wide jemalloc arena at the same instant. The shutdown GCs hold that same lock while the main thread is rendering the results. Matching is the only thing a table wants those strings for, it is the same answer for every table, and it is a pure function of metadata the main thread has anyway. So matchBucketKeys now does it once per comparison -- folding the case of name keys, as buildKeyMap used to per shard -- and hands out an integer key id per bucket, shared between the two sides. WorkerInit is three Int32Arrays. A table groups by indexing a dense array instead of rebuilding a string Map, which the eight shards were each doing. Rows come back as BucketTableRow, two bucket indices, and resolveBucketTableRows fills in key, bucketName, baseFunc and newFunc from metadata that never left. BucketTableMetadata is main-thread-only by construction. Which of a profile's own colliding buckets represents a row stays in the table, because it is chosen by weight and the weights are the table's: the same key can be represented by one bucket in the global view and another in a subtest. The worker reports that as an integer too. Tested both ways round. The numbers do not move. Dumped every field of every row of all 21 tables at full precision, before and after, on the same profile pair: 15053 rows, byte for byte identical, row order included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The arrow appeared only once a row's bucket table had arrived, which for the overall row is about a second in and for the last subtest several. An invitation that shows up that late is one the reader has already looked past, and while it is missing there is nothing to distinguish "not yet" from "never" -- a subtest the new profile did not run looks exactly the same as one still being computed, except for a spinner two columns over. So a pending row opens too, and says what it is waiting for, in the same place and the same voice as the "nothing passed the thresholds" message. `none` now means only "there is no table and there never will be", which is the one state that does not open. A table that came back empty is `ready` with a count of zero rather than `none`: it would otherwise have collapsed under a reader who opened it while pending, taking the message they were reading with it and leaving them no answer. BucketTable already has prose for a list with nothing in it. The test fixture can now stand still in the middle of the staged rendering instead of racing it: compareStatsProgressively already takes a TableRunner, so the mock supplies one that holds each table until the test releases it. That is what makes "open a row, then let its table land, and check it is still open" testable at all, and it lets the existing test assert that the rows fill in one at a time rather than just that they eventually do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The spinner was a standalone element sized by hand to resemble the badge the count arrives in, and the two boxes did not match. Two reasons, both invisible in the declarations: the spinner is an empty bordered box with no box-sizing, so its height was 0.9em *plus* 4px of border; and as an empty inline-block in a baseline-aligned flex line its whole box sits above the baseline, where a badge puts its descender space below it. The label line came out a couple of pixels taller while a row was pending and snapped back when its number landed. So the placeholder is now an actual .benchmarkBadge with the spinner inside it. The box cannot drift from the badge's box because it is the badge's box -- its min-width and line-height are what hold the row steady, and the spinner is drawn inside as an inline-block small enough not to grow the line. Measured in headless Chrome against the real compare page, before and after the tables land: the score label line is 16.109px in both states, for all 21 rows. The two distinct row heights that remain are the overall row's 2px bottom border against the subtests' 1px, which is deliberate and was always there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bucket filter offered "Baseline is slower" and "New is slower", which names the slower side and is complete for two profiles: every subset either list could describe, the other one describes too. It is not, however, readable. Watching someone read a Firefox-vs-Chrome report: they asked how to see where Firefox was faster, and did not recognise "Chrome is slower" as the answer. The words they were looking for were not on the screen, and the substitution is one they have to make themselves, on an unfamiliar report, with nothing prompting them to. So the choice is a sentence with slots now -- "or where [Firefox] is (faster) (slower)" -- rather than a list of ready-made phrasings. Picking the other profile flips the direction with it, so the listed rows do not move: naming a subset differently should not change which subset it is, and watching the selection travel from "Firefox is slower" to "Chrome is faster" over an unchanged table says they are one question better than any tooltip could. The slot is deliberately not called the reference. `base` is the reference, meaning the side every delta is measured against, and picking a subject here does not touch a number. The two would have collided as soon as a third profile made the reference a control of its own -- which is the rest of this change. Any number of profiles can now be loaded. One report is still of one pair, since every delta, MDE, q and verdict in it is two-sample, and with three or more loaded a "Comparing [X] vs [Y]" control says which pair. Switching pairs re-computes but re-downloads nothing, and now re-extracts nothing either. The profiles outside the pair are mean columns, and the sentence grows a third slot for them: "where Firefox is slower than [the best of the others]". That aggregate is the point of loading a third profile -- it is the version of the question whose answer comes with an existence proof, since somebody being faster here means it can be done faster -- and it is also why the control is slots rather than enumerated phrasings, which could not express it at all: it is not an ordered pair, so there is no sentence to write out. An extra column costs one extraction, not one comparison. Filtering on the best of the others compares means, and means are what extraction already produces, so profile-means.ts runs no test and submits no table job. Two things had to hold for that to be correct rather than merely cheap: the shared suite factors are now computed across every loaded profile, so a column that is not part of the comparison is still on the comparison's scale and the factors do not shift when the selected pair changes; and the bucket-matching rule is now one exported function used both by the matcher and by the means pass, so a mean lands on the row for the bucket it belongs to rather than on a plausible neighbour. What the extra columns do not carry is significance, and a column with no q-value beside columns that have one reads as an oversight rather than as a claim not being made. So the table says so above itself, once. Also here, because it is the same sentence one level up: expanding a subtest now leads with what that subtest is worth -- "If Firefox were as fast as Chrome on Beta, it would spend 20.63% less time overall." It needs only the score row, so it is on screen immediately, while the list of functions behind it is still seconds away. That sentence existed for expanded functions and not for the rows that contain them, which is the level at which the decision to spend an afternoon is actually made. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mstange
force-pushed
the
comparison-reports
branch
from
August 21, 2026 18:44
cb3d5e9 to
a54e0f7
Compare
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
2 participants
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.
Example comparison