Improve benchmark result reporting (#357)

* Add script to summarize benchmark results

* Mess with github actions benchmark script

* Formatting fixes

* Comment

* Bring back commit comments, upload to DO Spaces

* Remove cache

We now have servers for benchmarks outside of GH infrastructure and
downloading cache takes way more time than its worth it

* base and head refs are not needed anymore

* Run benchmarks only on master

* Test a branch build

* Fix branch becnhmarks path

* test

* Run only on master again

---------

Signed-off-by: Piotr Sarnacki <drogus@gmail.com>
Co-authored-by: Piotr Sarnacki <drogus@gmail.com>

.github/workflows/benchmarks.yml

@@ -2,14 +2,18 @@ on:
   push:
     branches:
       - master
-      - kazimuth/benchwrangle
+
   workflow_dispatch:
     inputs:
       pr_number:
         description: 'Pull Request Number'
         required: false
         default: ''
 
+  # note: the "benchmarks please" comments aren't dispatched here,
+  # there's a script running on one of our internal servers that reads those and then
+  # dispatches to this workflow using the workflow_dispatch there.
+
 name: Benchmarks
 
 env:
@@ -20,6 +24,9 @@ jobs:
     name: run benchmarks
     runs-on: benchmarks-runner
     steps:
+      - name: Enable CPU boost
+        run: echo "1" | sudo tee /sys/devices/system/cpu/cpufreq/boost
+
       - name: Checkout sources for a PR
         if: ${{ github.event.inputs.ref }}
         uses: actions/checkout@v3
@@ -37,9 +44,7 @@ jobs:
         if: github.event.inputs.pr_number
         run: |
           echo "PR_NUMBER=${{ github.event.inputs.pr_number }}" >> $GITHUB_ENV
-          PR_DATA=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.inputs.pr_number }} --jq '{ baseRefName: .base.ref, headRefName: .head.ref }')
-          echo "PR_BASE_REF=$(echo $PR_DATA | jq -r '.baseRefName')" >> $GITHUB_ENV
-          echo "PR_HEAD_REF=$(echo $PR_DATA | jq -r '.headRefName')" >> $GITHUB_ENV
+
       - name: Install stable toolchain
         uses: actions-rs/toolchain@v1
         with:
@@ -48,19 +53,6 @@ jobs:
           target: wasm32-unknown-unknown
           override: true
 
-      - name: ⚡ Cache
-        uses: actions/cache@v3
-        with:
-          path: |
-            ~/.cargo/bin/
-            ~/.cargo/registry/index/
-            ~/.cargo/registry/cache/
-            ~/.cargo/git/db/
-            ~/.cargo/.crates.toml
-            ~/.cargo/.crates2.json
-            target/
-          key: ${{ runner.os }}-cargo-bench-${{ hashFiles('**/Cargo.lock') }}
-
       - name: Build
         working-directory: crates/bench/
         run: |
@@ -70,19 +62,132 @@ jobs:
         run: |
           rustup component add clippy
 
