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 <marvin.drees@9elements.com>

.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='<!-- lneto-bench -->'
+
+          # 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

.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: Generate benchmark report
-      # - name: Report benchmark regressions
+        run: |
-      #   uses: benchmark-action/github-action-benchmark@a887eba2af2000fda74e733fcf952aa823b65916 # v1.21.0
+          go run ./internal/benchci -current bench-results.txt -out bench-report.md
-      #   with:
+          cat bench-report.md >> "$GITHUB_STEP_SUMMARY"
-      #     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: 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:

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 = "<!-- lneto-bench -->"
+
+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)
+	}
+}