-
Notifications
You must be signed in to change notification settings - Fork 5
test(v4): measure and guard mounting at page scale #829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
eee9d16
test(v4): measure and guard mounting at page scale
titouanmathis e144747
ci(v4): compare browser benchmarks against the base commit
titouanmathis 4d99be0
fix(v4): reduce the long-task repeats conservatively
titouanmathis 2ee80d6
fix(ci): fail a broken baseline instead of inventing an empty one
titouanmathis ebb3d48
fix(ci): fail on a report that cannot be read
titouanmathis 4f2857e
fix(v4): interleave the two sides of each ratio guard
titouanmathis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| # bench-diff | ||
|
|
||
| Run a [vitest](https://vitest.dev) benchmark suite for the base and the head commit **on one runner**, alternating between them, and upsert a sticky pull-request comment with the difference. | ||
|
|
||
| It is the timing counterpart of [`weareikko/export-size`](https://github.com/weareikko/export-size): same shape — measure head, check the base sha out into a subdirectory of the same runner, measure it with the same action-provided script, diff, comment — with the changes a stopwatch needs that a byte counter does not. | ||
|
|
||
| ## Why it works this way | ||
|
|
||
| **Both sides on one runner.** Cross-machine timing noise is the reason services like CodSpeed exist. Measuring the base on the runner that measures the head removes it, with no account, no token and nothing to store. A cached baseline from an earlier `main` run would put that noise straight back, and would be un-interleavable by construction. | ||
|
|
||
| **Alternating, not sequential.** Export-size measures head then base. For byte counts that is fine. For timings it puts every bit of thermal drift and every noisy neighbour on one side of the comparison. This action alternates the sides within a round and flips the order between rounds, so drift lands on both. | ||
|
|
||
| **Median, never mean.** A benchmark's value for a round is tinybench's median; its value for the report is the median across rounds. One GC pause should not become the headline. | ||
|
|
||
| **A measured threshold, and a resolution floor.** Run one commit against itself, read the spread, and set `threshold` from it. Benchmarks whose median falls under `floor` milliseconds are reported but never flagged: Chromium clamps `performance.now()` to 100 µs, so a 1 ms benchmark cannot resolve a percentage. | ||
|
|
||
| **It comments; it does not block — but it fails when it could not measure.** A timing _threshold_ that fails a build on a shared runner is a threshold that gets deleted, so regressions are reported rather than enforced. A broken _measurement_ is the opposite: the job fails, because a comparison that silently did not happen is worse than no comparison. The one state that degrades is a base with no benchmark suite yet, which is a real thing on the pull request that adds one. Absent is not broken: | ||
|
|
||
| | side | every round measured | none measured | some measured | | ||
| | ---- | -------------------- | ----------------------------------------- | ------------- | | ||
| | head | compare | fail | fail | | ||
| | base | compare | empty base, warn, and the comment says so | fail | | ||
|
|
||
| Base `install` and `prepare` are never suppressed either. That distinction is the whole reason counting output is worth doing. | ||
|
|
||
| **It knows nothing about environments.** It takes a directory, a command and an output path, and reads the JSON vitest emits — the same schema whether the benchmark bodies run in Node, in happy-dom or in a real browser over CDP. A second suite is another step with another `id`, not a branch inside the action. Keep it that way. | ||
|
|
||
| ## Usage | ||
|
|
||
| A browser suite, which needs a browser downloaded first: | ||
|
|
||
| ```yaml | ||
| - uses: actions/checkout@v4 | ||
| - uses: ./.github/actions/bench-diff | ||
| with: | ||
| id: v4-mount | ||
| title: v4 mount benchmarks | ||
| unit: component | ||
| working-directory: packages/v4 | ||
| prepare: npx playwright install --with-deps chromium | ||
| bench: npm exec vitest bench -- --config vitest.bench.config.js --run --outputJson "$BENCH_JSON" | ||
| rounds: '3' | ||
| threshold: '25' | ||
| floor: '5' | ||
| ``` | ||
|
|
||
| A Node suite, which needs nothing extra — the only difference is the command: | ||
|
|
||
| ```yaml | ||
| - uses: ./.github/actions/bench-diff | ||
| with: | ||
| id: v3 | ||
| title: v3 benchmarks | ||
| working-directory: packages/js-toolkit | ||
| bench: npm exec vitest bench -- --config vitest.bench.config.ts --run --outputJson "$BENCH_JSON" | ||
| ``` | ||
|
|
||
| `bench` must write a `vitest bench --outputJson` file to `$BENCH_JSON`. Anything the suite needs from the environment can be set as `env:` on the step — composite `run` steps inherit it — or written as a prefix assignment on the command itself, which is unambiguous. | ||
|
|
||
| `id` namespaces the sticky comment and the temporary files, so several suites can each keep their own comment on one pull request. Two suites in the _same job_ would share the base checkout; prefer a job or a workflow each, which also lets each one carry its own `paths:` filter. | ||
|
|
||
| The job needs `pull-requests: write` to comment. | ||
|
|
||
| ### Inputs | ||
|
|
||
| | input | default | meaning | | ||
| | ------------------- | ----------------------------- | ---------------------------------------------------------- | | ||
| | `id` | `bench-diff` | Suite identifier; namespaces the comment and temp files. | | ||
| | `title` | `Benchmarks` | Heading of the sticky comment. | | ||
| | `unit` | `unit` | What a group's count counts, for the per-unit column. | | ||
| | `bench` | — | Command writing a bench JSON to `$BENCH_JSON`. | | ||
| | `working-directory` | `.` | Where to run it, inside each checkout. | | ||
| | `install` | `npm ci --no-audit --no-fund` | Dependency install, at each checkout root. | | ||
| | `prepare` | — | Anything else each checkout needs before benchmarking. | | ||
| | `rounds` | `3` | Sampling rounds per side. | | ||
| | `threshold` | `25` | Percent change reported as a change. | | ||
| | `floor` | `5` | Benchmarks under this many ms are reported, never flagged. | | ||
| | `comment` | `true` | Upsert the sticky comment. | | ||
|
|
||
| ## Reading a group as a per-unit cost | ||
|
|
||
| `bench-report.mjs` divides a benchmark's median by the first integer in its group title. A group named `mount 5000 components, one insertion` therefore reports microseconds per component alongside milliseconds per operation, which is what makes a non-linear curve legible as a number rather than as a shape. Name the unit with `unit:`. A group whose title carries no number simply has no per-unit figure, and the column reads `-`. | ||
|
|
||
| ## Locally | ||
|
|
||
| ```sh | ||
| node .github/actions/bench-diff/bench-report.mjs <vitest-bench.json> # one run, as a table | ||
| node .github/actions/bench-diff/bench-report.mjs a.json b.json --json out.json # several rounds, aggregated | ||
| node .github/actions/bench-diff/bench-comment.mjs base.json head.json # the comment body | ||
| ``` |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,208 @@ | ||
| name: 'Benchmark diff' | ||
| description: 'Run a vitest benchmark suite for the base and the head commit on one runner, alternating, and comment the difference.' | ||
| author: 'Studio Meta' | ||
| branding: | ||
| icon: 'activity' | ||
| color: 'blue' | ||
|
|
||
| # The action knows nothing about how a suite runs. It takes a directory, a | ||
| # command and an output path, and reads the JSON vitest emits — the same | ||
| # schema whether the benchmark bodies execute in Node or in a browser. Keep it | ||
| # that way: a second suite is added by adding a step, not by adding a branch. | ||
|
|
||
| inputs: | ||
| id: | ||
| description: 'Identifier for this suite. Namespaces the sticky comment and the temporary files, so several suites can each keep their own comment on one pull request.' | ||
| required: false | ||
| default: 'bench-diff' | ||
| title: | ||
| description: 'Heading of the sticky comment.' | ||
| required: false | ||
| default: 'Benchmarks' | ||
| unit: | ||
| description: "What a group's count counts, for the per-unit column — `component`, `element`, `node`. Groups whose title carries no number have no per-unit figure." | ||
| required: false | ||
| default: 'unit' | ||
| bench: | ||
| description: 'Command producing a `vitest bench --outputJson` file at `$BENCH_JSON`, run in `working-directory` inside each checkout.' | ||
| required: true | ||
| working-directory: | ||
| description: 'Directory to run `bench` in, relative to a checkout root.' | ||
| required: false | ||
| default: '.' | ||
| install: | ||
| description: 'Command that installs dependencies, run at the root of each checkout.' | ||
| required: false | ||
| default: 'npm ci --no-audit --no-fund' | ||
| prepare: | ||
| description: 'Optional command run after `install` in each checkout, for anything the benchmarks need on disk.' | ||
| required: false | ||
| default: '' | ||
| rounds: | ||
| description: 'Sampling rounds per side. Sides alternate within a round, and the order flips between rounds, so thermal drift and runner contention land on both sides equally.' | ||
| required: false | ||
| default: '3' | ||
| threshold: | ||
| description: 'Percent change reported as a change. Set it from a measured same-commit noise floor, never from taste.' | ||
| required: false | ||
| default: '25' | ||
| floor: | ||
| description: 'Benchmarks with a median under this many milliseconds are reported but never flagged: the timer cannot resolve them.' | ||
| required: false | ||
| default: '5' | ||
| node-version: | ||
| description: 'Node.js version.' | ||
| required: false | ||
| default: '24' | ||
| comment: | ||
| description: 'Upsert a sticky pull-request comment with the diff.' | ||
| required: false | ||
| default: 'true' | ||
| github-token: | ||
| description: 'Token used to upsert the comment (needs `pull-requests: write`).' | ||
| required: false | ||
| default: ${{ github.token }} | ||
|
|
||
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ inputs.node-version }} | ||
| cache: npm | ||
|
|
||
| - name: Install the pull request | ||
| shell: bash | ||
| run: | | ||
| ${{ inputs.install }} | ||
| ${{ inputs.prepare }} | ||
|
|
||
| # The base is measured on this runner, from the same action-provided | ||
| # scripts, rather than read from a cache. A cached baseline comes from | ||
| # another machine under other contention, which re-imports exactly the | ||
| # cross-machine noise this design exists to remove — and a cached number | ||
| # cannot be interleaved with anything. | ||
| - name: Check out the base branch | ||
| if: github.event_name == 'pull_request' | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event.pull_request.base.sha }} | ||
| path: __bench-diff-base | ||
|
|
||
| # No `|| true` here. A base that cannot install or prepare is a broken | ||
| # job, and suppressing it would turn a broken baseline into a confident | ||
| # "everything is new" comment — a green report of a comparison that never | ||
| # happened. Only a base with no benchmark suite may degrade, and that is | ||
| # decided further down, from whether it produced any output at all. | ||
| - name: Install the base branch | ||
| if: github.event_name == 'pull_request' | ||
| shell: bash | ||
| working-directory: __bench-diff-base | ||
| run: | | ||
| ${{ inputs.install }} | ||
| ${{ inputs.prepare }} | ||
|
|
||
| - name: Run both sides, alternating | ||
| shell: bash | ||
| env: | ||
| BENCH_ID: ${{ inputs.id }} | ||
| BENCH_ROUNDS: ${{ inputs.rounds }} | ||
| BENCH_WORKDIR: ${{ inputs.working-directory }} | ||
| BENCH_HAS_BASE: ${{ github.event_name == 'pull_request' }} | ||
| run: | | ||
| # `shell: bash` already runs with `-e -o pipefail`; saying so here | ||
| # keeps the guarantee visible to whoever edits this next. | ||
| set -euo pipefail | ||
| run_side() { | ||
| local side="$1" round="$2" root="$3" | ||
| local out="$RUNNER_TEMP/$BENCH_ID-$side-$round.json" | ||
| # `export` on its own line, not a prefix assignment: `$BENCH_JSON` | ||
| # inside the command is expanded by this shell before the command | ||
|
titouanmathis marked this conversation as resolved.
|
||
| # runs, so a prefix assignment would come too late for it. | ||
| ( cd "$root/$BENCH_WORKDIR" && export BENCH_JSON="$out" && ${{ inputs.bench }} ) \ | ||
| || echo "::warning::the $side benchmark run failed in round $round" | ||
| } | ||
| for round in $(seq 1 "$BENCH_ROUNDS"); do | ||
| if [ "$BENCH_HAS_BASE" != 'true' ]; then | ||
| run_side head "$round" "$GITHUB_WORKSPACE" | ||
| elif [ $((round % 2)) -eq 1 ]; then | ||
| run_side head "$round" "$GITHUB_WORKSPACE" | ||
| run_side base "$round" "$GITHUB_WORKSPACE/__bench-diff-base" | ||
| else | ||
| run_side base "$round" "$GITHUB_WORKSPACE/__bench-diff-base" | ||
| run_side head "$round" "$GITHUB_WORKSPACE" | ||
| fi | ||
| done | ||
|
|
||
| # Three outcomes per side, and they are not the same thing. | ||
| # | ||
| # every round measured -> compare | ||
| # no round measured -> only the base may do this, and only because | ||
| # "the suite does not exist on this commit yet" | ||
| # is a real, expected state on the pull request | ||
| # that adds it. Degrade to an empty base, and | ||
| # say so in the comment. | ||
| # some rounds measured -> a suite that ran and then did not is broken, | ||
| # not absent. Never degrade; fail. | ||
| # | ||
| # Counting output is what separates absent from broken without the action | ||
| # having to know anything about the command it was given. | ||
| - name: Aggregate the rounds | ||
| shell: bash | ||
| env: | ||
| BENCH_ID: ${{ inputs.id }} | ||
| BENCH_ROUNDS: ${{ inputs.rounds }} | ||
| BENCH_HAS_BASE: ${{ github.event_name == 'pull_request' }} | ||
| run: | | ||
| set -euo pipefail | ||
| shopt -s nullglob | ||
|
titouanmathis marked this conversation as resolved.
|
||
| head_rounds=("$RUNNER_TEMP/$BENCH_ID"-head-*.json) | ||
| base_rounds=("$RUNNER_TEMP/$BENCH_ID"-base-*.json) | ||
|
|
||
| if [ ${#head_rounds[@]} -ne "$BENCH_ROUNDS" ]; then | ||
| echo "::error::the head commit measured ${#head_rounds[@]} of $BENCH_ROUNDS rounds" >&2 | ||
| exit 1 | ||
| fi | ||
| node "$GITHUB_ACTION_PATH/bench-report.mjs" "${head_rounds[@]}" \ | ||
| --json "$RUNNER_TEMP/$BENCH_ID-head.json" --markdown | ||
|
|
||
| if [ "$BENCH_HAS_BASE" != 'true' ] || [ ${#base_rounds[@]} -eq 0 ]; then | ||
| if [ "$BENCH_HAS_BASE" = 'true' ]; then | ||
| echo '::warning::the base commit produced no benchmark output; reporting every benchmark as new' >&2 | ||
| fi | ||
| echo '[]' > "$RUNNER_TEMP/$BENCH_ID-base.json" | ||
| elif [ ${#base_rounds[@]} -ne "$BENCH_ROUNDS" ]; then | ||
| echo "::error::the base commit measured ${#base_rounds[@]} of $BENCH_ROUNDS rounds, so its suite is broken rather than absent" >&2 | ||
| exit 1 | ||
| else | ||
| node "$GITHUB_ACTION_PATH/bench-report.mjs" "${base_rounds[@]}" \ | ||
| --json "$RUNNER_TEMP/$BENCH_ID-base.json" | ||
| fi | ||
|
|
||
| - name: Comment on the pull request | ||
| if: inputs.comment == 'true' && github.event_name == 'pull_request' | ||
| shell: bash | ||
| env: | ||
| GH_TOKEN: ${{ inputs.github-token }} | ||
| BENCH_ID: ${{ inputs.id }} | ||
| run: | | ||
| node "$GITHUB_ACTION_PATH/bench-comment.mjs" \ | ||
| "$RUNNER_TEMP/$BENCH_ID-base.json" "$RUNNER_TEMP/$BENCH_ID-head.json" \ | ||
| --threshold '${{ inputs.threshold }}' \ | ||
| --floor '${{ inputs.floor }}' \ | ||
| --rounds '${{ inputs.rounds }}' \ | ||
| --id '${{ inputs.id }}' \ | ||
| --title '${{ inputs.title }}' \ | ||
| --unit '${{ inputs.unit }}' > "$RUNNER_TEMP/$BENCH_ID-comment.md" | ||
| cat "$RUNNER_TEMP/$BENCH_ID-comment.md" >> "$GITHUB_STEP_SUMMARY" | ||
| marker="<!-- bench-diff:$BENCH_ID -->" | ||
| repo='${{ github.repository }}' | ||
| pr='${{ github.event.pull_request.number }}' | ||
| comment_id=$(gh api "repos/$repo/issues/$pr/comments" --paginate \ | ||
| --jq ".[] | select(.body | contains(\"$marker\")) | .id" | head -n1) | ||
| if [ -n "$comment_id" ]; then | ||
| gh api --method PATCH "repos/$repo/issues/comments/$comment_id" -F body=@"$RUNNER_TEMP/$BENCH_ID-comment.md" >/dev/null | ||
| else | ||
| gh api --method POST "repos/$repo/issues/$pr/comments" -F body=@"$RUNNER_TEMP/$BENCH_ID-comment.md" >/dev/null | ||
| fi | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.