uefi-bench: Comment benchmark deltas on PRs

.github/workflows/uefi-bench-pr-comment.yml

@@ -0,0 +1,155 @@
+# GitHub Actions: posts patina_boot benchmark results back to the originating
+# pull request.
+#
+# SPDX-License-Identifier: MIT
+#
+
+# This workflow triggers after the uefi-bench workflow has run. uefi-bench runs
+# in the pull request context, which is read-only for pull requests from forks,
+# so it uploads its results as an artifact instead of commenting. This workflow
+# runs in the base repository context, where it has pull-requests: write, and
+# renders the artifact into a single comment that is updated in place on each
+# push rather than appended to.
+#
+# The comment is a reviewer aid, not a gate. It shows each benchmark at the
+# base commit alongside the same benchmark on the pull request so that an
+# unexpected swing is visible during review. Changes smaller than the combined
+# run-to-run spread are labelled as such, because shared CI runners routinely
+# move these numbers by more than a real code change would.
+
+name: uefi-bench PR comment
+
+on:
+  workflow_run:
+    workflows: [uefi-bench]
+    types:
+      - completed
+
+permissions:
+  contents: read
+  pull-requests: write
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.workflow_run.head_branch || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  comment:
+    name: post results
+    runs-on: ubuntu-latest
+    # Only pull request runs have a comment to post to, and a failed bench run
+    # uploads no results worth rendering. uefi-bench already fails loudly in
+    # that case.
+    if: >-
+      github.event.workflow_run.event == 'pull_request' &&
+      github.event.workflow_run.conclusion == 'success'
+    steps:
+      - name: Download results
+        uses: actions/download-artifact@v4
+        with:
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          name: bench-results
+          path: bench-results/
+          run-id: ${{ github.event.workflow_run.id }}
+
+      - name: Get PR number
+        id: get-pr-number
+        shell: bash
+        run: |
+          set -euo pipefail
+          echo "pr_number=$(cat ./bench-results/NR)" >> "$GITHUB_OUTPUT"
+
+      - name: Render results table
+        shell: bash
+        env:
+          HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
+        run: |
+          set -euo pipefail
+
+          # Bencher lines look like:
+          #   test connect_all/512 ... bench:  142019 cycles/iter (+/- 39534)
+          # Reduce both runs to name/value/unit/spread so they can be joined.
+          normalize() {
+            sed -nE 's@^test (.+) \.\.\. bench:[[:space:]]+([0-9]+) ([^ ]+) \(\+/- ([0-9]+)\)$@\1\t\2\t\3\t\4@p' "$1"
+          }
+
+          normalize ./bench-results/results-base.txt > base.tsv
+          normalize ./bench-results/results-head.txt > head.tsv
+
+          if [ ! -s head.tsv ]; then
+            echo "No benchmark lines could be parsed from the results artifact" >&2
+            exit 1
+          fi
+
+          {
+            echo '## patina_boot benchmarks'
+            echo
+            echo "Base commit vs. this pull request, measured at \`${HEAD_SHA}\`."
+            echo
+            echo '| Benchmark | Base | This PR | Change | Spread |'
+            echo '| --- | ---: | ---: | ---: | ---: |'
+          } > comment.md
+
+          # The unit travels with each value so that a benchmark quietly
+          # switching its measurement shows up instead of being compared as if
+          # the numbers were still commensurable.
+          awk -F'\t' '
+            NR == FNR {
+              bval[$1] = $2; bunit[$1] = $3; bvar[$1] = $4
+              next
+            }
+            {
+              name = $1; hval = $2; hunit = $3; hvar = $4
+              if (!(name in bval)) {
+                printf "| `%s` | — | %s %s | new | ± %s |\n", name, hval, hunit, hvar
+                next
+              }
+              if (bunit[name] != hunit) {
+                printf "| `%s` | %s %s | %s %s | unit changed | ± %s |\n", \
+                  name, bval[name], bunit[name], hval, hunit, hvar
+                next
+              }
+              diff = hval - bval[name]
+              pct = bval[name] > 0 ? diff / bval[name] * 100 : 0
+              change = sprintf("%+.1f%%", pct)
+              magnitude = diff < 0 ? -diff : diff
+              if (magnitude <= bvar[name] + hvar) {
+                change = change " (within spread)"
+              }
+              printf "| `%s` | %s %s | %s %s | %s | ± %s |\n", \
+                name, bval[name], bunit[name], hval, hunit, change, hvar
+            }
+          ' base.tsv head.tsv >> comment.md
+
+          {
+            echo
+            echo 'Informational only; this does not gate the merge. Shared CI'
+            echo 'runners are noisy, so treat a change marked "within spread"'
+            echo 'as no signal and confirm anything surprising locally with'
+            echo '`cargo bench -p patina_boot` before acting on it.'
+            echo
+            echo '<!--'
+            echo 'This comment is auto-generated by the uefi-bench workflow.'
+            echo 'Please do not edit it directly.'
+            echo
+            echo 'comment-tag: [uefi-bench]'
+            echo '-->'
+          } >> comment.md
+
+      - name: Find existing comment
+        id: find-comment
+        uses: peter-evans/find-comment@v3
+        with:
+          issue-number: ${{ steps.get-pr-number.outputs.pr_number }}
+          comment-author: 'github-actions[bot]'
+          body-includes: 'comment-tag: [uefi-bench]'
+
+      # An empty comment-id creates a new comment; a populated one replaces the
+      # previous results so the pull request keeps a single, current comment.
+      - name: Post results
+        uses: peter-evans/create-or-update-comment@v4
+        with:
+          comment-id: ${{ steps.find-comment.outputs.comment-id }}
+          issue-number: ${{ steps.get-pr-number.outputs.pr_number }}
+          body-path: comment.md
+          edit-mode: replace