-      - name: Criterion compare base branch
-        if: ${{ env.PR_BASE_REF }}
-        uses: clockworklabs/criterion-compare-action@main
+      - name: Disable CPU boost
+        run: echo "0" | sudo tee /sys/devices/system/cpu/cpufreq/boost
+
+      - name: Extract branch name
+        if: "! github.event.inputs.pr_number"
+        shell: bash
+        run: |
+          BRANCH_NAME=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}
+          echo "BRANCH_NAME=$BRANCH_NAME" >> $GITHUB_ENV
+          echo "NORMALIZED_BRANCH_NAME=${BRANCH_NAME//\//-}" >> $GITHUB_ENV
+
+      - name: Branch; run bench
+        if: "! github.event.inputs.pr_number"
+        run: |
+          echo "Running benchmarks with sqlite"
+          pushd crates/bench
+          cargo bench --bench generic --bench special -- --save-baseline $NORMALIZED_BRANCH_NAME
+          cargo run --bin summarize pack $NORMALIZED_BRANCH_NAME
+          popd
+          mkdir criterion-results
+          cp target/criterion/$NORMALIZED_BRANCH_NAME.json criterion-results/
+          cp target/criterion/$NORMALIZED_BRANCH_NAME.json criterion-results/$GITHUB_SHA.json
+
+      # TODO: can we optionally download if it only might fail?
+      #- name: PR; download bench results for compare
+      #  if: github.event.inputs.pr_number
+      #  uses: actions/github-script@v6  
+      #  with:
+      #    github-token: ${{secrets.GITHUB_TOKEN}}
+      #    script: |
+      #      try {
+      #        let artifact = github.rest.actions.getArtifact({
+      #          owner: "clockwork",
+      #          repo: "SpacetimeDB",
+      #          
+      #        })
+      #      }
+
+      - name: PR; run bench
+        if: github.event.inputs.pr_number
+        run: |
+          echo "Running benchmarks without sqlite"
+          # have to pass explicit names, otherwise it will try to run the tests and fail for some reason...
+          pushd crates/bench
+          cargo bench --bench generic --bench special -- --save-baseline branch '(special|stdb_module|stdb_raw)'
+          cargo run --bin summarize pack branch
+          popd
+          mkdir criterion-results
+          cp target/criterion/branch.json criterion-results/pr-$PR_NUMBER.json
+
+      - name: PR; compare benchmarks
+        if: github.event.inputs.pr_number
+        working-directory: crates/bench/
+        run: |
+          if [ -e target/criterion/$NORMALIZED_BRANCH_NAME.json ]; then
+            cargo run --bin summarize markdown-report branch.json $NORMALIZED_BRANCH_NAME.json --report-name report
+          else
+            cargo run --bin summarize markdown-report branch.json --report-name report
+          fi
+
+      # this will work for both PR and master
+      - name: Upload criterion results to DO spaces
+        uses: shallwefootball/s3-upload-action@master
         with:
-          cwd: "crates/bench"
-          branchName: ${{ env.PR_BASE_REF }}
+          aws_key_id: ${{ secrets.AWS_KEY_ID }}
+          aws_secret_access_key: ${{ secrets.AWS_SECRET_ACCESS_KEY}}
+          aws_bucket: "spacetimedb-ci-benchmarks"
+          source_dir: criterion-results
+          endpoint: https://nyc3.digitaloceanspaces.com
+          destination_dir: benchmarks
+
+      - name: Fetch markdown summary PR
+        if: github.event.inputs.pr_number
+        run: |
+          curl -sS https://benchmarks.spacetimedb.com/compare/master/pr-$PR_NUMBER > report.md
+
+      - name: Fetch markdown summary PR
+        if: "! github.event.inputs.pr_number"
+        run: |
+          git fetch
+          old=$(git rev-parse HEAD~1)
+          curl -sS https://benchmarks.spacetimedb.com/compare/$old/$GITHUB_SHA > report.md
 
-      - name: Criterion compare previous commit
-        if: env.PR_BASE_REF == ''
-        uses: clockworklabs/criterion-compare-action@main
+      # https://stackoverflow.com/questions/58066966/commenting-a-pull-request-in-a-github-action
+      # https://github.com/boa-dev/criterion-compare-action/blob/main/main.js
+      - name: test comment
+        uses: actions/github-script@v6
         with:
-          cwd: "crates/bench"
-          branchName: "HEAD~1"
+          github-token: ${{secrets.GITHUB_TOKEN}}
+          script: |
+            let stuff = require('fs').readFileSync('report.md', 'utf8');
+            let body = `<details><summary>Benchmark results</summary>\n\n${stuff}\n\n</details>`;
+
+            try {
+              if (process.env.PR_NUMBER) {
+                let number = parseInt(process.env.PR_NUMBER);
+                core.info("context: issue number: "+number)
+                const { data: comment } = await github.rest.issues.createComment({
+                  owner: "clockworklabs",
+                  repo: "SpacetimeDB",
+                  issue_number: number,
+                  body: body,
+                });
+                core.info(
+                  `Created comment id '${comment.id}' on issue '${number}' in 'clockworklabs/SpacetimeDB'.`
+                );
+                core.setOutput("comment-id", comment.id);
+              } else {
+                const { data: comment } = github.rest.repos.createCommitComment({
+                  commit_sha: context.sha,
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  body: body
+                })
+                core.info(
+                  `Created comment id '${comment.id}' on commit '${context.sha}' in 'clockworklabs/SpacetimeDB'.`
+                );
+                core.setOutput("comment-id", comment.id);
+              }
+            } catch (err) {
+              core.warning(`Failed to comment: ${err}`);
+              core.info("Commenting is not possible from forks.");
+              core.info("Logging here instead.");
+              console.log(body);
+            }
+
 
       - name: Clean up
         if: always()

