From 104a2b0b26507d9f3f9b443417764e4f9c6b90dc Mon Sep 17 00:00:00 2001 From: Marvin Drees Date: Fri, 3 Jul 2026 01:21:49 +0200 Subject: [PATCH] feat(ci): add benchci for better visualization (#149) This adds a small in-repo tool and updates the ci.yml file in order to make benchmark and most importantly allocation results visible in a PR. It also drops the 3rdparty action we had commented out. Signed-off-by: Marvin Drees --- .github/workflows/bench-comment.yaml | 78 ++++++++++ .github/workflows/ci.yaml | 28 ++-- internal/benchci/main.go | 219 +++++++++++++++++++++++++++ 3 files changed, 311 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/bench-comment.yaml create mode 100644 internal/benchci/main.go diff --git a/.github/workflows/bench-comment.yaml b/.github/workflows/bench-comment.yaml new file mode 100644 index 0000000..f52e7b1 --- /dev/null +++ b/.github/workflows/bench-comment.yaml @@ -0,0 +1,78 @@ +name: Benchmark Comment + +# Rationale: This more privileged workflow runs in the base repo's +# context (with a write token) only after the 'untrusted' CI workflow finishes. +# It does NOT check out or execute PR code; it only consumes the benchmark +# report artifact as inert, validated data. +on: + workflow_run: + workflows: [CI] + types: [completed] + +permissions: + contents: read + +jobs: + comment: + # Only for PR-triggered CI runs that succeeded to suppress noise + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write # needed to comment + steps: + - name: Download generated benchmark report + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.1 + with: + name: bench-report + path: bench-report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Validate report and upsert PR comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + set -euo pipefail + + report="bench-report/bench-report.md" + marker='' + + # Treat the artifact as untrusted input: it was produced by a job that + # ran PR code. Basic validation before posting. TODO: Make more robust. + test -f "$report" + size="$(wc -c < "$report")" + if [ "$size" -le 0 ] || [ "$size" -gt 1000000 ]; then + echo "::error::benchmark report has unexpected size: ${size} bytes" + exit 1 + fi + if ! grep -qF "$marker" "$report"; then + echo "::error::benchmark report missing marker ${marker}" + exit 1 + fi + + # Resolve the PR from the run's head SHA rather than trusting any + # value carried in the artifact, so we can only comment on the PR that + # actually produced this run. + pr="$(gh api "repos/${REPO}/commits/${HEAD_SHA}/pulls" \ + --jq 'map(select(.state == "open")) | .[0].number // empty')" + if [ -z "$pr" ]; then + echo "No open PR found for ${HEAD_SHA}; nothing to comment." + exit 0 + fi + + 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=@"$report" + else + gh api --method POST "repos/${REPO}/issues/${pr}/comments" \ + -f body=@"$report" + fi diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3c7f362..c1f2914 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -104,7 +104,7 @@ jobs: needs: [test] runs-on: ubuntu-latest permissions: - pull-requests: write + contents: read steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -115,20 +115,20 @@ jobs: - name: Run benchmarks run: go test -bench=. -benchmem -count=5 -shuffle=on -run='^$' -timeout=15m ./... | tee bench-results.txt - # Fails for a weird reason, will be investigated in the future - # - name: Report benchmark regressions - # uses: benchmark-action/github-action-benchmark@a887eba2af2000fda74e733fcf952aa823b65916 # v1.21.0 - # with: - # tool: go - # output-file-path: bench-results.txt - # summary-always: true - # comment-on-alert: true - # alert-threshold: "120%" - # fail-on-alert: false - # auto-push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - # github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Generate benchmark report + run: | + go run ./internal/benchci -current bench-results.txt -out bench-report.md + cat bench-report.md >> "$GITHUB_STEP_SUMMARY" - - name: Upload benchmark results + - name: Upload generated benchmark report + if: github.event_name == 'pull_request' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: bench-report + path: bench-report.md + retention-days: 30 + + - name: Upload raw benchmark results if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/internal/benchci/main.go b/internal/benchci/main.go new file mode 100644 index 0000000..3e92aed --- /dev/null +++ b/internal/benchci/main.go @@ -0,0 +1,219 @@ +// Command benchci parses `go test -bench` output and renders a Markdown +// report. It is repo-owned tooling so CI does not depend on third-party +// benchmark actions. With -count>1 it reports the median of each metric to +// reduce noise. +package main + +import ( + "bufio" + "flag" + "fmt" + "io" + "os" + "sort" + "strconv" + "strings" +) + +// commentMarker is a stable HTML marker so a PR commenter can locate and +// update an existing report comment instead of posting duplicates. +const commentMarker = "" + +func main() { + var ( + currentPath = flag.String("current", "", "path to `go test -bench` output (default stdin)") + outPath = flag.String("out", "", "path to write Markdown report (default stdout)") + title = flag.String("title", "Benchmark results", "report heading") + ) + flag.Parse() + + in := io.Reader(os.Stdin) + if *currentPath != "" { + f, err := os.Open(*currentPath) + if err != nil { + fatal(err) + } + defer f.Close() + in = f + } + + results, err := parse(in) + if err != nil { + fatal(err) + } + + out := io.Writer(os.Stdout) + if *outPath != "" { + f, err := os.Create(*outPath) + if err != nil { + fatal(err) + } + defer f.Close() + out = f + } + + if err := render(out, *title, results); err != nil { + fatal(err) + } +} + +func fatal(err error) { + fmt.Fprintln(os.Stderr, "benchci:", err) + os.Exit(1) +} + +// result is the aggregated metrics for a single benchmark. +type result struct { + pkg string + name string // benchmark name including the GOMAXPROCS suffix, e.g. BenchmarkFoo-12 + + nsPerOp []float64 + bytesPerOp []float64 + allocsPerOp []float64 +} + +// parse reads `go test -bench -benchmem` output and groups metric samples by +// package and benchmark name. Repeated lines (from -count) accumulate samples. +func parse(r io.Reader) ([]result, error) { + sc := bufio.NewScanner(r) + sc.Buffer(make([]byte, 0, 64*1024), 1024*1024) + + byKey := make(map[string]*result) + var order []string + var pkg string + + for sc.Scan() { + line := sc.Text() + fields := strings.Fields(line) + if len(fields) == 0 { + continue + } + if fields[0] == "pkg:" && len(fields) >= 2 { + pkg = fields[1] + continue + } + if !strings.HasPrefix(fields[0], "Benchmark") || len(fields) < 4 { + continue + } + // fields: name iters value unit [value unit]... + name := fields[0] + if _, err := strconv.Atoi(fields[1]); err != nil { + continue // second field must be the iteration count + } + + key := pkg + "\x00" + name + res := byKey[key] + if res == nil { + res = &result{pkg: pkg, name: name} + byKey[key] = res + order = append(order, key) + } + parseMetrics(res, fields[2:]) + } + if err := sc.Err(); err != nil { + return nil, err + } + + out := make([]result, 0, len(order)) + for _, k := range order { + out = append(out, *byKey[k]) + } + sort.Slice(out, func(i, j int) bool { + if out[i].pkg != out[j].pkg { + return out[i].pkg < out[j].pkg + } + return out[i].name < out[j].name + }) + return out, nil +} + +// parseMetrics consumes (value, unit) pairs and appends the metrics benchci +// reports on. Unknown metrics are ignored. +func parseMetrics(res *result, tokens []string) { + for i := 0; i+1 < len(tokens); i += 2 { + v, err := strconv.ParseFloat(tokens[i], 64) + if err != nil { + continue + } + switch tokens[i+1] { + case "ns/op": + res.nsPerOp = append(res.nsPerOp, v) + case "B/op": + res.bytesPerOp = append(res.bytesPerOp, v) + case "allocs/op": + res.allocsPerOp = append(res.allocsPerOp, v) + } + } +} + +// median returns the median of samples. ok is false when there are no samples. +func median(samples []float64) (value float64, ok bool) { + if len(samples) == 0 { + return 0, false + } + s := append([]float64(nil), samples...) + sort.Float64s(s) + n := len(s) + if n%2 == 1 { + return s[n/2], true + } + return (s[n/2-1] + s[n/2]) / 2, true +} + +func render(w io.Writer, title string, results []result) error { + bw := bufio.NewWriter(w) + fmt.Fprintf(bw, "%s\n\n", commentMarker) + fmt.Fprintf(bw, "### %s\n\n", title) + + if len(results) == 0 { + fmt.Fprintln(bw, "_No benchmarks found._") + return bw.Flush() + } + + fmt.Fprintln(bw, "_Timing results (`ns/op`) depend on the host CPU and are only a rough guideline. Memory results (`B/op` and `allocs/op`) are not affected._") + fmt.Fprintln(bw) + fmt.Fprintln(bw, "| Package | Benchmark | ns/op | B/op | allocs/op |") + fmt.Fprintln(bw, "|---|---|---:|---:|---:|") + for _, r := range results { + fmt.Fprintf(bw, "| %s | %s | %s | %s | %s |\n", + shortPkg(r.pkg), r.name, + formatNs(median(r.nsPerOp)), + formatCount(median(r.bytesPerOp)), + formatCount(median(r.allocsPerOp)), + ) + } + return bw.Flush() +} + +// shortPkg trims the well-known module prefix for readability. +func shortPkg(pkg string) string { + const prefix = "github.com/soypat/lneto/" + if pkg == "" { + return "-" + } + return strings.TrimPrefix(pkg, prefix) +} + +func formatCount(v float64, ok bool) string { + if !ok { + return "-" + } + return strconv.FormatFloat(v, 'f', -1, 64) +} + +// formatNs renders a ns/op value using a human-friendly time unit. +func formatNs(v float64, ok bool) string { + if !ok { + return "-" + } + switch { + case v >= 1e9: + return fmt.Sprintf("%.3f s", v/1e9) + case v >= 1e6: + return fmt.Sprintf("%.3f ms", v/1e6) + case v >= 1e3: + return fmt.Sprintf("%.3f µs", v/1e3) + default: + return fmt.Sprintf("%.3f ns", v) + } +}