.github/workflows/uefi-bench.yml

@@ -0,0 +1,152 @@
+# GitHub Actions: runs the patina_boot microbenchmarks on the pull request and
+# on its base commit, and saves both result sets for the PR comment workflow.
+#
+# SPDX-License-Identifier: MIT
+#
+
+# Benchmarks run on pull requests that touch patina_boot. The same benches are
+# run twice, once at the pull request's base commit and once at the merge
+# result, so the comment can show a per-benchmark change rather than a bare
+# number that a reviewer has nothing to weigh against.
+#
+# Results are uploaded as an artifact rather than commented on from here: a
+# pull_request run from a fork gets a read-only token and cannot post comments.
+# The companion uefi-bench-pr-comment workflow runs in the base repository
+# context and does the writing.
+#
+# These numbers are informational. Shared CI runners are noisy, so this reports
+# variance for a reviewer to judge and never fails the build on a slow result.
+
+permissions:
+  contents: read
+
+on:
+  pull_request:
+    paths:
+      - 'uefi/crates/patina_boot/**'
+      - '.github/workflows/uefi-bench.yml'
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+name: uefi-bench
+
+jobs:
+  bench:
+    name: bench / patina_boot
+    runs-on: ubuntu-latest
+    steps:
+      # Full history: the base commit must be present locally to check out and
+      # benchmark it.
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      # `rust-toolchain.toml` pins the channel + components + targets, but
+      # `rustup show` doesn't reliably install missing pieces in CI. Install
+      # them explicitly. Update the channel here whenever rust-toolchain.toml
+      # is bumped.
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: nightly-2026-02-27
+          components: rust-src
+
+      # The Cycles measurement reads the x86 timestamp counter, so the runner
+      # must be x86_64. Fail with a clear message rather than a build error.
+      - name: Verify runner architecture
+        shell: bash
+        run: |
+          set -euo pipefail
+          arch="$(uname -m)"
+          if [ "$arch" != "x86_64" ]; then
+            echo "patina_boot benches require an x86_64 runner (rdtsc); got $arch" >&2
+            exit 1
+          fi
+
+      - name: Prepare results directory
+        shell: bash
+        run: |
+          set -euo pipefail
+          # Both bench runs write here. Kept outside the working tree so that
+          # switching commits between runs cannot disturb it.
+          mkdir -p "$RUNNER_TEMP/bench-results"
+
+      # The base commit may predate a benchmark, or fail to build for reasons
+      # that are not this pull request's problem. A missing baseline is
+      # reported per benchmark as "new" rather than failing the run.
+      - name: Benchmark base commit
+        shell: bash
+        working-directory: uefi/crates/patina_boot
+        env:
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+        run: |
+          set -uo pipefail
+          git checkout --quiet --detach "$BASE_SHA"
+
+          # `cargo bench --benches` would also run the lib unittest target,
+          # which is libtest and rejects criterion's arguments. Name the bench
+          # targets explicitly so this works on any commit, including ones
+          # predating a given bench.
+          bench_args=()
+          while IFS= read -r name; do
+            bench_args+=(--bench "$name")
+          done < <(cargo metadata --no-deps --format-version 1 \
+            | jq -r '.packages[].targets[] | select(.kind[] == "bench") | .name')
+
+          if [ "${#bench_args[@]}" -eq 0 ]; then
+            echo "Base commit $BASE_SHA defines no bench targets; reporting no baseline." >&2
+            : > "$RUNNER_TEMP/bench-results/results-base.txt"
+            exit 0
+          fi
+
+          if cargo bench "${bench_args[@]}" -- --output-format bencher > base-output.txt 2>&1; then
+            grep '^test ' base-output.txt > "$RUNNER_TEMP/bench-results/results-base.txt" || true
+          else
+            echo "Base commit $BASE_SHA failed to benchmark; reporting no baseline." >&2
+            sed -n '1,40p' base-output.txt >&2
+            : > "$RUNNER_TEMP/bench-results/results-base.txt"
+          fi
+
+      - name: Benchmark pull request
+        shell: bash
+        working-directory: uefi/crates/patina_boot
+        env:
+          HEAD_SHA: ${{ github.sha }}
+        run: |
+          set -euo pipefail
+          git checkout --quiet --detach "$HEAD_SHA"
+
+          # See the base step: name the bench targets rather than using
+          # --benches, which would also run the lib unittest target.
+          bench_args=()
+          while IFS= read -r name; do
+            bench_args+=(--bench "$name")
+          done < <(cargo metadata --no-deps --format-version 1 \
+            | jq -r '.packages[].targets[] | select(.kind[] == "bench") | .name')
+
+          if [ "${#bench_args[@]}" -eq 0 ]; then
+            echo "No bench targets are defined in patina_boot" >&2
+            exit 1
+          fi
+
+          cargo bench "${bench_args[@]}" -- --output-format bencher | tee head-output.txt
+          if ! grep '^test ' head-output.txt > "$RUNNER_TEMP/bench-results/results-head.txt"; then
+            echo "No benchmark result lines found in the bench output" >&2
+            exit 1
+          fi
+
+      - name: Record pull request number
+        shell: bash
+        run: |
+          set -euo pipefail
+          # The comment workflow is triggered by workflow_run and so cannot see
+          # the pull request number. Carry it across in the artifact.
+          echo "${{ github.event.number }}" > "$RUNNER_TEMP/bench-results/NR"
+
+      - uses: actions/upload-artifact@v4
+        with:
+          name: bench-results
+          path: ${{ runner.temp }}/bench-results/
+          overwrite: true

