Bug 2065580 - Add claude skill for how to triage and resolve performance regressions r=kshampur,ai4dev-reviewers,suhaib

We previously added a "perftest" skill which helps you run a performance test.
A recent event where a person struggled to resolve a regression made it clear we need to do more, we need a performance regression triage skill.
This skill should help all engineers resolve regressions whenever they appear faster(which is useful now that our release schedule is doubled)
I included a test set to ensure that the "perftest" and "perf-regression-triage" skills do not get loaded too often and that if it's just a question on how to run a test only the perftest skill gets loaded.

Differential Revision: https://phabricator.services.mozilla.com/D320525
This commit is contained in:
Andrej Glavic
2026-09-02 16:06:34 +00:00
committed by aglavic@mozilla.com
parent ce0226a1ed
commit c2f1c1e75f
12 changed files with 962 additions and 14 deletions
@@ -0,0 +1,137 @@
---
name: perf-regression-triage
description: >
Handle a Perfherder performance regression bug end to end: read the alert bug, confirm
whether the regression is real, find the cause, and iterate to a fix. Use when the user
has a Perfherder/browsertime regression bug (the "N% <test> ... regression on <date>"
bugs filed in Testing::Performance), needs to establish whether an alert summary is a
real regression, is interpreting a PerfCompare baseRev/newRev comparison, or asks what
to do about a patch of theirs that regressed a benchmark.
Not for simply running perf tests or pushing them to try — including running an alert
summary's tests with `mach try perf --alert` (use perftest) — nor for choosing or
writing perf tests (use perftest), analyzing a profile you already have in hand
(use profiler-analysis), SpiderMonkey microbenchmarks (use js-perf-investigation), or
non-performance regressions such as failing tests, crashes, or build bustage.
allowed-tools:
- Bash(./mach try perf --help:*)
- Bash(./mach try perf --no-push:*)
- Bash(treeherder-cli:*)
- Bash(profiler-cli:*)
- Bash(git log:*)
- Bash(git show:*)
- Bash(git status:*)
- Bash(jj log:*)
- Bash(hg log:*)
- mcp__moz__get_bugzilla_bug
- Read
- Grep
- Glob
---
# Triaging a Perfherder regression bug
## Tooling
Three tools cover the three kinds of evidence in a regression bug. Use them rather than
fetching URLs by hand — `WebFetch` on a Treeherder or profiler URL returns a UI shell with
no data in it.
- **The regression bug** — `mcp__moz__get_bugzilla_bug`, or the `@moz:bugzilla://bug/{id}`
resource. There is no Bugzilla CLI; this is the supported path.
- **Push and job data** — `treeherder-cli <rev>`. Use `--perf` for performance and resource
data, `--repo autoland` to inspect the culprit push, and `--watch --notify` to wait on a
running try push instead of polling it.
- **Before/After profiles** — `profiler-cli`, driven through the **profiler-analysis**
skill. If it is not installed: `npm install -g @firefox-devtools/profiler-cli@latest`.
`profiler-analysis` owns the profiler-cli protocol (run `profiler-cli guide` first, stop
the daemon when done). Hand profiles to that skill instead of reimplementing it here.
The regression policy gives the patch author **3 business days** to acknowledge and start
investigating before the patch may be backed out. Establish early whether that clock is
running, and tell the user if it is close to expiring.
## Step 1: read the bug
Fetch it with `@moz:bugzilla://bug/{id}`. Every Perfherder alert bug carries the same
machine-readable payload. Pull out all of it before doing anything else:
- **Alert summary ID** — the `perfherder/alerts?id=NNNNN` link. Feeds `--alert` in step 2.
- **Culprit push revision** — the `pushloghtml?changeset=...` link in comment 0.
- **Base / new revisions** — `baseRev=` and `newRev=` in the PerfCompare link.
- **Regressed tests** — first column of the alert table.
- **Platform and options** — e.g. `linux2404-64-shippable`, `fission webrender`.
- **Before/After profiles** — the profiler.firefox.com links in the last column.
The culprit push usually contains several patches. Inspect it directly rather than reading
the pushlog HTML:
```
treeherder-cli <culprit-rev> --repo autoland --perf
```
Identify which patch is the user's and which files it touched (`git show <rev>` / the
linked Phabricator revision). If the push has multiple candidate patches and it is not
obvious which is responsible, say so — the confirmation push below tests the user's patch
specifically, which is what settles it.
Summarize for the user: how big the regression is, which tests, which platform, and what
the patch changed. Then move to confirmation.
## Step 2: confirm it
Do this before any investigation. A meaningful fraction of alerts do not reproduce.
The whole confirmation is **one command**, not two manual pushes. `--alert` runs exactly
the tests in the alert summary and compares your working revision against the base revision
your patch sits on, pushing both sides for you:
```
./mach try perf --alert <ALERT_ID> --rebuild 10
```
Read `references/confirm.md` before running it — it covers getting the local repo onto the
right base revision, how many retriggers are actually needed, the pgo/shippable trap, and
how to read the result.
**Never push to try without explicit approval from the user.** Show the exact command, say
roughly what it will cost in CI, and wait.
Then branch on the outcome:
- **Reproduces** — continue to step 3.
- **Does not reproduce** — do not start optimizing. Go to `references/close-out.md`; this
is likely noise or an unrelated patch in the same push.
- **Ambiguous / overlapping distributions** — more retriggers, or narrow to the single
most-regressed test. See `references/confirm.md`.
## Step 3: find the cause
Start with the Before/After profiles already linked in the bug — they are free and
specific to the regressed test. Hand them to the **profiler-analysis** skill rather than
fetching them yourself; `WebFetch` on a profiler URL only retrieves the UI shell.
Compare against what the patch actually changed. Most Perfherder regressions on
speedometer-class benchmarks come from work added to a hot path, a lost fast path, extra
allocation, or added main-thread sync work.
## Step 4: iterate to a fix
Read `references/iterate.md`. The loop is change → narrow try push → check, and the entire
point of that file is keeping each round cheap: narrow the test set, drop to one platform,
and only re-run the full alert set for the final confirmation.
## Step 5: close out
Read `references/close-out.md` for the possible resolutions, the bug fields to set, and
who to talk to when the right answer is "this regression is acceptable."
## Cost discipline
This skill exists partly to keep regression work cheap, in CI and in tokens.
- One push per question. `--alert` gives base and new together; do not push twice.
- Confirm broadly once, then iterate narrowly. Full alert-set reruns are for the final
check only.
- Do not read the reference files up front. Read the one for the step you are on.
- Read profiles through profiler-analysis, not by downloading them into context.
@@ -0,0 +1,70 @@
# Closing out a perf regression bug
Every path here ends with a comment in the bug. The perf sheriffs track these, and the
regression policy clock keeps running until the bug reflects reality.
## Acknowledge early, regardless of outcome
The policy gives **3 business days** from the bug being filed to acknowledge and begin
investigating, after which the patch may be backed out. If the user has not commented yet
and the bug is more than a day or two old, say so and draft the acknowledgement before
anything else. A one-line "looking at this, confirmation push running" resets the social
clock even when you have no answer yet.
## Outcomes
### Confirmed and fixed
- Post the final unnarrowed PerfCompare link from `iterate.md` showing the alert set back
at base.
- Reference the fix bug or Phabricator revision.
- Resolve **FIXED** once the fix lands. If the fix is a separate bug, leave this one open
and blocked on it rather than resolving early.
### Confirmed, not going to fix
This is a legitimate outcome — a correctness fix or a feature can be worth a few percent —
but it is **not the patch author's call alone**. Do not resolve WONTFIX unilaterally.
- Comment with: the confirmed magnitude, why the patch is worth it, and what was tried.
- Needinfo the perf sheriff named in comment 0 of the alert bug, and raise it in
[#perf-help](https://mozilla.enterprise.slack.com/archives/C03U19JCSFQ) on Slack or
[#perftest:mozilla.org](https://matrix.to/#/#perftest:mozilla.org) on Matrix.
- Let them set the resolution.
### Not reproduced
The confirmation push showed no regression, or the distributions overlapped completely.
- Post the PerfCompare link and state the measured delta versus the reported one.
- Say which of these it looks like:
- **Noise** — the alert fired on variance; the confirmation separates cleanly at base.
- **A different patch in the same push** — your patch isolated cleanly and showed
nothing. Name the other candidates from the pushlog so the sheriffs can redirect.
- **Infrastructure or environment** — a machine pool change, a test harness change, or a
dependency bump landing around the same time.
- Resolve **INVALID** for noise or an infra artifact. For a wrong-patch attribution, leave
it open and needinfo the sheriff rather than resolving — the regression is real, just not
yours.
### Still ambiguous after two confirmation rounds
Do not keep pushing. Comment with both PerfCompare links, state that the effect is inside
the noise band at the retrigger counts tried, and needinfo the perf sheriff. They have
history on which suites and platforms are chronically noisy and can often resolve it
without more CI.
## Bug fields
- **Severity / priority** — set if still `--`. Match the magnitude: a 10%+ regression on a
headline benchmark like speedometer3 is not S4.
- **`regressed_by`** — should already point at the culprit push. Fix it if the confirmation
push identified a different patch.
- **Assignee** — if the user is not the right owner (their patch isolated clean), unassign
rather than silently sitting on it.
- **Keywords** — `perf` and `regression` are usually set by the filer already.
## Draft, do not post
Write the comment and show it to the user. Never post to Bugzilla, change bug state, or
needinfo anyone without their explicit approval.
@@ -0,0 +1,104 @@
# Confirming the regression
Goal: one command that answers "does this patch actually cause the alert." Everything
here is about getting a trustworthy answer on the first attempt, because a second
confirmation round costs another few hours and a lot of CI.
## Get the local repo into the right shape
`--alert` compares your working revision against **the base revision your patches sit on**
in the local repository. The comparison is only meaningful if that base matches the alert.
1. Take `baseRev` from the PerfCompare link in the bug — that is the push immediately
before the culprit.
2. Update to it, then apply the user's patch on top so exactly one patch separates the two
sides. If the user still has the patch as a local commit, rebase it onto `baseRev`.
3. Confirm with `git log --oneline -3` (or `jj log`) that the patch is a single commit
directly on top of the base.
If the culprit push contained several patches and only one is the user's, this setup is
what isolates it. That is the point: the alert blames a push, this push blames a patch.
Verify the task selection without submitting anything:
```
./mach try perf --no-push --alert <ALERT_ID> --rebuild 10
```
`--no-push` prints the calculated task selection and changes nothing. Keep it immediately
after `./mach try perf` — the skill's `allowed-tools` entry is the prefix
`Bash(./mach try perf --no-push:*)`, so putting the flag anywhere later means it no longer
matches and the dry run prompts for permission like a real push. Show the user the task
list and the command before asking to push.
## The push
```
./mach try perf --alert <ALERT_ID> --rebuild 10 -m "Confirm bug <BUG> perf regression"
```
Do not submit base and new yourself. This one command pushes both sides for you.
**Never submit this without the user's explicit approval.**
### Retriggers
`--rebuild` accepts 1-20. Use **10**. That is the floor for separating a small real shift
from run-to-run variance on speedometer-class tests, and it covers the 3-10% range where
most alerts land.
Go higher — 12 to 15 — only for a sub-3% regression or a suite already known to be noisy.
Do not go lower to save CI: a thin run routinely produces an ambiguous result, and the
second round it forces costs far more than the retriggers you skipped.
### Do not use `--non-pgo` here
Alerts are almost always detected on shippable/pgo builds (`linux2404-64-shippable` in the
alert table). `--non-pgo` builds faster but is a different optimization configuration, and
a regression can appear or vanish across that boundary. Confirm on the same configuration
the alert fired on. `--non-pgo` belongs in the iteration loop, not here.
### Narrowing
If the alert lists 18 tests, `--alert` runs all of them. That is correct for confirmation —
you want to know the true blast radius, and a fix that helps one test can hurt another.
Narrow only if the user is explicitly trading coverage for turnaround:
```
./mach try perf --alert <ALERT_ID> --tests speedometer3 --platforms linux --rebuild 10
```
Say plainly which tests you dropped. Silent narrowing turns "confirmed" into a claim the
push does not support.
## Reading the result
`mach try perf` prints a PerfCompare link when it finishes. That is the primary artifact —
open it, or give it to the user. For the raw job state:
```
treeherder-cli <try-revision> --perf
treeherder-cli <try-revision> --watch --notify # if it is still running
```
Judge it on three things, in order:
1. **Direction and size.** Does the delta match the bug's reported magnitude? A bug
claiming 12% that reproduces at 1% is not confirmation.
2. **Overlap.** With 10 retriggers per side, do the two distributions actually separate? A
large mean delta with heavily overlapping runs is noise.
3. **Consistency across the alert set.** Real regressions from one patch usually move a
coherent group of subtests, not one outlier out of eighteen.
PerfCompare's own confidence indicator is a reasonable tiebreaker, but the overlap check
matters more than the label.
## Outcomes
- **Confirmed** — record the measured delta per test in the bug (it becomes the target the
fix has to clear). Continue to step 3 in SKILL.md.
- **Not reproduced** — do not start optimizing. Go to `close-out.md`.
- **Ambiguous** — one more round at higher `--rebuild`, narrowed to the two or three
most-regressed subtests. If it is still ambiguous, that is itself the finding; take it to
`close-out.md` and the perf sheriffs rather than burning more CI.
@@ -0,0 +1,81 @@
# Iterating to a fix
The loop is: hypothesis, minimal change, narrow try push, check. Each round takes hours of
wall clock and real CI capacity, so the value is almost entirely in making each round
answer one clean question.
## Before the first round
Have these written down:
- The confirmed delta per test, from `confirm.md`. This is the number the fix must clear.
- One or two **target tests** — the most-regressed subtests from the alert. These are the
signal you iterate against.
- A specific hypothesis about the mechanism, from the Before/After profiles (via the
**profiler-analysis** skill) and the patch diff.
Iterating without a mechanism hypothesis turns into guess-and-push, which is the single
most expensive failure mode here.
## The iteration push
Deliberately not the confirmation command. Narrow hard:
```
./mach try perf --alert <ALERT_ID> \
--tests <target-test> \
--platforms linux \
--non-pgo \
--rebuild 6 \
-m "bug <BUG> perf fix attempt N: <one-line hypothesis>"
```
What each narrowing buys, and what it costs:
- `--tests <target>` — cuts the task count sharply. Cost: you stop seeing collateral
movement in the other regressed subtests.
- `--platforms linux` — one platform instead of all. Cost: platform-specific behaviour is
invisible. Use the platform the alert fired on.
- `--non-pgo` — noticeably faster builds. Cost: different optimization configuration, so
treat the result as **directional only**. A fix that works here still has to be proven on
shippable.
- `--rebuild 5` or `6` — deliberately thinner than the confirmation run's 10. Enough to see
whether a change moved the number at all, which is the only question a round needs to
answer. Cost: a marginal result here is not trustworthy on its own, so do not treat a
small improvement as a fix until the unnarrowed run below confirms it at 10.
Put the hypothesis in `-m`. Three attempts later it is the only thing that tells you what
each push was testing.
Every push still needs the user's explicit approval.
## Between rounds
- Change **one thing** per round. Two changes in one push means an ambiguous result and a
wasted round.
- If a round shows no movement, suspect the hypothesis before suspecting the measurement —
especially if the change was small and the noise band is wide.
- Prefer changes that restore the old behaviour on the hot path over changes that add new
optimization. Reverting a fast path you removed is easy to reason about; a new
optimization needs its own justification and its own review.
- If three rounds produce nothing, stop pushing and go back to the profiles. Consider
whether the honest answer is a partial fix or an accepted tradeoff — see
`close-out.md`.
- Want a profile of your own attempt rather than the ones in the bug: add `--profile` to
the push, then analyze it through **profiler-analysis**.
## Final verification
A narrowed `--non-pgo` result is not sufficient to close the bug. Once a round looks good,
run the confirmation command again, unnarrowed, with the fix applied:
```
./mach try perf --alert <ALERT_ID> --rebuild 10 -m "bug <BUG> perf fix verification"
```
This must show the full alert set back at or near the base numbers. Check specifically that
no other subtest in the set got worse — a fix that trades one speedometer subtest for
another is not a fix, and the perf sheriffs will catch it.
Link that PerfCompare result in the bug and on the review. It is the evidence the fix
works, and it is what lets a reviewer approve a perf-motivated change quickly.
@@ -0,0 +1,79 @@
#!/usr/bin/env bash
# Cold-tests which skill fires for a set of prompts, to catch trigger regressions on the
# perftest / perf-regression-triage / profiler-analysis boundary.
#
# ./trigger-tests.sh [results-dir]
#
# Exits 0 if every case routes as expected, 1 otherwise. Each run spawns a fresh headless
# session granted only the Skill tool: granting Read as well lets the model reach skill
# content without invoking Skill, which silently defeats the measurement.
#
# Advisory, not a gate. This measures model behaviour, so a case can fail for reasons
# unrelated to the skill descriptions (model update, nondeterminism). Re-run before
# concluding an edit broke routing, and do not wire it into CI.
#
# Add a case as "name|expected-skill|prompt", using <none> when nothing should fire.
set -uo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../.." && pwd)"
RESULTS_DIR="${1:-$REPO_ROOT/artifacts/skill-trigger-tests/$(date +%Y%m%dT%H%M%S)}"
# Project skills only load for sessions rooted in the repo; without this every case
# silently reports <none> when the script is invoked from elsewhere.
cd "$REPO_ROOT" || { echo "cannot cd to $REPO_ROOT" >&2; exit 2; }
CASES=(
"pos-bug-live-framing|perf-regression-triage|https://bugzilla.mozilla.org/show_bug.cgi?id=2034476 my patch caused this regression, what do I do now"
"pos-alert-no-url|perf-regression-triage|Perfherder filed an alert saying my patch regressed speedometer3 by 12% on linux2404-64-shippable. Alert summary 49649. What now?"
"pos-perfcompare|perf-regression-triage|what does this PerfCompare result mean, my patch looks slower"
"neg-raptor-local|perftest|how do I run raptor speedometer3 locally"
"neg-alert-tests-on-try|perftest|how do I run the tests from alert summary 49649 on try"
"neg-profiler-link|profiler-analysis|can you analyze https://share.firefox.dev/4xbihh8"
"neg-mochitest-failure|<none>|this mochitest started failing on autoland"
)
command -v claude >/dev/null || { echo "claude not found in PATH" >&2; exit 2; }
command -v rg >/dev/null || { echo "rg not found in PATH; run ./mach bootstrap" >&2; exit 2; }
mkdir -p "$RESULTS_DIR"
echo "Running ${#CASES[@]} cases -> $RESULTS_DIR"
echo
for entry in "${CASES[@]}"; do
name="${entry%%|*}"
rest="${entry#*|}"
prompt="${rest#*|}"
claude -p "$prompt" \
--allowedTools Skill \
--output-format stream-json \
--verbose \
> "$RESULTS_DIR/$name.jsonl" 2>&1 &
done
wait
failures=0
for entry in "${CASES[@]}"; do
name="${entry%%|*}"
rest="${entry#*|}"
expected="${rest%%|*}"
fired=$(rg -o '"name":"Skill","input":\{"skill":"[^"]*"' "$RESULTS_DIR/$name.jsonl" 2>/dev/null \
| sed 's/.*"skill":"//; s/"$//' | sort -u | paste -sd, -)
[ -z "$fired" ] && fired="<none>"
if [ "$fired" = "$expected" ]; then
printf 'PASS %-24s %s\n' "$name" "$fired"
else
printf 'FAIL %-24s expected %s, got %s\n' "$name" "$expected" "$fired"
failures=$((failures + 1))
fi
done
echo
if [ "$failures" -eq 0 ]; then
echo "All ${#CASES[@]} cases routed as expected."
exit 0
fi
echo "$failures of ${#CASES[@]} cases mis-routed. Transcripts in $RESULTS_DIR"
exit 1
+10 -7
View File
@@ -2,13 +2,16 @@
name: perftest
description: >
Run Firefox performance tests locally or in CI. Use when the user asks how to run a
perf test, wants to reproduce a performance alert or regression locally, needs the
mach invocation for Raptor, Talos, MozPerftest, AWSY, or browsertime, wants to push
perf tests to try, or mentions `mach try perf`, `mach perftest`, `mach raptor`,
`mach talos-test`, an alert summary ID, PerfCompare, or Compare View. Covers picking
the right harness, finding a test's name, and the local-vs-CI tradeoff.
Not for analyzing an existing Firefox profile (use profiler-analysis) or for
SpiderMonkey/JS engine benchmarking (use js-perf-investigation).
perf test, needs the mach invocation for Raptor, Talos, MozPerftest, AWSY, or
browsertime, wants to push perf tests to try, wants to run the tests belonging to an
alert summary (`mach try perf --alert <id>`), or mentions `mach try perf`,
`mach perftest`, `mach raptor`, or `mach talos-test`. Covers picking the right
harness, finding a test's name, and the local-vs-CI tradeoff.
Not for triaging or root-causing a Perfherder regression bug — confirming whether a
regression is real, identifying the culprit patch, interpreting a PerfCompare result,
or deciding backout vs fix-forward (use perf-regression-triage). Not for analyzing an
existing Firefox profile (use profiler-analysis) or SpiderMonkey/JS engine
benchmarking (use js-perf-investigation).
allowed-tools:
- Bash(./mach perftest --help:*)
- Bash(./mach raptor --help:*)
@@ -0,0 +1,137 @@
---
name: perf-regression-triage
description: >
Handle a Perfherder performance regression bug end to end: read the alert bug, confirm
whether the regression is real, find the cause, and iterate to a fix. Use when the user
has a Perfherder/browsertime regression bug (the "N% <test> ... regression on <date>"
bugs filed in Testing::Performance), needs to establish whether an alert summary is a
real regression, is interpreting a PerfCompare baseRev/newRev comparison, or asks what
to do about a patch of theirs that regressed a benchmark.
Not for simply running perf tests or pushing them to try — including running an alert
summary's tests with `mach try perf --alert` (use perftest) — nor for choosing or
writing perf tests (use perftest), analyzing a profile you already have in hand
(use profiler-analysis), SpiderMonkey microbenchmarks (use js-perf-investigation), or
non-performance regressions such as failing tests, crashes, or build bustage.
allowed-tools:
- Bash(./mach try perf --help:*)
- Bash(./mach try perf --no-push:*)
- Bash(treeherder-cli:*)
- Bash(profiler-cli:*)
- Bash(git log:*)
- Bash(git show:*)
- Bash(git status:*)
- Bash(jj log:*)
- Bash(hg log:*)
- mcp__moz__get_bugzilla_bug
- Read
- Grep
- Glob
---
# Triaging a Perfherder regression bug
## Tooling
Three tools cover the three kinds of evidence in a regression bug. Use them rather than
fetching URLs by hand — `WebFetch` on a Treeherder or profiler URL returns a UI shell with
no data in it.
- **The regression bug** — `mcp__moz__get_bugzilla_bug`, or the `@moz:bugzilla://bug/{id}`
resource. There is no Bugzilla CLI; this is the supported path.
- **Push and job data** — `treeherder-cli <rev>`. Use `--perf` for performance and resource
data, `--repo autoland` to inspect the culprit push, and `--watch --notify` to wait on a
running try push instead of polling it.
- **Before/After profiles** — `profiler-cli`, driven through the **profiler-analysis**
skill. If it is not installed: `npm install -g @firefox-devtools/profiler-cli@latest`.
`profiler-analysis` owns the profiler-cli protocol (run `profiler-cli guide` first, stop
the daemon when done). Hand profiles to that skill instead of reimplementing it here.
The regression policy gives the patch author **3 business days** to acknowledge and start
investigating before the patch may be backed out. Establish early whether that clock is
running, and tell the user if it is close to expiring.
## Step 1: read the bug
Fetch it with `@moz:bugzilla://bug/{id}`. Every Perfherder alert bug carries the same
machine-readable payload. Pull out all of it before doing anything else:
- **Alert summary ID** — the `perfherder/alerts?id=NNNNN` link. Feeds `--alert` in step 2.
- **Culprit push revision** — the `pushloghtml?changeset=...` link in comment 0.
- **Base / new revisions** — `baseRev=` and `newRev=` in the PerfCompare link.
- **Regressed tests** — first column of the alert table.
- **Platform and options** — e.g. `linux2404-64-shippable`, `fission webrender`.
- **Before/After profiles** — the profiler.firefox.com links in the last column.
The culprit push usually contains several patches. Inspect it directly rather than reading
the pushlog HTML:
```
treeherder-cli <culprit-rev> --repo autoland --perf
```
Identify which patch is the user's and which files it touched (`git show <rev>` / the
linked Phabricator revision). If the push has multiple candidate patches and it is not
obvious which is responsible, say so — the confirmation push below tests the user's patch
specifically, which is what settles it.
Summarize for the user: how big the regression is, which tests, which platform, and what
the patch changed. Then move to confirmation.
## Step 2: confirm it
Do this before any investigation. A meaningful fraction of alerts do not reproduce.
The whole confirmation is **one command**, not two manual pushes. `--alert` runs exactly
the tests in the alert summary and compares your working revision against the base revision
your patch sits on, pushing both sides for you:
```
./mach try perf --alert <ALERT_ID> --rebuild 10
```
Read `references/confirm.md` before running it — it covers getting the local repo onto the
right base revision, how many retriggers are actually needed, the pgo/shippable trap, and
how to read the result.
**Never push to try without explicit approval from the user.** Show the exact command, say
roughly what it will cost in CI, and wait.
Then branch on the outcome:
- **Reproduces** — continue to step 3.
- **Does not reproduce** — do not start optimizing. Go to `references/close-out.md`; this
is likely noise or an unrelated patch in the same push.
- **Ambiguous / overlapping distributions** — more retriggers, or narrow to the single
most-regressed test. See `references/confirm.md`.
## Step 3: find the cause
Start with the Before/After profiles already linked in the bug — they are free and
specific to the regressed test. Hand them to the **profiler-analysis** skill rather than
fetching them yourself; `WebFetch` on a profiler URL only retrieves the UI shell.
Compare against what the patch actually changed. Most Perfherder regressions on
speedometer-class benchmarks come from work added to a hot path, a lost fast path, extra
allocation, or added main-thread sync work.
## Step 4: iterate to a fix
Read `references/iterate.md`. The loop is change → narrow try push → check, and the entire
point of that file is keeping each round cheap: narrow the test set, drop to one platform,
and only re-run the full alert set for the final confirmation.
## Step 5: close out
Read `references/close-out.md` for the possible resolutions, the bug fields to set, and
who to talk to when the right answer is "this regression is acceptable."
## Cost discipline
This skill exists partly to keep regression work cheap, in CI and in tokens.
- One push per question. `--alert` gives base and new together; do not push twice.
- Confirm broadly once, then iterate narrowly. Full alert-set reruns are for the final
check only.
- Do not read the reference files up front. Read the one for the step you are on.
- Read profiles through profiler-analysis, not by downloading them into context.
@@ -0,0 +1,70 @@
# Closing out a perf regression bug
Every path here ends with a comment in the bug. The perf sheriffs track these, and the
regression policy clock keeps running until the bug reflects reality.
## Acknowledge early, regardless of outcome
The policy gives **3 business days** from the bug being filed to acknowledge and begin
investigating, after which the patch may be backed out. If the user has not commented yet
and the bug is more than a day or two old, say so and draft the acknowledgement before
anything else. A one-line "looking at this, confirmation push running" resets the social
clock even when you have no answer yet.
## Outcomes
### Confirmed and fixed
- Post the final unnarrowed PerfCompare link from `iterate.md` showing the alert set back
at base.
- Reference the fix bug or Phabricator revision.
- Resolve **FIXED** once the fix lands. If the fix is a separate bug, leave this one open
and blocked on it rather than resolving early.
### Confirmed, not going to fix
This is a legitimate outcome — a correctness fix or a feature can be worth a few percent —
but it is **not the patch author's call alone**. Do not resolve WONTFIX unilaterally.
- Comment with: the confirmed magnitude, why the patch is worth it, and what was tried.
- Needinfo the perf sheriff named in comment 0 of the alert bug, and raise it in
[#perf-help](https://mozilla.enterprise.slack.com/archives/C03U19JCSFQ) on Slack or
[#perftest:mozilla.org](https://matrix.to/#/#perftest:mozilla.org) on Matrix.
- Let them set the resolution.
### Not reproduced
The confirmation push showed no regression, or the distributions overlapped completely.
- Post the PerfCompare link and state the measured delta versus the reported one.
- Say which of these it looks like:
- **Noise** — the alert fired on variance; the confirmation separates cleanly at base.
- **A different patch in the same push** — your patch isolated cleanly and showed
nothing. Name the other candidates from the pushlog so the sheriffs can redirect.
- **Infrastructure or environment** — a machine pool change, a test harness change, or a
dependency bump landing around the same time.
- Resolve **INVALID** for noise or an infra artifact. For a wrong-patch attribution, leave
it open and needinfo the sheriff rather than resolving — the regression is real, just not
yours.
### Still ambiguous after two confirmation rounds
Do not keep pushing. Comment with both PerfCompare links, state that the effect is inside
the noise band at the retrigger counts tried, and needinfo the perf sheriff. They have
history on which suites and platforms are chronically noisy and can often resolve it
without more CI.
## Bug fields
- **Severity / priority** — set if still `--`. Match the magnitude: a 10%+ regression on a
headline benchmark like speedometer3 is not S4.
- **`regressed_by`** — should already point at the culprit push. Fix it if the confirmation
push identified a different patch.
- **Assignee** — if the user is not the right owner (their patch isolated clean), unassign
rather than silently sitting on it.
- **Keywords** — `perf` and `regression` are usually set by the filer already.
## Draft, do not post
Write the comment and show it to the user. Never post to Bugzilla, change bug state, or
needinfo anyone without their explicit approval.
@@ -0,0 +1,104 @@
# Confirming the regression
Goal: one command that answers "does this patch actually cause the alert." Everything
here is about getting a trustworthy answer on the first attempt, because a second
confirmation round costs another few hours and a lot of CI.
## Get the local repo into the right shape
`--alert` compares your working revision against **the base revision your patches sit on**
in the local repository. The comparison is only meaningful if that base matches the alert.
1. Take `baseRev` from the PerfCompare link in the bug — that is the push immediately
before the culprit.
2. Update to it, then apply the user's patch on top so exactly one patch separates the two
sides. If the user still has the patch as a local commit, rebase it onto `baseRev`.
3. Confirm with `git log --oneline -3` (or `jj log`) that the patch is a single commit
directly on top of the base.
If the culprit push contained several patches and only one is the user's, this setup is
what isolates it. That is the point: the alert blames a push, this push blames a patch.
Verify the task selection without submitting anything:
```
./mach try perf --no-push --alert <ALERT_ID> --rebuild 10
```
`--no-push` prints the calculated task selection and changes nothing. Keep it immediately
after `./mach try perf` — the skill's `allowed-tools` entry is the prefix
`Bash(./mach try perf --no-push:*)`, so putting the flag anywhere later means it no longer
matches and the dry run prompts for permission like a real push. Show the user the task
list and the command before asking to push.
## The push
```
./mach try perf --alert <ALERT_ID> --rebuild 10 -m "Confirm bug <BUG> perf regression"
```
Do not submit base and new yourself. This one command pushes both sides for you.
**Never submit this without the user's explicit approval.**
### Retriggers
`--rebuild` accepts 1-20. Use **10**. That is the floor for separating a small real shift
from run-to-run variance on speedometer-class tests, and it covers the 3-10% range where
most alerts land.
Go higher — 12 to 15 — only for a sub-3% regression or a suite already known to be noisy.
Do not go lower to save CI: a thin run routinely produces an ambiguous result, and the
second round it forces costs far more than the retriggers you skipped.
### Do not use `--non-pgo` here
Alerts are almost always detected on shippable/pgo builds (`linux2404-64-shippable` in the
alert table). `--non-pgo` builds faster but is a different optimization configuration, and
a regression can appear or vanish across that boundary. Confirm on the same configuration
the alert fired on. `--non-pgo` belongs in the iteration loop, not here.
### Narrowing
If the alert lists 18 tests, `--alert` runs all of them. That is correct for confirmation —
you want to know the true blast radius, and a fix that helps one test can hurt another.
Narrow only if the user is explicitly trading coverage for turnaround:
```
./mach try perf --alert <ALERT_ID> --tests speedometer3 --platforms linux --rebuild 10
```
Say plainly which tests you dropped. Silent narrowing turns "confirmed" into a claim the
push does not support.
## Reading the result
`mach try perf` prints a PerfCompare link when it finishes. That is the primary artifact —
open it, or give it to the user. For the raw job state:
```
treeherder-cli <try-revision> --perf
treeherder-cli <try-revision> --watch --notify # if it is still running
```
Judge it on three things, in order:
1. **Direction and size.** Does the delta match the bug's reported magnitude? A bug
claiming 12% that reproduces at 1% is not confirmation.
2. **Overlap.** With 10 retriggers per side, do the two distributions actually separate? A
large mean delta with heavily overlapping runs is noise.
3. **Consistency across the alert set.** Real regressions from one patch usually move a
coherent group of subtests, not one outlier out of eighteen.
PerfCompare's own confidence indicator is a reasonable tiebreaker, but the overlap check
matters more than the label.
## Outcomes
- **Confirmed** — record the measured delta per test in the bug (it becomes the target the
fix has to clear). Continue to step 3 in SKILL.md.
- **Not reproduced** — do not start optimizing. Go to `close-out.md`.
- **Ambiguous** — one more round at higher `--rebuild`, narrowed to the two or three
most-regressed subtests. If it is still ambiguous, that is itself the finding; take it to
`close-out.md` and the perf sheriffs rather than burning more CI.
@@ -0,0 +1,81 @@
# Iterating to a fix
The loop is: hypothesis, minimal change, narrow try push, check. Each round takes hours of
wall clock and real CI capacity, so the value is almost entirely in making each round
answer one clean question.
## Before the first round
Have these written down:
- The confirmed delta per test, from `confirm.md`. This is the number the fix must clear.
- One or two **target tests** — the most-regressed subtests from the alert. These are the
signal you iterate against.
- A specific hypothesis about the mechanism, from the Before/After profiles (via the
**profiler-analysis** skill) and the patch diff.
Iterating without a mechanism hypothesis turns into guess-and-push, which is the single
most expensive failure mode here.
## The iteration push
Deliberately not the confirmation command. Narrow hard:
```
./mach try perf --alert <ALERT_ID> \
--tests <target-test> \
--platforms linux \
--non-pgo \
--rebuild 6 \
-m "bug <BUG> perf fix attempt N: <one-line hypothesis>"
```
What each narrowing buys, and what it costs:
- `--tests <target>` — cuts the task count sharply. Cost: you stop seeing collateral
movement in the other regressed subtests.
- `--platforms linux` — one platform instead of all. Cost: platform-specific behaviour is
invisible. Use the platform the alert fired on.
- `--non-pgo` — noticeably faster builds. Cost: different optimization configuration, so
treat the result as **directional only**. A fix that works here still has to be proven on
shippable.
- `--rebuild 5` or `6` — deliberately thinner than the confirmation run's 10. Enough to see
whether a change moved the number at all, which is the only question a round needs to
answer. Cost: a marginal result here is not trustworthy on its own, so do not treat a
small improvement as a fix until the unnarrowed run below confirms it at 10.
Put the hypothesis in `-m`. Three attempts later it is the only thing that tells you what
each push was testing.
Every push still needs the user's explicit approval.
## Between rounds
- Change **one thing** per round. Two changes in one push means an ambiguous result and a
wasted round.
- If a round shows no movement, suspect the hypothesis before suspecting the measurement —
especially if the change was small and the noise band is wide.
- Prefer changes that restore the old behaviour on the hot path over changes that add new
optimization. Reverting a fast path you removed is easy to reason about; a new
optimization needs its own justification and its own review.
- If three rounds produce nothing, stop pushing and go back to the profiles. Consider
whether the honest answer is a partial fix or an accepted tradeoff — see
`close-out.md`.
- Want a profile of your own attempt rather than the ones in the bug: add `--profile` to
the push, then analyze it through **profiler-analysis**.
## Final verification
A narrowed `--non-pgo` result is not sufficient to close the bug. Once a round looks good,
run the confirmation command again, unnarrowed, with the fix applied:
```
./mach try perf --alert <ALERT_ID> --rebuild 10 -m "bug <BUG> perf fix verification"
```
This must show the full alert set back at or near the base numbers. Check specifically that
no other subtest in the set got worse — a fix that trades one speedometer subtest for
another is not a fix, and the perf sheriffs will catch it.
Link that PerfCompare result in the bug and on the review. It is the evidence the fix
works, and it is what lets a reviewer approve a perf-motivated change quickly.
@@ -0,0 +1,79 @@
#!/usr/bin/env bash
# Cold-tests which skill fires for a set of prompts, to catch trigger regressions on the
# perftest / perf-regression-triage / profiler-analysis boundary.
#
# ./trigger-tests.sh [results-dir]
#
# Exits 0 if every case routes as expected, 1 otherwise. Each run spawns a fresh headless
# session granted only the Skill tool: granting Read as well lets the model reach skill
# content without invoking Skill, which silently defeats the measurement.
#
# Advisory, not a gate. This measures model behaviour, so a case can fail for reasons
# unrelated to the skill descriptions (model update, nondeterminism). Re-run before
# concluding an edit broke routing, and do not wire it into CI.
#
# Add a case as "name|expected-skill|prompt", using <none> when nothing should fire.
set -uo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../.." && pwd)"
RESULTS_DIR="${1:-$REPO_ROOT/artifacts/skill-trigger-tests/$(date +%Y%m%dT%H%M%S)}"
# Project skills only load for sessions rooted in the repo; without this every case
# silently reports <none> when the script is invoked from elsewhere.
cd "$REPO_ROOT" || { echo "cannot cd to $REPO_ROOT" >&2; exit 2; }
CASES=(
"pos-bug-live-framing|perf-regression-triage|https://bugzilla.mozilla.org/show_bug.cgi?id=2034476 my patch caused this regression, what do I do now"
"pos-alert-no-url|perf-regression-triage|Perfherder filed an alert saying my patch regressed speedometer3 by 12% on linux2404-64-shippable. Alert summary 49649. What now?"
"pos-perfcompare|perf-regression-triage|what does this PerfCompare result mean, my patch looks slower"
"neg-raptor-local|perftest|how do I run raptor speedometer3 locally"
"neg-alert-tests-on-try|perftest|how do I run the tests from alert summary 49649 on try"
"neg-profiler-link|profiler-analysis|can you analyze https://share.firefox.dev/4xbihh8"
"neg-mochitest-failure|<none>|this mochitest started failing on autoland"
)
command -v claude >/dev/null || { echo "claude not found in PATH" >&2; exit 2; }
command -v rg >/dev/null || { echo "rg not found in PATH; run ./mach bootstrap" >&2; exit 2; }
mkdir -p "$RESULTS_DIR"
echo "Running ${#CASES[@]} cases -> $RESULTS_DIR"
echo
for entry in "${CASES[@]}"; do
name="${entry%%|*}"
rest="${entry#*|}"
prompt="${rest#*|}"
claude -p "$prompt" \
--allowedTools Skill \
--output-format stream-json \
--verbose \
> "$RESULTS_DIR/$name.jsonl" 2>&1 &
done
wait
failures=0
for entry in "${CASES[@]}"; do
name="${entry%%|*}"
rest="${entry#*|}"
expected="${rest%%|*}"
fired=$(rg -o '"name":"Skill","input":\{"skill":"[^"]*"' "$RESULTS_DIR/$name.jsonl" 2>/dev/null \
| sed 's/.*"skill":"//; s/"$//' | sort -u | paste -sd, -)
[ -z "$fired" ] && fired="<none>"
if [ "$fired" = "$expected" ]; then
printf 'PASS %-24s %s\n' "$name" "$fired"
else
printf 'FAIL %-24s expected %s, got %s\n' "$name" "$expected" "$fired"
failures=$((failures + 1))
fi
done
echo
if [ "$failures" -eq 0 ]; then
echo "All ${#CASES[@]} cases routed as expected."
exit 0
fi
echo "$failures of ${#CASES[@]} cases mis-routed. Transcripts in $RESULTS_DIR"
exit 1
+10 -7
View File
@@ -2,13 +2,16 @@
name: perftest
description: >
Run Firefox performance tests locally or in CI. Use when the user asks how to run a
perf test, wants to reproduce a performance alert or regression locally, needs the
mach invocation for Raptor, Talos, MozPerftest, AWSY, or browsertime, wants to push
perf tests to try, or mentions `mach try perf`, `mach perftest`, `mach raptor`,
`mach talos-test`, an alert summary ID, PerfCompare, or Compare View. Covers picking
the right harness, finding a test's name, and the local-vs-CI tradeoff.
Not for analyzing an existing Firefox profile (use profiler-analysis) or for
SpiderMonkey/JS engine benchmarking (use js-perf-investigation).
perf test, needs the mach invocation for Raptor, Talos, MozPerftest, AWSY, or
browsertime, wants to push perf tests to try, wants to run the tests belonging to an
alert summary (`mach try perf --alert <id>`), or mentions `mach try perf`,
`mach perftest`, `mach raptor`, or `mach talos-test`. Covers picking the right
harness, finding a test's name, and the local-vs-CI tradeoff.
Not for triaging or root-causing a Perfherder regression bug — confirming whether a
regression is real, identifying the culprit patch, interpreting a PerfCompare result,
or deciding backout vs fix-forward (use perf-regression-triage). Not for analyzing an
existing Firefox profile (use profiler-analysis) or SpiderMonkey/JS engine
benchmarking (use js-perf-investigation).
allowed-tools:
- Bash(./mach perftest --help:*)
- Bash(./mach raptor --help:*)