Compare pull requests against a baseline measured on the same machine

.github/workflows/ValidatePullRequest.yml

@@ -66,6 +66,26 @@ jobs:
       arch: ${{ matrix.arch }}
       config: ${{ matrix.config }}
 
+  # Guests for the commit the pull request branched from, so the benchmarks
+  # can measure both sides on one machine. Only the configuration the
+  # benchmark suite loads is needed.
+  build-baseline-guests:
+    needs: [docs-pr, update-guest-locks]
+    # Required because update-guest-locks is skipped on non-dependabot PRs,
+    # and a skipped dependency transitively skips all downstream jobs.
+    # See: https://github.com/actions/runner/issues/2205
+    if: ${{ !cancelled() && !failure() }}
+    uses: ./.github/workflows/dep_build_guests.yml
+    secrets: inherit
+    with:
+      docs_only: ${{ needs.docs-pr.outputs.docs-only }}
+      arch: X64
+      config: release
+      # The checkout is the merge this run validates, so where its two
+      # parents branched apart is where the pull request did.
+      ref: HEAD^1...HEAD^2
+      artifact_suffix: -baseline
+
   # Code checks (fmt, clippy, MSRV) - runs in parallel with build-guests
   code-checks:
     needs: [docs-pr, update-guest-locks]
@@ -232,6 +252,7 @@ jobs:
     needs:
       - docs-pr
       - build-guests
+      - build-baseline-guests
     # Required because update-guest-locks is skipped on non-dependabot PRs,
     # and a skipped dependency transitively skips all downstream jobs.
     # See: https://github.com/actions/runner/issues/2205
@@ -249,10 +270,11 @@ jobs:
       hypervisor: ${{ matrix.hypervisor }}
       cpu_vendor: ${{ matrix.cpu_vendor }}
       arch: ${{ matrix.arch }}
+      baseline_ref: HEAD^1...HEAD^2
 
-  # Report every configuration the run measured against the branch point the
-  # pull request was built from, for the hyperlight-gh-bot to post as a PR
-  # comment. Only runs for PRs (not merge groups) with code changes.
+  # Report every configuration the run measured against the commit the pull
+  # request branched from, for the hyperlight-gh-bot to post as a PR comment.
+  # Only runs for PRs (not merge groups) with code changes.
   benchmark-comment:
     name: post-benchmark-comment
     needs:
@@ -277,9 +299,10 @@ jobs:
         run: |
           echo '## Benchmark Results' > pr-comment.md
           echo '' >> pr-comment.md
+          # No baseline is named: each run measured the branch point alongside
+          # the pull request, so the comparison it carries is the one to read.
           cargo ci bench-report --config-file bench_report.toml --reproduce \
-            --candidate run:${{ github.run_id }} \
-            --baseline base-of:${{ github.event.pull_request.number }} >> pr-comment.md
+            --candidate run:${{ github.run_id }} >> pr-comment.md
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
@@ -314,6 +337,7 @@ jobs:
       - docs-pr
       - update-guest-locks
       - build-guests
+      - build-baseline-guests
       - code-checks
       - check-golden-label
       - build-test

.github/workflows/dep_benchmarks.yml

@@ -31,6 +31,14 @@ on:
         description: CPU architecture for the build, X64 or arm64 (passed from caller matrix)
         required: true
         type: string
+      baseline_ref:
+        description: >-
+          Commit to measure these results against. When set, its benchmarks run
+          first on this machine, so the comparison reflects the change rather
+          than the difference between two runners.
+        required: false
+        type: string
+        default: ""
 env:
   CARGO_TERM_COLOR: always
   RUST_BACKTRACE: full
@@ -57,6 +65,9 @@ jobs:
         || '["self-hosted", "Linux", "arm64", "kvm"]') }}
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+        with:
+          # The baseline pass checks out another commit, which needs its history.
+          fetch-depth: ${{ inputs.baseline_ref == '' && 1 || 0 }}
 
       - uses: hyperlight-dev/ci-setup-workflow@2f4142ba17cf573af44fc1e1f1ffc743daded5b3  # v1.10.0
         with:
@@ -90,17 +101,44 @@ jobs:
           name: c-guests-${{ inputs.arch }}-release
           path: src/tests/c_guests/bin/release/
 
+      - name: Download baseline Rust guests
+        if: inputs.baseline_ref != ''
+        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c  # v8.0.1
+        with:
+          name: rust-guests-${{ inputs.arch }}-release-baseline
+          path: ${{ runner.temp }}/baseline-guests/rust/
+
+      - name: Download baseline C guests
+        if: inputs.baseline_ref != ''
+        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c  # v8.0.1
+        with:
+          name: c-guests-${{ inputs.arch }}-release-baseline
+          path: ${{ runner.temp }}/baseline-guests/c/
+
+      # Both commits are measured on this machine, so the comparison reflects
+      # them rather than the difference between two runners. The branch point
+      # is read off the merge this run validates, so on anything else there is
+      # no second parent and the comparison would measure the wrong commit.
       - name: Build
         run: just build release
 
       - name: Run benchmarks