uefi/crates/patina_boot/benches/orchestrator.rs

@@ -7,6 +7,10 @@
 //! Add `-- --output-format bencher` for libtest-style lines that
 //! standard perf-tracking tooling consumes.
 //!
+//! Reports elapsed reference cycles (`rdtsc`) per iteration via the shared
+//! [`support::Cycles`] measurement, matching the other benches in this crate
+//! so every reported number shares one unit.
+//!
 //! ## License
 //!
 //! Copyright (c) Microsoft Corporation.
@@ -24,6 +28,9 @@ use patina::uefi::boot_services::{MockBootServices, boxed::BootServicesBox};
 use patina_boot::helpers;
 use r_efi::efi;
 
+#[path = "support/mod.rs"]
+mod support;
+
 /// Build a `MockBootServices` whose method expectations cover the
 /// sequence `connect_all` + `signal_bds_phase_entry` +
 /// `signal_ready_to_boot` exercise: `locate_handle_buffer`,
@@ -84,7 +91,7 @@ fn build_mock() -> &'static MockBootServices {
 /// table with stub function pointers) that does not exist yet.
 /// Pending that, this composite is the closest end-to-end measurement
 /// of the BDS chain achievable against the public helper surface.
-fn bds_phase_composite(c: &mut Criterion) {
+fn bds_phase_composite(c: &mut Criterion<support::Cycles>) {
     let mock = build_mock();
     let iter_count = AtomicUsize::new(0);
 
@@ -100,5 +107,9 @@ fn bds_phase_composite(c: &mut Criterion) {
     });
 }
 
-criterion_group!(benches, bds_phase_composite);
+criterion_group! {
+    name = benches;
+    config = Criterion::default().with_measurement(support::Cycles);
+    targets = bds_phase_composite
+}
 criterion_main!(benches);