diff --git a/.github/workflows/uefi-bench-pr-comment.yml b/.github/workflows/uefi-bench-pr-comment.yml new file mode 100644 index 0000000..a38b613 --- /dev/null +++ b/.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 '' + } >> 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 diff --git a/.github/workflows/uefi-bench.yml b/.github/workflows/uefi-bench.yml new file mode 100644 index 0000000..1468eb3 --- /dev/null +++ b/.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 diff --git a/uefi/crates/patina_boot/benches/orchestrator.rs b/uefi/crates/patina_boot/benches/orchestrator.rs index 2f6daa4..1454439 100644 --- a/uefi/crates/patina_boot/benches/orchestrator.rs +++ b/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) { 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);