-        run: just bench-ci main
+        run: |
+          if [ -n "${{ inputs.baseline_ref }}" ]; then
+            git rev-parse --verify --quiet HEAD^2 >/dev/null || {
+              echo "::error::HEAD is not a merge, so ${{ inputs.baseline_ref }} is not the branch point"
+              exit 1
+            }
+            just bench-against ${{ inputs.baseline_ref }} "$RUNNER_TEMP/baseline-guests"
+          else
+            just bench-ci main
+          fi
 
       - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a  # v7.0.1
         with:
           name: benchmarks_${{ runner.os }}_${{ inputs.hypervisor }}_${{ inputs.cpu_vendor }}
           path: ./target/criterion/
           if-no-files-found: error
-          # The longest GitHub keeps an artifact, so that a pull request can be
-          # compared against the branch point it was built from.
+          # The longest GitHub keeps an artifact, so that a run stays readable
+          # for as long as the history it contributes to.
           retention-days: 90

.github/workflows/dep_build_guests.yml

@@ -18,6 +18,18 @@ on:
         description: Build configuration (debug or release)
         required: true
         type: string
+      ref:
+        description: >-
+          Commit to build the guests from, as a revision git resolves against
+          the checkout. Defaults to the triggering ref.
+        required: false
+        type: string
+        default: ""
+      artifact_suffix:
+        description: Appended to the artifact names, to keep separate builds apart.
+        required: false
+        type: string
+        default: ""
 
 env:
   CARGO_TERM_COLOR: always
@@ -35,15 +47,32 @@ jobs:
     timeout-minutes: ${{ fromJSON(vars.BUILD_GUESTS_JOB_TIMEOUT || '15') }}
     # Spread load across vendor pools: AMD when run_id ends in 0,2,4,6,8, Intel otherwise.
     runs-on: ${{ fromJson(inputs.arch == 'X64'
-        && format('["self-hosted", "Linux", "X64", "1ES.Pool=hld-kvm-{0}", "JobId=build-guests-{1}-{2}-{3}-{4}"]',
+        && format('["self-hosted", "Linux", "X64", "1ES.Pool=hld-kvm-{0}", "JobId=build-guests-{1}{5}-{2}-{3}-{4}"]',
             (endsWith(github.run_id, '0') || endsWith(github.run_id, '2') || endsWith(github.run_id, '4') || endsWith(github.run_id, '6') || endsWith(github.run_id, '8')) && 'amd' || 'intel',
             inputs.config,
             github.run_id,
             github.run_number,
-            github.run_attempt)
+            github.run_attempt,
+            inputs.artifact_suffix)
         || '["self-hosted", "Linux", "arm64"]') }}
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+        with:
+          # Resolving a revision needs the history it names.
+          fetch-depth: ${{ inputs.ref == '' && 1 || 0 }}
+
+      # The checkout is the merge a pull request's validation builds, so where
+      # its two parents branched apart is where the pull request did. On
+      # anything but a merge there is no second parent to read, which would
+      # build guests for the wrong commit without saying so.
+      - name: Check out the commit to build from
+        if: inputs.ref != ''
+        run: |
+          git rev-parse --verify --quiet HEAD^2 >/dev/null || {
+            echo "::error::HEAD is not a merge, so ${{ inputs.ref }} is not the branch point"
+            exit 1
+          }
+          git checkout --detach ${{ inputs.ref }}
 
       - uses: hyperlight-dev/ci-setup-workflow@2f4142ba17cf573af44fc1e1f1ffc743daded5b3  # v1.10.0
         with:
@@ -103,7 +132,7 @@ jobs:
       - name: Upload Rust guests
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a  # v7.0.1
         with:
-          name: rust-guests-${{ inputs.arch }}-${{ inputs.config }}
+          name: rust-guests-${{ inputs.arch }}-${{ inputs.config }}${{ inputs.artifact_suffix }}
           path: |
             src/tests/rust_guests/bin/${{ inputs.config }}/
           retention-days: 1
@@ -112,7 +141,7 @@ jobs:
       - name: Upload C guests
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a  # v7.0.1
         with:
-          name: c-guests-${{ inputs.arch }}-${{ inputs.config }}
+          name: c-guests-${{ inputs.arch }}-${{ inputs.config }}${{ inputs.artifact_suffix }}
           path: src/tests/c_guests/bin/${{ inputs.config }}/
           retention-days: 1
           if-no-files-found: error

Justfile

@@ -462,6 +462,10 @@ tar-static-lib: (build-rust-capi "release") (build-rust-capi "debug")
 bench-ci baseline features="":
     cargo ci bench {{ if features == "" {''} else { "--features " + features } }} --verbose --save-baseline {{ baseline }}
 
+# Measure a commit and the current tree on this machine, and compare them
+bench-against commit guests features="":
+    cargo ci bench {{ if features == "" {''} else { "--features " + features } }} --baseline-ref {{ commit }} --baseline-guests {{ guests }} --verbose
+
 bench features="":
     cargo ci bench {{ if features == "" {''} else { "--features " + features } }} --verbose
 

src/hyperlight_ci/Cargo.toml

@@ -12,7 +12,7 @@ workspace = true
 [dependencies]
 anyhow = "1"
 clap = { version = "4.6.1", features = ["derive"] }
-criterion-markdown = "0.2.1"
+criterion-markdown = "0.2.2"
 criterion-swarm = "0.2.2"
 regex = "1"
 serde = { version = "1.0", features = ["derive"] }