[Deploy] Comparison reports - #6012
Draft
mstange wants to merge 31 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.73% 81.44% -2.29%
==========================================
Files 350 359 +9
Lines 37523 39240 +1717
Branches 10543 10997 +454
==========================================
+ Hits 31420 31960 +540
- Misses 5676 6853 +1177
Partials 427 427 ☔ 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.
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.
mstange
force-pushed
the
comparison-reports
branch
from
August 13, 2026 19:14
bce3231 to
94a476f
Compare
`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.
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.
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