Cargo.toml

@@ -175,6 +175,7 @@ tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
 url = "2.3.1"
 urlencoding = "2.1.2"
 uuid = { version = "1.2.1", features = ["v4"] }
+walkdir = "2.2.5"
 wasmbin = "0.6"
 
 # wasmer prior to 4.1.1 had a signal handling bug on macOS.

crates/bench/Cargo.toml

@@ -13,6 +13,9 @@ harness = false
 name = "generic"
 harness = false
 
+[[bin]]
+name = "summarize"
+
 [lib]
 bench = false
 
@@ -39,3 +42,5 @@ byte-unit.workspace = true
 futures.workspace = true
 tracing-subscriber.workspace = true
 lazy_static.workspace = true
+walkdir.workspace = true
+regex.workspace = true

crates/bench/README.md

@@ -57,13 +57,36 @@ cargo bench -- 'mem/.*/unique'
 ```
 Will run benchmarks involving unique primary keys against all databases, without writing to disc.
 
+## Pretty report
+To generate a nicely formatted markdown report, you can use the "summarize" binary.
+This is used on CI (see [`../../.github/workflows/benchmarks.yml`](../../.github/workflows/benchmarks.yml)).
+
+To generate a report without comparisons, use:
+```bash
+cargo bench --bench generic --bench special -- --save-baseline current
+cargo run --bin summarize markdown-report current
+```
+
+To compare to another branch, do:
+```bash
+git checkout master
+cargo bench --bench generic --bench special -- --save-baseline base
+git checkout high-octane-feature-branch
+cargo bench --bench generic --bench special -- --save-baseline current
+cargo run --bin summarize markdown-report current base
+```
+
+Of course, this will take about an hour, so it might be better to let the CI do it for you.
+
 ## Adding more
 
 There are two ways to write benchmarks:
 
 - Targeted, non-generic benchmarks (`benches/special.rs`)
 - Generic over database backends (`benches/generic.rs`)
 
+See the following sections for how to write these.
+
 ### Targeted benchmarks
 These are regular [Criterion.rs](https://github.com/bheisler/criterion.rs) benchmarks. Nothing fancy, do whatever you like with these. Put them in `benches/special.rs`.
 
@@ -95,12 +118,6 @@ There are also some scripts that rely on external tools to extract data from the
 cargo install critcmp
 ```
 
-The simplest way to use critcmp is to save two baselines with Criterion's benchmark harness and then compare them. For example:
-```bash
-cargo bench -- --save-baseline before
-cargo bench -- --save-baseline change
-critcmp before change
-```
 
 ### OSX Only
 

crates/bench/benches/special.rs

@@ -47,7 +47,7 @@ fn custom_module_benchmarks(c: &mut Criterion) {
 fn serialize_benchmarks<T: BenchTable + RandomTable>(c: &mut Criterion) {
     let name = T::name_snake_case();
     let count = 100;
-    let mut group = c.benchmark_group("serialize");
+    let mut group = c.benchmark_group("special/serialize");
     group.throughput(criterion::Throughput::Elements(count));
 
     let data = create_sequential::<T>(0xdeadbeef, count as u32, 100);

crates/bench/clippy.toml

@@ -0,0 +1,2 @@
+# we use println in summarize.rs, don't complain about it
+disallowed-macros = []