From 0e6c1b682c7d7a181c595a9caf544577d36a8d93 Mon Sep 17 00:00:00 2001 From: rainman456 <81997753+rainman456@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:57:41 +0100 Subject: [PATCH 1/5] [bench]: Add hot-path benchmarks and an A/B benchmark driver The repo has no benchmarks, so performance claims have nothing to measure against. This adds benchmarks for every stage of the request path: namespace extraction and translation (protoutil), rule matching and the routing decision including the decision metric (router), and one full unary round trip through the gateway shape - hybrid codec, metrics interceptor, PeekInterceptor, Director, per-upstream proxy - against a no-proxy baseline over the same bufconn upstream. Whole-path allocations are the headline number: a proxied unary call allocates 400 times against 160 for a direct one. bench/run-benchmarks.sh drives A/B comparison of two revisions: it alternates base and candidate one round at a time, pins GOMAXPROCS, and reports benchstat plus pprof tops. The header it prints carries the commit SHAs, CPU model, and Go version, so a captured run is self-attesting. CI compiles the benchmarks but does not run them; mise test:bench runs them on demand. The test helpers dialBufconn, serve, prepared, newTestReporter, and newRelayWith take testing.TB now, so benchmarks reuse them instead of duplicating the bufconn harness. --- bench/run-benchmarks.sh | 137 ++++++++++++++++ internal/protoutil/extractor_bench_test.go | 74 +++++++++ internal/protoutil/translate_bench_test.go | 57 +++++++ internal/router/compile_test.go | 2 +- internal/router/director_bench_test.go | 57 +++++++ internal/router/handler_test.go | 12 +- internal/router/mux_bench_test.go | 60 +++++++ internal/router/relay_bench_test.go | 175 +++++++++++++++++++++ 8 files changed, 567 insertions(+), 7 deletions(-) create mode 100755 bench/run-benchmarks.sh create mode 100644 internal/protoutil/extractor_bench_test.go create mode 100644 internal/protoutil/translate_bench_test.go create mode 100644 internal/router/director_bench_test.go create mode 100644 internal/router/mux_bench_test.go create mode 100644 internal/router/relay_bench_test.go diff --git a/bench/run-benchmarks.sh b/bench/run-benchmarks.sh new file mode 100755 index 0000000..94144da --- /dev/null +++ b/bench/run-benchmarks.sh @@ -0,0 +1,137 @@ +#!/usr/bin/env bash +# A/B benchmark driver for the proxy hot path. +# +# Runs the committed Go benchmarks against two revisions, alternating base and +# candidate one round at a time (strict A/B/A/B, -count=1 per run) so noise +# spreads evenly across both sides, then compares the runs with benchstat. +# +# Usage: +# ./bench/run-benchmarks.sh [--base REF] [--cand REF] [--count N] +# [--benchtime D] [--bench REGEX] [--packages PKGS] [--profiles DIR] +# +# Defaults: base=origin/main, cand=HEAD, count=10, benchtime=1s, bench=".", +# packages="./internal/protoutil ./internal/router". +# +# The output header records the commit SHAs, CPU model, nproc, GOMAXPROCS, and +# Go version, so a captured run is self-attesting. For a quick single-revision +# sanity run, `mise run test:bench` does that without this script. + +set -euo pipefail + +BASE=origin/main +CAND=HEAD +COUNT=10 +BENCHTIME=1s +BENCH=. +PKGS="./internal/protoutil ./internal/router" +PROFILES="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --base) BASE="$2"; shift 2 ;; + --cand) CAND="$2"; shift 2 ;; + --count) COUNT="$2"; shift 2 ;; + --benchtime) BENCHTIME="$2"; shift 2 ;; + --bench) BENCH="$2"; shift 2 ;; + --packages) PKGS="$2"; shift 2 ;; + --profiles) PROFILES="$2"; shift 2 ;; + *) echo "unknown flag: $1" >&2; exit 2 ;; + esac +done + +command -v go >/dev/null 2>&1 || { echo "go is required (mise install go works too)" >&2; exit 1; } +git diff-index --quiet HEAD -- || { echo "working tree is dirty; commit or stash first" >&2; exit 1; } + +baseSHA="$(git rev-parse --short "${BASE}^{commit}")" +candSHA="$(git rev-parse --short "${CAND}^{commit}")" + +# GOMAXPROCS is fixed for the whole run: ns/op comparisons mean nothing if the +# parallelism changes between rounds. +export GOMAXPROCS="$(nproc)" + +{ + echo "# base: $BASE ($baseSHA)" + echo "# cand: $CAND ($candSHA)" + echo "# date: $(date -u +%Y-%m-%dT%H:%M:%SZ)" + echo "# kernel: $(uname -r)" + echo "# cpu: $(grep -m1 'model name' /proc/cpuinfo | cut -d: -f2 | sed 's/^ *//')" + echo "# nproc: $(nproc)" + echo "# GOMAXPROCS: $GOMAXPROCS" + echo "# go: $(go version)" + echo "# bench: $BENCH benchtime: $BENCHTIME rounds: $COUNT" + echo "# packages: $PKGS" +} | tee bench-header.txt + +WORK="$(mktemp -d)" +trap 'git worktree remove --force "$WORK/base" >/dev/null 2>&1 || true; git worktree remove --force "$WORK/cand" >/dev/null 2>&1 || true; rm -rf "$WORK"' EXIT + +git worktree add --detach "$WORK/base" "$BASE" >/dev/null +git worktree add --detach "$WORK/cand" "$CAND" >/dev/null + +# Warm both build caches before the first round, so round 1 does not pay the +# compile. +(cd "$WORK/base" && go build ./internal/...) >/dev/null +(cd "$WORK/cand" && go build ./internal/...) >/dev/null + +: > "$WORK/base.txt" +: > "$WORK/cand.txt" +for i in $(seq 1 "$COUNT"); do + echo "=== round $i/$COUNT: base ===" >&2 + # PKGS is intentionally unquoted: it is a space-separated package list. + (cd "$WORK/base" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/base.txt" + + echo "=== round $i/$COUNT: cand ===" >&2 + (cd "$WORK/cand" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/cand.txt" +done + +cp "$WORK/base.txt" "bench-$baseSHA.txt" +cp "$WORK/cand.txt" "bench-$candSHA.txt" + +# A side that ran no benchmarks (for example, a base revision without the +# benchmark files) yields an empty file and a meaningless benchstat table. +for side in base cand; do + if ! grep -q "^Benchmark" "$WORK/$side.txt"; then + echo "ERROR: no benchmarks ran on the $side side ($([[ $side == base ]] && echo "$BASE ($baseSHA)" || echo "$CAND ($candSHA)"))." >&2 + echo "Does that revision contain the benchmark files? Pick two revisions that both have them." >&2 + exit 1 + fi +done + +echo +echo "=== benchstat ($baseSHA vs $candSHA) ===" +go run golang.org/x/perf/cmd/benchstat@latest "$WORK/base.txt" "$WORK/cand.txt" | tee "benchstat-$baseSHA-vs-$candSHA.txt" + + if [[ -n "$PROFILES" ]]; then + mkdir -p "$PROFILES" + for side in base cand; do + if [[ $side == base ]]; then ref="$baseSHA"; else ref="$candSHA"; fi + dir="$PROFILES/$ref" + mkdir -p "$dir" + echo "=== profiles for $side ($ref) ===" >&2 + # One package per profile pass: go test -o only accepts a single + # package, and -o keeps the test binary next to the profiles, which is + # what pprof needs for symbolization. + # PKGS is intentionally unquoted: it is a space-separated package list. + for pkg in $PKGS; do + name="$(echo "$pkg" | tr '/' '-')" + (cd "$WORK/$side" && go test -run '^$' -bench "$BENCH" -count=1 -benchtime "$BENCHTIME" "$pkg" \ + -o "$dir/$name.test" \ + -cpuprofile "$dir/$name-cpu.out" \ + -memprofile "$dir/$name-heap.out" \ + -blockprofile "$dir/$name-block.out" \ + -mutexprofile "$dir/$name-mutex.out") >/dev/null + + go tool pprof -top -nodecount=25 "$dir/$name.test" "$dir/$name-cpu.out" | tee "$dir/$name-cpu-top.txt" + go tool pprof -top -nodecount=25 -sample_index=alloc_space "$dir/$name.test" "$dir/$name-heap.out" | tee "$dir/$name-heap-top.txt" + go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-block.out" | tee "$dir/$name-block-top.txt" + go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-mutex.out" | tee "$dir/$name-mutex-top.txt" + # Block and mutex profiling are sampling-disabled until enabled; the + # test binary enables them under -blockprofile/-mutexprofile, but + # rates stay small. Zero tops are normal for short runs. + done + done + echo "profiles written under $PROFILES" +fi + +echo +echo "artifacts: bench-header.txt bench-$baseSHA.txt bench-$candSHA.txt benchstat-$baseSHA-vs-$candSHA.txt ${PROFILES:+$PROFILES/}" diff --git a/internal/protoutil/extractor_bench_test.go b/internal/protoutil/extractor_bench_test.go new file mode 100644 index 0000000..cab970a --- /dev/null +++ b/internal/protoutil/extractor_bench_test.go @@ -0,0 +1,74 @@ +package protoutil_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "go.temporal.io/api/common/v1" + "go.temporal.io/api/taskqueue/v1" + "go.temporal.io/api/workflowservice/v1" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoregistry" + + "github.com/temporalio/temporal-proxy/internal/protoutil" +) + +// benchStartRequest builds a StartWorkflowExecutionRequest shaped like the ones +// SDK clients send: identity fields, a typed workflow, and a 1 KiB input +// payload, so extraction measures a realistic unmarshal rather than a +// two-field toy message. +func benchStartRequest() *workflowservice.StartWorkflowExecutionRequest { + input := make([]byte, 1024) + for i := range input { + input[i] = byte('a' + i%26) + } + + return &workflowservice.StartWorkflowExecutionRequest{ + Namespace: "payments-production", + WorkflowId: "billing-run-2026-10-10-abc123", + WorkflowType: &common.WorkflowType{Name: "billing.daily-run"}, + TaskQueue: &taskqueue.TaskQueue{Name: "billing-tq"}, + Input: &common.Payloads{Payloads: []*common.Payload{{ + Metadata: map[string][]byte{"encoding": []byte("json/plain")}, + Data: input, + }}}, + Identity: "1@worker-7", + } +} + +func benchMarshal(b *testing.B, m proto.Message) []byte { + b.Helper() + + payload, err := proto.Marshal(m) + require.NoError(b, err) + return payload +} + +func BenchmarkExtractor_NamespacedRequest(b *testing.B) { + payload := benchMarshal(b, benchStartRequest()) + ex := protoutil.NewExtractor(protoregistry.GlobalFiles, protoregistry.GlobalTypes) + require.Equal(b, "payments-production", ex.Namespace(startMethod, payload), "warm-up call must extract the namespace") + + b.ReportAllocs() + for b.Loop() { + if ns := ex.Namespace(startMethod, payload); ns == "" { + b.Fatal("namespace was not extracted") + } + } +} + +// BenchmarkExtractor_SystemRequest covers the request the SDK sends on every +// connect: no namespace accessor, so the full unmarshal runs and the cast +// misses. +func BenchmarkExtractor_SystemRequest(b *testing.B) { + payload := benchMarshal(b, &workflowservice.GetSystemInfoRequest{}) + ex := protoutil.NewExtractor(protoregistry.GlobalFiles, protoregistry.GlobalTypes) + require.Equal(b, "", ex.Namespace(systemMethod, payload)) + + b.ReportAllocs() + for b.Loop() { + if ns := ex.Namespace(systemMethod, payload); ns != "" { + b.Fatal("system request unexpectedly carried a namespace") + } + } +} diff --git a/internal/protoutil/translate_bench_test.go b/internal/protoutil/translate_bench_test.go new file mode 100644 index 0000000..240404d --- /dev/null +++ b/internal/protoutil/translate_bench_test.go @@ -0,0 +1,57 @@ +package protoutil_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "go.temporal.io/api/namespace/v1" + "go.temporal.io/api/workflowservice/v1" + "google.golang.org/protobuf/reflect/protoregistry" + + "github.com/temporalio/temporal-proxy/internal/protoutil" +) + +// Benchmarks for Translator.Translate on the two message shapes the proxy +// rewrites per request: a namespaced command (request direction) and a +// DescribeNamespace response, which reaches NamespaceInfo.name only through +// the override list. + +func BenchmarkTranslator_StartWorkflowRequest(b *testing.B) { + tr := protoutil.NewTranslator(protoregistry.GlobalFiles) + require.NoError(b, tr.WarmService(wfService)) + req := benchStartRequest() + + b.ReportAllocs() + for b.Loop() { + // The mapper returns a fixed name so repeated in-place rewrites stay + // stable and the benchmark measures the walk, not string growth. + tr.Translate(req, func(string) string { return "payments-production-remote" }) + } +} + +func BenchmarkTranslator_StartWorkflowRequestCold(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + // A fresh Translator per iteration pays plan construction on the first + // use, which is what an operator skipping WarmService pays per type. + tr := protoutil.NewTranslator(protoregistry.GlobalFiles) + tr.Translate(benchStartRequest(), func(string) string { return "remote" }) + } +} + +func BenchmarkTranslator_DescribeNamespaceResponse(b *testing.B) { + tr := protoutil.NewTranslator(protoregistry.GlobalFiles) + require.NoError(b, tr.WarmService(wfService)) + resp := &workflowservice.DescribeNamespaceResponse{ + NamespaceInfo: &namespace.NamespaceInfo{Name: "payments-production"}, + } + // WarmService stores a plan per top-level method input/output type, so the + // response type itself is what IsWarm can see; NamespaceInfo's plan lives + // inside the response plan's child graph. + require.True(b, tr.IsWarm(resp.ProtoReflect().Descriptor().FullName()), "plan for DescribeNamespaceResponse must be warm through WarmService") + + b.ReportAllocs() + for b.Loop() { + tr.Translate(resp, func(string) string { return "payments-production-local" }) + } +} diff --git a/internal/router/compile_test.go b/internal/router/compile_test.go index 42e04e9..a60d34e 100644 --- a/internal/router/compile_test.go +++ b/internal/router/compile_test.go @@ -112,7 +112,7 @@ func TestMuxFor(t *testing.T) { // prepared returns r after preparing it inside a minimal config with one // upstream per name r references. -func prepared(t *testing.T, r config.Routing) config.Routing { +func prepared(t testing.TB, r config.Routing) config.Routing { t.Helper() names := map[string]struct{}{"primary": {}} diff --git a/internal/router/director_bench_test.go b/internal/router/director_bench_test.go new file mode 100644 index 0000000..5b3284b --- /dev/null +++ b/internal/router/director_bench_test.go @@ -0,0 +1,57 @@ +package router_test + +import ( + "testing" + + "google.golang.org/grpc" + + "github.com/temporalio/temporal-proxy/internal/config" + "github.com/temporalio/temporal-proxy/internal/router" +) + +// BenchmarkDirector_Resolve measures the routing decision including the +// decision metric: the real MuxFor mux and a Prometheus-backed Reporter on a +// fresh registry, with the handlers map the dataplane would hold. The logger +// is nil so Debug entries take the same disabled path as production's default +// log level. +func BenchmarkDirector_Resolve(b *testing.B) { + mux := router.MuxFor(prepared(b, config.Routing{ + DefaultUpstream: "primary", + Rules: []config.RoutingRule{ + {Upstream: "prod", Match: config.RoutingMatch{Namespace: "prod-*"}}, + }, + })) + + rep, _ := newTestReporter(b, "primary", "prod") + handlers := map[string]grpc.StreamHandler{ + "primary": func(any, grpc.ServerStream) error { return nil }, + "prod": func(any, grpc.ServerStream) error { return nil }, + } + d := router.NewDirector(mux, handlers, rep, nil) + + b.Run("namespace-match", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + target, err := d.Resolve(b.Context(), "/svc/Method", "prod-eu-1", nil) + if err != nil { + b.Fatal(err) + } + if target.Upstream != "prod" { + b.Fatalf("routed to %q", target.Upstream) + } + } + }) + + b.Run("default", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + target, err := d.Resolve(b.Context(), "/svc/Method", "tenant-x", nil) + if err != nil { + b.Fatal(err) + } + if target.Upstream != "primary" { + b.Fatalf("routed to %q", target.Upstream) + } + } + }) +} diff --git a/internal/router/handler_test.go b/internal/router/handler_test.go index 6c34db6..9021fad 100644 --- a/internal/router/handler_test.go +++ b/internal/router/handler_test.go @@ -681,7 +681,7 @@ func capturingUpstream(t *testing.T) (*grpc.ClientConn, <-chan string) { return dialBufconn(t, lis), got } -func dialBufconn(t *testing.T, lis *bufconn.Listener) *grpc.ClientConn { +func dialBufconn(t testing.TB, lis *bufconn.Listener) *grpc.ClientConn { t.Helper() conn, err := grpc.NewClient( "passthrough:///bufnet", @@ -695,7 +695,7 @@ func dialBufconn(t *testing.T, lis *bufconn.Listener) *grpc.ClientConn { return conn } -func serve(t *testing.T, srv *grpc.Server, lis *bufconn.Listener) { +func serve(t testing.TB, srv *grpc.Server, lis *bufconn.Listener) { t.Helper() go func() { _ = srv.Serve(lis) }() t.Cleanup(srv.Stop) @@ -705,7 +705,7 @@ func serve(t *testing.T, srv *grpc.Server, lis *bufconn.Listener) { // production namespace and subsystem so emitted series match the // tmprl_proxy_router_* names, for tests that need to observe (or discard) what // the handler records. -func newTestReporter(t *testing.T, upstreams ...string) (*router.Reporter, *prometheus.Registry) { +func newTestReporter(t testing.TB, upstreams ...string) (*router.Reporter, *prometheus.Registry) { t.Helper() return newLabeledTestReporter(t, metrics.MetadataLabels{}, upstreams...) } @@ -713,7 +713,7 @@ func newTestReporter(t *testing.T, upstreams ...string) (*router.Reporter, *prom // newLabeledTestReporter is newTestReporter for the cases that care what // metadata labels do to the label set and to pre-resolution. func newLabeledTestReporter( - t *testing.T, + t testing.TB, labels metrics.MetadataLabels, upstreams ...string, ) (*router.Reporter, *prometheus.Registry) { @@ -726,7 +726,7 @@ func newLabeledTestReporter( // newRelayToUpstream stands up a fake upstream (configured by registerUpstream), // then a bare relay server that forwards all methods to it via router.Handler // using pass-through routing. Returns a client conn pointed at the relay. -func newRelayToUpstream(t *testing.T, registerUpstream func(*grpc.Server)) *grpc.ClientConn { +func newRelayToUpstream(t testing.TB, registerUpstream func(*grpc.Server)) *grpc.ClientConn { t.Helper() return newRelayWith( @@ -740,7 +740,7 @@ func newRelayToUpstream(t *testing.T, registerUpstream func(*grpc.Server)) *grpc // Reflector, and Reporter, so tests can observe how the handler routes and what // it records. makeDirector receives the connection to the fake upstream. func newRelayWith( - t *testing.T, + t testing.TB, registerUpstream func(*grpc.Server), makeDirector func(cc *grpc.ClientConn) router.Director, reflector router.Reflector, diff --git a/internal/router/mux_bench_test.go b/internal/router/mux_bench_test.go new file mode 100644 index 0000000..22f21f2 --- /dev/null +++ b/internal/router/mux_bench_test.go @@ -0,0 +1,60 @@ +package router_test + +import ( + "testing" + + "github.com/temporalio/temporal-proxy/internal/config" + "github.com/temporalio/temporal-proxy/internal/router" +) + +// BenchmarkMux_Switch measures rule evaluation for the shapes dev/config.yaml +// and the README describe: a namespace rule that matches, one that falls +// through, and a metadata-keyed rule. +func BenchmarkMux_Switch(b *testing.B) { + mux := router.MuxFor(prepared(b, config.Routing{ + DefaultUpstream: "primary", + SystemUpstream: "system", + Rules: []config.RoutingRule{ + {Upstream: "prod", Match: config.RoutingMatch{Namespace: "prod-*"}}, + {Upstream: "staging", Match: config.RoutingMatch{Namespace: "staging-*"}}, + {Upstream: "cluster-3", Match: config.RoutingMatch{Metadata: map[string]string{"x-cluster": "3"}}}, + }, + })) + + b.Run("namespace-match", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if up, _ := mux.Switch("prod-eu-1", nil); up != "prod" { + b.Fatalf("routed to %q", up) + } + } + }) + + b.Run("metadata-match", func(b *testing.B) { + md := map[string][]string{"x-cluster": {"3"}} + b.ReportAllocs() + for b.Loop() { + if up, _ := mux.Switch("other-ns", md); up != "cluster-3" { + b.Fatalf("routed to %q", up) + } + } + }) + + b.Run("default", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if up, _ := mux.Switch("tenant-x", nil); up != "primary" { + b.Fatalf("routed to %q", up) + } + } + }) + + b.Run("system", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if up, _ := mux.Switch("", nil); up != "system" { + b.Fatalf("routed to %q", up) + } + } + }) +} diff --git a/internal/router/relay_bench_test.go b/internal/router/relay_bench_test.go new file mode 100644 index 0000000..0b82856 --- /dev/null +++ b/internal/router/relay_bench_test.go @@ -0,0 +1,175 @@ +package router_test + +import ( + "context" + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/stretchr/testify/require" + "go.temporal.io/api/common/v1" + "go.temporal.io/api/taskqueue/v1" + "go.temporal.io/api/workflowservice/v1" + "google.golang.org/grpc" + "google.golang.org/grpc/test/bufconn" + "google.golang.org/protobuf/reflect/protoregistry" + + "github.com/temporalio/temporal-proxy/internal/config" + "github.com/temporalio/temporal-proxy/internal/metrics" + "github.com/temporalio/temporal-proxy/internal/protoutil" + "github.com/temporalio/temporal-proxy/internal/proxy" + "github.com/temporalio/temporal-proxy/internal/router" + "github.com/temporalio/temporal-proxy/internal/server" + "github.com/temporalio/temporal-proxy/internal/services" +) + +const ( + benchService = "temporal.api.workflowservice.v1.WorkflowService" + benchMethod = "/" + benchService + "/StartWorkflowExecution" + benchRunID = "0198f4a2-c0de-7f31-a5b2-9e11d1c2a3b4" + benchInputSize = 1024 +) + +// benchUpstreamServer returns a gRPC server that answers StartWorkflowExecution +// under the real service name, so the forwarder resolves the method against the +// production descriptor and type registries. The handler decodes the request and +// returns a run, which is the observable part of the response SDKs rely on. +func benchUpstreamServer() *grpc.Server { + desc := grpc.ServiceDesc{ + ServiceName: benchService, + HandlerType: (*any)(nil), + Methods: []grpc.MethodDesc{{ + MethodName: "StartWorkflowExecution", + Handler: func(_ any, _ context.Context, dec func(any) error, _ grpc.UnaryServerInterceptor) (any, error) { + req := new(workflowservice.StartWorkflowExecutionRequest) + if err := dec(req); err != nil { + return nil, err + } + + return &workflowservice.StartWorkflowExecutionResponse{RunId: benchRunID}, nil + }, + }}, + Streams: []grpc.StreamDesc{}, + } + + srv := grpc.NewServer() + srv.RegisterService(&desc, nil) + return srv +} + +// benchRequest builds the same request shape the extractor benchmark uses, so +// the per-stage and whole-path numbers describe one payload. +func benchRequest() *workflowservice.StartWorkflowExecutionRequest { + input := make([]byte, benchInputSize) + for i := range input { + input[i] = byte('a' + i%26) + } + + return &workflowservice.StartWorkflowExecutionRequest{ + Namespace: "payments-production", + WorkflowId: "billing-run-2026-10-10-abc123", + WorkflowType: &common.WorkflowType{Name: "billing.daily-run"}, + TaskQueue: &taskqueue.TaskQueue{Name: "billing-tq"}, + Input: &common.Payloads{Payloads: []*common.Payload{{ + Metadata: map[string][]byte{"encoding": []byte("json/plain")}, + Data: input, + }}}, + Identity: "1@worker-7", + } +} + +// benchInvoke runs one unary call and checks the response survived the round +// trip, so a codec or forwarder regression fails the benchmark instead of +// feeding it garbage. +func benchInvoke(b *testing.B, conn *grpc.ClientConn, req *workflowservice.StartWorkflowExecutionRequest) { + b.Helper() + + resp := new(workflowservice.StartWorkflowExecutionResponse) + if err := conn.Invoke(b.Context(), benchMethod, req, resp); err != nil { + b.Fatal(err) + } + if got := resp.GetRunId(); got != benchRunID { + b.Fatalf("response carried run id %q", got) + } +} + +// benchRelayConn stands up the production gateway shape: the hybrid codec, the +// server metrics interceptor, PeekInterceptor backed by the real extractor, and +// Handler backed by a real Director whose single upstream is a Forwarder over a +// bufconn connection to the fake upstream. Authentication (off by default) and +// panic recovery are not installed. +func benchRelayConn(b *testing.B) *grpc.ClientConn { + b.Helper() + + upstreamLis := bufconn.Listen(1024 * 1024) + upstream := benchUpstreamServer() + serve(b, upstream, upstreamLis) + upstreamConn := dialBufconn(b, upstreamLis) + b.Cleanup(func() { _ = upstreamConn.Close() }) + + fw, err := proxy.NewForwarder(upstreamConn) + require.NoError(b, err) + + mux := router.MuxFor(prepared(b, config.Routing{DefaultUpstream: "primary"})) + rep, _ := newTestReporter(b, "primary") + d := router.NewDirector(mux, map[string]grpc.StreamHandler{"primary": fw.Handle}, rep, nil) + + extractor := protoutil.NewExtractor(protoregistry.GlobalFiles, protoregistry.GlobalTypes) + allowlist := services.NewAllowlist(services.Default()) + + reg := prometheus.NewRegistry() + serverRep := server.NewReporter( + metrics.New("tmprl_proxy", promauto.With(reg)).ForSubsystem("server"), + metrics.MetadataLabels{}, + ) + + relayLis := bufconn.Listen(1024 * 1024) + relay := grpc.NewServer( + grpc.ForceServerCodecV2(router.Codec()), + grpc.ChainStreamInterceptor( + serverRep.StreamInterceptor(), + router.PeekInterceptor(extractor, allowlist), + ), + grpc.UnknownServiceHandler(router.Handler(d, allowlist)), + ) + serve(b, relay, relayLis) + + conn := dialBufconn(b, relayLis) + b.Cleanup(func() { _ = conn.Close() }) + return conn +} + +// BenchmarkUnaryThroughProxy measures one StartWorkflowExecution round trip as +// an SDK client observes it: client -> gateway (codec, metrics, peek, routing) +// -> forwarder (typed codec hop) -> upstream and back. Compare with +// BenchmarkUnaryDirect: the difference is what the proxy adds per unary call. +func BenchmarkUnaryThroughProxy(b *testing.B) { + conn := benchRelayConn(b) + req := benchRequest() + benchInvoke(b, conn, req) + + b.ReportAllocs() + for b.Loop() { + benchInvoke(b, conn, req) + } +} + +// BenchmarkUnaryDirect is the no-proxy baseline for BenchmarkUnaryThroughProxy: +// the same request to the same fake upstream over bufconn, with the standard +// proto codec, as an SDK client would speak to a Temporal Service directly. +func BenchmarkUnaryDirect(b *testing.B) { + upstreamLis := bufconn.Listen(1024 * 1024) + upstream := benchUpstreamServer() + serve(b, upstream, upstreamLis) + + conn := dialBufconn(b, upstreamLis) + b.Cleanup(func() { _ = conn.Close() }) + + req := benchRequest() + benchInvoke(b, conn, req) + + b.ReportAllocs() + for b.Loop() { + benchInvoke(b, conn, req) + } +} From 8fe39a02e0f9b135afa4f4ea7ecd72fe4aa170a8 Mon Sep 17 00:00:00 2001 From: rainman456 <81997753+rainman456@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:50:18 +0100 Subject: [PATCH 2/5] [bench]: Apply review feedback on the benchmark suite Share the StartWorkflowExecution bench request through internal/testutils so the extractor, translator, and relay benchmarks describe one payload. Fold the standalone mux benchmark into BenchmarkDirector_Resolve, which covers the same shapes through the real Resolve path, and drop the cold translator benchmark and the warm-service assertion the fx module already guards. --- internal/protoutil/extractor_bench_test.go | 28 +--------- internal/protoutil/translate_bench_test.go | 22 +------- internal/router/director_bench_test.go | 44 +++++++++++++--- internal/router/mux_bench_test.go | 60 ---------------------- internal/router/relay_bench_test.go | 35 +++---------- internal/testutils/request.go | 32 ++++++++++++ 6 files changed, 80 insertions(+), 141 deletions(-) delete mode 100644 internal/router/mux_bench_test.go create mode 100644 internal/testutils/request.go diff --git a/internal/protoutil/extractor_bench_test.go b/internal/protoutil/extractor_bench_test.go index cab970a..99b2ed7 100644 --- a/internal/protoutil/extractor_bench_test.go +++ b/internal/protoutil/extractor_bench_test.go @@ -4,38 +4,14 @@ import ( "testing" "github.com/stretchr/testify/require" - "go.temporal.io/api/common/v1" - "go.temporal.io/api/taskqueue/v1" "go.temporal.io/api/workflowservice/v1" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoregistry" "github.com/temporalio/temporal-proxy/internal/protoutil" + "github.com/temporalio/temporal-proxy/internal/testutils" ) -// benchStartRequest builds a StartWorkflowExecutionRequest shaped like the ones -// SDK clients send: identity fields, a typed workflow, and a 1 KiB input -// payload, so extraction measures a realistic unmarshal rather than a -// two-field toy message. -func benchStartRequest() *workflowservice.StartWorkflowExecutionRequest { - input := make([]byte, 1024) - for i := range input { - input[i] = byte('a' + i%26) - } - - return &workflowservice.StartWorkflowExecutionRequest{ - Namespace: "payments-production", - WorkflowId: "billing-run-2026-10-10-abc123", - WorkflowType: &common.WorkflowType{Name: "billing.daily-run"}, - TaskQueue: &taskqueue.TaskQueue{Name: "billing-tq"}, - Input: &common.Payloads{Payloads: []*common.Payload{{ - Metadata: map[string][]byte{"encoding": []byte("json/plain")}, - Data: input, - }}}, - Identity: "1@worker-7", - } -} - func benchMarshal(b *testing.B, m proto.Message) []byte { b.Helper() @@ -45,7 +21,7 @@ func benchMarshal(b *testing.B, m proto.Message) []byte { } func BenchmarkExtractor_NamespacedRequest(b *testing.B) { - payload := benchMarshal(b, benchStartRequest()) + payload := benchMarshal(b, testutils.StartWorkflowRequest()) ex := protoutil.NewExtractor(protoregistry.GlobalFiles, protoregistry.GlobalTypes) require.Equal(b, "payments-production", ex.Namespace(startMethod, payload), "warm-up call must extract the namespace") diff --git a/internal/protoutil/translate_bench_test.go b/internal/protoutil/translate_bench_test.go index 240404d..03577e4 100644 --- a/internal/protoutil/translate_bench_test.go +++ b/internal/protoutil/translate_bench_test.go @@ -9,17 +9,13 @@ import ( "google.golang.org/protobuf/reflect/protoregistry" "github.com/temporalio/temporal-proxy/internal/protoutil" + "github.com/temporalio/temporal-proxy/internal/testutils" ) -// Benchmarks for Translator.Translate on the two message shapes the proxy -// rewrites per request: a namespaced command (request direction) and a -// DescribeNamespace response, which reaches NamespaceInfo.name only through -// the override list. - func BenchmarkTranslator_StartWorkflowRequest(b *testing.B) { tr := protoutil.NewTranslator(protoregistry.GlobalFiles) require.NoError(b, tr.WarmService(wfService)) - req := benchStartRequest() + req := testutils.StartWorkflowRequest() b.ReportAllocs() for b.Loop() { @@ -29,26 +25,12 @@ func BenchmarkTranslator_StartWorkflowRequest(b *testing.B) { } } -func BenchmarkTranslator_StartWorkflowRequestCold(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - // A fresh Translator per iteration pays plan construction on the first - // use, which is what an operator skipping WarmService pays per type. - tr := protoutil.NewTranslator(protoregistry.GlobalFiles) - tr.Translate(benchStartRequest(), func(string) string { return "remote" }) - } -} - func BenchmarkTranslator_DescribeNamespaceResponse(b *testing.B) { tr := protoutil.NewTranslator(protoregistry.GlobalFiles) require.NoError(b, tr.WarmService(wfService)) resp := &workflowservice.DescribeNamespaceResponse{ NamespaceInfo: &namespace.NamespaceInfo{Name: "payments-production"}, } - // WarmService stores a plan per top-level method input/output type, so the - // response type itself is what IsWarm can see; NamespaceInfo's plan lives - // inside the response plan's child graph. - require.True(b, tr.IsWarm(resp.ProtoReflect().Descriptor().FullName()), "plan for DescribeNamespaceResponse must be warm through WarmService") b.ReportAllocs() for b.Loop() { diff --git a/internal/router/director_bench_test.go b/internal/router/director_bench_test.go index 5b3284b..f301276 100644 --- a/internal/router/director_bench_test.go +++ b/internal/router/director_bench_test.go @@ -11,21 +11,26 @@ import ( // BenchmarkDirector_Resolve measures the routing decision including the // decision metric: the real MuxFor mux and a Prometheus-backed Reporter on a -// fresh registry, with the handlers map the dataplane would hold. The logger -// is nil so Debug entries take the same disabled path as production's default -// log level. +// fresh registry, with the handlers map the dataplane would hold. The shapes +// cover a namespace rule that matches, a metadata-keyed rule, the system +// upstream, and the default fall-through. The logger is nil so Debug entries +// take the same disabled path as production's default log level. func BenchmarkDirector_Resolve(b *testing.B) { mux := router.MuxFor(prepared(b, config.Routing{ DefaultUpstream: "primary", + SystemUpstream: "system", Rules: []config.RoutingRule{ {Upstream: "prod", Match: config.RoutingMatch{Namespace: "prod-*"}}, + {Upstream: "cluster-3", Match: config.RoutingMatch{Metadata: map[string]string{"x-cluster": "3"}}}, }, })) - rep, _ := newTestReporter(b, "primary", "prod") + rep, _ := newTestReporter(b, "primary", "prod", "cluster-3", "system") handlers := map[string]grpc.StreamHandler{ - "primary": func(any, grpc.ServerStream) error { return nil }, - "prod": func(any, grpc.ServerStream) error { return nil }, + "primary": func(any, grpc.ServerStream) error { return nil }, + "prod": func(any, grpc.ServerStream) error { return nil }, + "cluster-3": func(any, grpc.ServerStream) error { return nil }, + "system": func(any, grpc.ServerStream) error { return nil }, } d := router.NewDirector(mux, handlers, rep, nil) @@ -42,6 +47,33 @@ func BenchmarkDirector_Resolve(b *testing.B) { } }) + b.Run("metadata-match", func(b *testing.B) { + md := map[string][]string{"x-cluster": {"3"}} + b.ReportAllocs() + for b.Loop() { + target, err := d.Resolve(b.Context(), "/svc/Method", "other-ns", md) + if err != nil { + b.Fatal(err) + } + if target.Upstream != "cluster-3" { + b.Fatalf("routed to %q", target.Upstream) + } + } + }) + + b.Run("system", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + target, err := d.Resolve(b.Context(), "/svc/Method", "", nil) + if err != nil { + b.Fatal(err) + } + if target.Upstream != "system" { + b.Fatalf("routed to %q", target.Upstream) + } + } + }) + b.Run("default", func(b *testing.B) { b.ReportAllocs() for b.Loop() { diff --git a/internal/router/mux_bench_test.go b/internal/router/mux_bench_test.go deleted file mode 100644 index 22f21f2..0000000 --- a/internal/router/mux_bench_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package router_test - -import ( - "testing" - - "github.com/temporalio/temporal-proxy/internal/config" - "github.com/temporalio/temporal-proxy/internal/router" -) - -// BenchmarkMux_Switch measures rule evaluation for the shapes dev/config.yaml -// and the README describe: a namespace rule that matches, one that falls -// through, and a metadata-keyed rule. -func BenchmarkMux_Switch(b *testing.B) { - mux := router.MuxFor(prepared(b, config.Routing{ - DefaultUpstream: "primary", - SystemUpstream: "system", - Rules: []config.RoutingRule{ - {Upstream: "prod", Match: config.RoutingMatch{Namespace: "prod-*"}}, - {Upstream: "staging", Match: config.RoutingMatch{Namespace: "staging-*"}}, - {Upstream: "cluster-3", Match: config.RoutingMatch{Metadata: map[string]string{"x-cluster": "3"}}}, - }, - })) - - b.Run("namespace-match", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - if up, _ := mux.Switch("prod-eu-1", nil); up != "prod" { - b.Fatalf("routed to %q", up) - } - } - }) - - b.Run("metadata-match", func(b *testing.B) { - md := map[string][]string{"x-cluster": {"3"}} - b.ReportAllocs() - for b.Loop() { - if up, _ := mux.Switch("other-ns", md); up != "cluster-3" { - b.Fatalf("routed to %q", up) - } - } - }) - - b.Run("default", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - if up, _ := mux.Switch("tenant-x", nil); up != "primary" { - b.Fatalf("routed to %q", up) - } - } - }) - - b.Run("system", func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - if up, _ := mux.Switch("", nil); up != "system" { - b.Fatalf("routed to %q", up) - } - } - }) -} diff --git a/internal/router/relay_bench_test.go b/internal/router/relay_bench_test.go index 0b82856..b69abcc 100644 --- a/internal/router/relay_bench_test.go +++ b/internal/router/relay_bench_test.go @@ -7,8 +7,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/stretchr/testify/require" - "go.temporal.io/api/common/v1" - "go.temporal.io/api/taskqueue/v1" "go.temporal.io/api/workflowservice/v1" "google.golang.org/grpc" "google.golang.org/grpc/test/bufconn" @@ -21,13 +19,13 @@ import ( "github.com/temporalio/temporal-proxy/internal/router" "github.com/temporalio/temporal-proxy/internal/server" "github.com/temporalio/temporal-proxy/internal/services" + "github.com/temporalio/temporal-proxy/internal/testutils" ) const ( - benchService = "temporal.api.workflowservice.v1.WorkflowService" - benchMethod = "/" + benchService + "/StartWorkflowExecution" - benchRunID = "0198f4a2-c0de-7f31-a5b2-9e11d1c2a3b4" - benchInputSize = 1024 + benchService = "temporal.api.workflowservice.v1.WorkflowService" + benchMethod = "/" + benchService + "/StartWorkflowExecution" + benchRunID = "0198f4a2-c0de-7f31-a5b2-9e11d1c2a3b4" ) // benchUpstreamServer returns a gRPC server that answers StartWorkflowExecution @@ -57,27 +55,6 @@ func benchUpstreamServer() *grpc.Server { return srv } -// benchRequest builds the same request shape the extractor benchmark uses, so -// the per-stage and whole-path numbers describe one payload. -func benchRequest() *workflowservice.StartWorkflowExecutionRequest { - input := make([]byte, benchInputSize) - for i := range input { - input[i] = byte('a' + i%26) - } - - return &workflowservice.StartWorkflowExecutionRequest{ - Namespace: "payments-production", - WorkflowId: "billing-run-2026-10-10-abc123", - WorkflowType: &common.WorkflowType{Name: "billing.daily-run"}, - TaskQueue: &taskqueue.TaskQueue{Name: "billing-tq"}, - Input: &common.Payloads{Payloads: []*common.Payload{{ - Metadata: map[string][]byte{"encoding": []byte("json/plain")}, - Data: input, - }}}, - Identity: "1@worker-7", - } -} - // benchInvoke runs one unary call and checks the response survived the round // trip, so a codec or forwarder regression fails the benchmark instead of // feeding it garbage. @@ -145,7 +122,7 @@ func benchRelayConn(b *testing.B) *grpc.ClientConn { // BenchmarkUnaryDirect: the difference is what the proxy adds per unary call. func BenchmarkUnaryThroughProxy(b *testing.B) { conn := benchRelayConn(b) - req := benchRequest() + req := testutils.StartWorkflowRequest() benchInvoke(b, conn, req) b.ReportAllocs() @@ -165,7 +142,7 @@ func BenchmarkUnaryDirect(b *testing.B) { conn := dialBufconn(b, upstreamLis) b.Cleanup(func() { _ = conn.Close() }) - req := benchRequest() + req := testutils.StartWorkflowRequest() benchInvoke(b, conn, req) b.ReportAllocs() diff --git a/internal/testutils/request.go b/internal/testutils/request.go new file mode 100644 index 0000000..13374d9 --- /dev/null +++ b/internal/testutils/request.go @@ -0,0 +1,32 @@ +// Package testutils holds helpers shared between the internal packages' test +// suites. +package testutils + +import ( + "go.temporal.io/api/common/v1" + "go.temporal.io/api/taskqueue/v1" + "go.temporal.io/api/workflowservice/v1" +) + +// StartWorkflowRequest builds a StartWorkflowExecutionRequest shaped like the +// ones SDK clients send: identity fields, a typed workflow and task queue, and +// a 1 KiB input payload, so extraction and translation measure a realistic +// unmarshal rather than a two-field toy message. +func StartWorkflowRequest() *workflowservice.StartWorkflowExecutionRequest { + input := make([]byte, 1024) + for i := range input { + input[i] = byte('a' + i%26) + } + + return &workflowservice.StartWorkflowExecutionRequest{ + Namespace: "payments-production", + WorkflowId: "billing-run-2026-10-10-abc123", + WorkflowType: &common.WorkflowType{Name: "billing.daily-run"}, + TaskQueue: &taskqueue.TaskQueue{Name: "billing-tq"}, + Input: &common.Payloads{Payloads: []*common.Payload{{ + Metadata: map[string][]byte{"encoding": []byte("json/plain")}, + Data: input, + }}}, + Identity: "1@worker-7", + } +} From 4d29631dbb225d7d4be54abc40f7a305d735e6d4 Mon Sep 17 00:00:00 2001 From: rainman456 <81997753+rainman456@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:50:19 +0100 Subject: [PATCH 3/5] [bench]: Portabilize the A/B driver and pin benchstat Use getconf and sysctl instead of nproc and /proc/cpuinfo so the header works on macOS, and rely on Go's default GOMAXPROCS like every other run. benchstat now comes from the mise tools section instead of go run @latest, and the artifacts the driver writes are gitignored. --- .gitignore | 4 ++ bench/run-benchmarks.sh | 144 +++++++++++++++++++++------------------- mise.lock | 5 ++ mise.toml | 1 + 4 files changed, 87 insertions(+), 67 deletions(-) diff --git a/.gitignore b/.gitignore index 1198189..63f180e 100644 --- a/.gitignore +++ b/.gitignore @@ -29,3 +29,7 @@ docs/ # build things ./proxy dist/ + +# bench artifacts +bench-*.txt +benchstat-*.txt diff --git a/bench/run-benchmarks.sh b/bench/run-benchmarks.sh index 94144da..c0187c1 100755 --- a/bench/run-benchmarks.sh +++ b/bench/run-benchmarks.sh @@ -12,8 +12,8 @@ # Defaults: base=origin/main, cand=HEAD, count=10, benchtime=1s, bench=".", # packages="./internal/protoutil ./internal/router". # -# The output header records the commit SHAs, CPU model, nproc, GOMAXPROCS, and -# Go version, so a captured run is self-attesting. For a quick single-revision +# The output header records the commit SHAs, CPU model, CPU count, and Go +# version, so a captured run is self-attesting. For a quick single-revision # sanity run, `mise run test:bench` does that without this script. set -euo pipefail @@ -27,39 +27,49 @@ PKGS="./internal/protoutil ./internal/router" PROFILES="" while [[ $# -gt 0 ]]; do - case "$1" in - --base) BASE="$2"; shift 2 ;; - --cand) CAND="$2"; shift 2 ;; - --count) COUNT="$2"; shift 2 ;; - --benchtime) BENCHTIME="$2"; shift 2 ;; - --bench) BENCH="$2"; shift 2 ;; - --packages) PKGS="$2"; shift 2 ;; - --profiles) PROFILES="$2"; shift 2 ;; - *) echo "unknown flag: $1" >&2; exit 2 ;; - esac + case "$1" in + --base) BASE="$2"; shift 2 ;; + --cand) CAND="$2"; shift 2 ;; + --count) COUNT="$2"; shift 2 ;; + --benchtime) BENCHTIME="$2"; shift 2 ;; + --bench) BENCH="$2"; shift 2 ;; + --packages) PKGS="$2"; shift 2 ;; + --profiles) PROFILES="$2"; shift 2 ;; + *) echo "unknown flag: $1" >&2; exit 2 ;; + esac done command -v go >/dev/null 2>&1 || { echo "go is required (mise install go works too)" >&2; exit 1; } +command -v benchstat >/dev/null 2>&1 || { echo "benchstat is required (mise install provides it)" >&2; exit 1; } git diff-index --quiet HEAD -- || { echo "working tree is dirty; commit or stash first" >&2; exit 1; } baseSHA="$(git rev-parse --short "${BASE}^{commit}")" candSHA="$(git rev-parse --short "${CAND}^{commit}")" -# GOMAXPROCS is fixed for the whole run: ns/op comparisons mean nothing if the -# parallelism changes between rounds. -export GOMAXPROCS="$(nproc)" +# Portable replacements for nproc and /proc/cpuinfo, neither of which macOS +# ships. +cpus() { + getconf _NPROCESSORS_ONLN 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 1 +} + +cpu_model() { + if [[ -r /proc/cpuinfo ]]; then + grep -m1 'model name' /proc/cpuinfo | cut -d: -f2- | sed 's/^ *//' + else + sysctl -n machdep.cpu.brand_string 2>/dev/null || echo unknown + fi +} { - echo "# base: $BASE ($baseSHA)" - echo "# cand: $CAND ($candSHA)" - echo "# date: $(date -u +%Y-%m-%dT%H:%M:%SZ)" - echo "# kernel: $(uname -r)" - echo "# cpu: $(grep -m1 'model name' /proc/cpuinfo | cut -d: -f2 | sed 's/^ *//')" - echo "# nproc: $(nproc)" - echo "# GOMAXPROCS: $GOMAXPROCS" - echo "# go: $(go version)" - echo "# bench: $BENCH benchtime: $BENCHTIME rounds: $COUNT" - echo "# packages: $PKGS" + echo "# base: $BASE ($baseSHA)" + echo "# cand: $CAND ($candSHA)" + echo "# date: $(date -u +%Y-%m-%dT%H:%M:%SZ)" + echo "# kernel: $(uname -r)" + echo "# cpu: $(cpu_model)" + echo "# cpus: $(cpus)" + echo "# go: $(go version)" + echo "# bench: $BENCH benchtime: $BENCHTIME rounds: $COUNT" + echo "# packages: $PKGS" } | tee bench-header.txt WORK="$(mktemp -d)" @@ -76,12 +86,12 @@ git worktree add --detach "$WORK/cand" "$CAND" >/dev/null : > "$WORK/base.txt" : > "$WORK/cand.txt" for i in $(seq 1 "$COUNT"); do - echo "=== round $i/$COUNT: base ===" >&2 - # PKGS is intentionally unquoted: it is a space-separated package list. - (cd "$WORK/base" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/base.txt" + echo "=== round $i/$COUNT: base ===" >&2 + # PKGS is intentionally unquoted: it is a space-separated package list. + (cd "$WORK/base" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/base.txt" - echo "=== round $i/$COUNT: cand ===" >&2 - (cd "$WORK/cand" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/cand.txt" + echo "=== round $i/$COUNT: cand ===" >&2 + (cd "$WORK/cand" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/cand.txt" done cp "$WORK/base.txt" "bench-$baseSHA.txt" @@ -90,47 +100,47 @@ cp "$WORK/cand.txt" "bench-$candSHA.txt" # A side that ran no benchmarks (for example, a base revision without the # benchmark files) yields an empty file and a meaningless benchstat table. for side in base cand; do - if ! grep -q "^Benchmark" "$WORK/$side.txt"; then - echo "ERROR: no benchmarks ran on the $side side ($([[ $side == base ]] && echo "$BASE ($baseSHA)" || echo "$CAND ($candSHA)"))." >&2 - echo "Does that revision contain the benchmark files? Pick two revisions that both have them." >&2 - exit 1 - fi + if ! grep -q "^Benchmark" "$WORK/$side.txt"; then + echo "ERROR: no benchmarks ran on the $side side ($([[ $side == base ]] && echo "$BASE ($baseSHA)" || echo "$CAND ($candSHA)"))." >&2 + echo "Does that revision contain the benchmark files? Pick two revisions that both have them." >&2 + exit 1 + fi done echo echo "=== benchstat ($baseSHA vs $candSHA) ===" -go run golang.org/x/perf/cmd/benchstat@latest "$WORK/base.txt" "$WORK/cand.txt" | tee "benchstat-$baseSHA-vs-$candSHA.txt" - - if [[ -n "$PROFILES" ]]; then - mkdir -p "$PROFILES" - for side in base cand; do - if [[ $side == base ]]; then ref="$baseSHA"; else ref="$candSHA"; fi - dir="$PROFILES/$ref" - mkdir -p "$dir" - echo "=== profiles for $side ($ref) ===" >&2 - # One package per profile pass: go test -o only accepts a single - # package, and -o keeps the test binary next to the profiles, which is - # what pprof needs for symbolization. - # PKGS is intentionally unquoted: it is a space-separated package list. - for pkg in $PKGS; do - name="$(echo "$pkg" | tr '/' '-')" - (cd "$WORK/$side" && go test -run '^$' -bench "$BENCH" -count=1 -benchtime "$BENCHTIME" "$pkg" \ - -o "$dir/$name.test" \ - -cpuprofile "$dir/$name-cpu.out" \ - -memprofile "$dir/$name-heap.out" \ - -blockprofile "$dir/$name-block.out" \ - -mutexprofile "$dir/$name-mutex.out") >/dev/null - - go tool pprof -top -nodecount=25 "$dir/$name.test" "$dir/$name-cpu.out" | tee "$dir/$name-cpu-top.txt" - go tool pprof -top -nodecount=25 -sample_index=alloc_space "$dir/$name.test" "$dir/$name-heap.out" | tee "$dir/$name-heap-top.txt" - go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-block.out" | tee "$dir/$name-block-top.txt" - go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-mutex.out" | tee "$dir/$name-mutex-top.txt" - # Block and mutex profiling are sampling-disabled until enabled; the - # test binary enables them under -blockprofile/-mutexprofile, but - # rates stay small. Zero tops are normal for short runs. - done - done - echo "profiles written under $PROFILES" +benchstat "$WORK/base.txt" "$WORK/cand.txt" | tee "benchstat-$baseSHA-vs-$candSHA.txt" + +if [[ -n "$PROFILES" ]]; then + mkdir -p "$PROFILES" + for side in base cand; do + if [[ $side == base ]]; then ref="$baseSHA"; else ref="$candSHA"; fi + dir="$PROFILES/$ref" + mkdir -p "$dir" + echo "=== profiles for $side ($ref) ===" >&2 + # One package per profile pass: go test -o only accepts a single + # package, and -o keeps the test binary next to the profiles, which is + # what pprof needs for symbolization. + # PKGS is intentionally unquoted: it is a space-separated package list. + for pkg in $PKGS; do + name="$(echo "$pkg" | tr '/' '-')" + (cd "$WORK/$side" && go test -run '^$' -bench "$BENCH" -count=1 -benchtime "$BENCHTIME" "$pkg" \ + -o "$dir/$name.test" \ + -cpuprofile "$dir/$name-cpu.out" \ + -memprofile "$dir/$name-heap.out" \ + -blockprofile "$dir/$name-block.out" \ + -mutexprofile "$dir/$name-mutex.out") >/dev/null + + go tool pprof -top -nodecount=25 "$dir/$name.test" "$dir/$name-cpu.out" | tee "$dir/$name-cpu-top.txt" + go tool pprof -top -nodecount=25 -sample_index=alloc_space "$dir/$name.test" "$dir/$name-heap.out" | tee "$dir/$name-heap-top.txt" + go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-block.out" | tee "$dir/$name-block-top.txt" + go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-mutex.out" | tee "$dir/$name-mutex-top.txt" + # Block and mutex profiling are sampling-disabled until enabled; the + # test binary enables them under -blockprofile/-mutexprofile, but + # rates stay small. Zero tops are normal for short runs. + done + done + echo "profiles written under $PROFILES" fi echo diff --git a/mise.lock b/mise.lock index 4ba6911..ebb29e6 100644 --- a/mise.lock +++ b/mise.lock @@ -162,6 +162,11 @@ url = "https://dl.google.com/go/go1.27.1.darwin-amd64.tar.gz" checksum = "sha256:a3911b5e0e1b1053f25ed0675f4c1c6aad1e2bfcf253df2b9be4caabd2edd95d" url = "https://dl.google.com/go/go1.27.1.windows-amd64.zip" +[[tools."go:golang.org/x/perf/cmd/benchstat"]] +version = "v0.0.0-20261009192801-be2c69fb417e" +backend = "go:golang.org/x/perf/cmd/benchstat" +specifiers = ["latest"] + [[tools.golangci-lint]] version = "2.13.2" backend = "aqua:golangci/golangci-lint" diff --git a/mise.toml b/mise.toml index 438a9b2..abbf22b 100644 --- a/mise.toml +++ b/mise.toml @@ -6,6 +6,7 @@ lockfile = true [tools] "github:mattn/goreman" = "latest" "github:temporalio/cli" = "latest" +"go:golang.org/x/perf/cmd/benchstat" = "latest" buf = "1.72" go = "1.27" golangci-lint = "2.13" From 3b3eda7f5120f803fd7637fe61f2a5681e69a2c9 Mon Sep 17 00:00:00 2001 From: rainman456 <81997753+rainman456@users.noreply.github.com> Date: Fri, 16 Oct 2026 22:49:26 +0100 Subject: [PATCH 4/5] [bench]: Replace the A/B driver script with a mise task bench/run-benchmarks.sh grew a profiling pass and a header block that duplicated what config-gated pprof routes and benchstat already do. The mise task keeps the useful core: alternate A/B rounds against a base revision in a worktree, then compare with the pinned benchstat. Task arguments cover count, benchtime, and packages; running without --base just benchmarks the working tree, replacing the old test:bench task. --- bench/run-benchmarks.sh | 147 ---------------------------------------- mise-tasks/test/bench | 38 +++++++++++ mise.toml | 9 --- 3 files changed, 38 insertions(+), 156 deletions(-) delete mode 100755 bench/run-benchmarks.sh create mode 100755 mise-tasks/test/bench diff --git a/bench/run-benchmarks.sh b/bench/run-benchmarks.sh deleted file mode 100755 index c0187c1..0000000 --- a/bench/run-benchmarks.sh +++ /dev/null @@ -1,147 +0,0 @@ -#!/usr/bin/env bash -# A/B benchmark driver for the proxy hot path. -# -# Runs the committed Go benchmarks against two revisions, alternating base and -# candidate one round at a time (strict A/B/A/B, -count=1 per run) so noise -# spreads evenly across both sides, then compares the runs with benchstat. -# -# Usage: -# ./bench/run-benchmarks.sh [--base REF] [--cand REF] [--count N] -# [--benchtime D] [--bench REGEX] [--packages PKGS] [--profiles DIR] -# -# Defaults: base=origin/main, cand=HEAD, count=10, benchtime=1s, bench=".", -# packages="./internal/protoutil ./internal/router". -# -# The output header records the commit SHAs, CPU model, CPU count, and Go -# version, so a captured run is self-attesting. For a quick single-revision -# sanity run, `mise run test:bench` does that without this script. - -set -euo pipefail - -BASE=origin/main -CAND=HEAD -COUNT=10 -BENCHTIME=1s -BENCH=. -PKGS="./internal/protoutil ./internal/router" -PROFILES="" - -while [[ $# -gt 0 ]]; do - case "$1" in - --base) BASE="$2"; shift 2 ;; - --cand) CAND="$2"; shift 2 ;; - --count) COUNT="$2"; shift 2 ;; - --benchtime) BENCHTIME="$2"; shift 2 ;; - --bench) BENCH="$2"; shift 2 ;; - --packages) PKGS="$2"; shift 2 ;; - --profiles) PROFILES="$2"; shift 2 ;; - *) echo "unknown flag: $1" >&2; exit 2 ;; - esac -done - -command -v go >/dev/null 2>&1 || { echo "go is required (mise install go works too)" >&2; exit 1; } -command -v benchstat >/dev/null 2>&1 || { echo "benchstat is required (mise install provides it)" >&2; exit 1; } -git diff-index --quiet HEAD -- || { echo "working tree is dirty; commit or stash first" >&2; exit 1; } - -baseSHA="$(git rev-parse --short "${BASE}^{commit}")" -candSHA="$(git rev-parse --short "${CAND}^{commit}")" - -# Portable replacements for nproc and /proc/cpuinfo, neither of which macOS -# ships. -cpus() { - getconf _NPROCESSORS_ONLN 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 1 -} - -cpu_model() { - if [[ -r /proc/cpuinfo ]]; then - grep -m1 'model name' /proc/cpuinfo | cut -d: -f2- | sed 's/^ *//' - else - sysctl -n machdep.cpu.brand_string 2>/dev/null || echo unknown - fi -} - -{ - echo "# base: $BASE ($baseSHA)" - echo "# cand: $CAND ($candSHA)" - echo "# date: $(date -u +%Y-%m-%dT%H:%M:%SZ)" - echo "# kernel: $(uname -r)" - echo "# cpu: $(cpu_model)" - echo "# cpus: $(cpus)" - echo "# go: $(go version)" - echo "# bench: $BENCH benchtime: $BENCHTIME rounds: $COUNT" - echo "# packages: $PKGS" -} | tee bench-header.txt - -WORK="$(mktemp -d)" -trap 'git worktree remove --force "$WORK/base" >/dev/null 2>&1 || true; git worktree remove --force "$WORK/cand" >/dev/null 2>&1 || true; rm -rf "$WORK"' EXIT - -git worktree add --detach "$WORK/base" "$BASE" >/dev/null -git worktree add --detach "$WORK/cand" "$CAND" >/dev/null - -# Warm both build caches before the first round, so round 1 does not pay the -# compile. -(cd "$WORK/base" && go build ./internal/...) >/dev/null -(cd "$WORK/cand" && go build ./internal/...) >/dev/null - -: > "$WORK/base.txt" -: > "$WORK/cand.txt" -for i in $(seq 1 "$COUNT"); do - echo "=== round $i/$COUNT: base ===" >&2 - # PKGS is intentionally unquoted: it is a space-separated package list. - (cd "$WORK/base" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/base.txt" - - echo "=== round $i/$COUNT: cand ===" >&2 - (cd "$WORK/cand" && go test -run '^$' -bench "$BENCH" -benchmem -count=1 -benchtime "$BENCHTIME" $PKGS) | tee -a "$WORK/cand.txt" -done - -cp "$WORK/base.txt" "bench-$baseSHA.txt" -cp "$WORK/cand.txt" "bench-$candSHA.txt" - -# A side that ran no benchmarks (for example, a base revision without the -# benchmark files) yields an empty file and a meaningless benchstat table. -for side in base cand; do - if ! grep -q "^Benchmark" "$WORK/$side.txt"; then - echo "ERROR: no benchmarks ran on the $side side ($([[ $side == base ]] && echo "$BASE ($baseSHA)" || echo "$CAND ($candSHA)"))." >&2 - echo "Does that revision contain the benchmark files? Pick two revisions that both have them." >&2 - exit 1 - fi -done - -echo -echo "=== benchstat ($baseSHA vs $candSHA) ===" -benchstat "$WORK/base.txt" "$WORK/cand.txt" | tee "benchstat-$baseSHA-vs-$candSHA.txt" - -if [[ -n "$PROFILES" ]]; then - mkdir -p "$PROFILES" - for side in base cand; do - if [[ $side == base ]]; then ref="$baseSHA"; else ref="$candSHA"; fi - dir="$PROFILES/$ref" - mkdir -p "$dir" - echo "=== profiles for $side ($ref) ===" >&2 - # One package per profile pass: go test -o only accepts a single - # package, and -o keeps the test binary next to the profiles, which is - # what pprof needs for symbolization. - # PKGS is intentionally unquoted: it is a space-separated package list. - for pkg in $PKGS; do - name="$(echo "$pkg" | tr '/' '-')" - (cd "$WORK/$side" && go test -run '^$' -bench "$BENCH" -count=1 -benchtime "$BENCHTIME" "$pkg" \ - -o "$dir/$name.test" \ - -cpuprofile "$dir/$name-cpu.out" \ - -memprofile "$dir/$name-heap.out" \ - -blockprofile "$dir/$name-block.out" \ - -mutexprofile "$dir/$name-mutex.out") >/dev/null - - go tool pprof -top -nodecount=25 "$dir/$name.test" "$dir/$name-cpu.out" | tee "$dir/$name-cpu-top.txt" - go tool pprof -top -nodecount=25 -sample_index=alloc_space "$dir/$name.test" "$dir/$name-heap.out" | tee "$dir/$name-heap-top.txt" - go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-block.out" | tee "$dir/$name-block-top.txt" - go tool pprof -top -nodecount=15 "$dir/$name.test" "$dir/$name-mutex.out" | tee "$dir/$name-mutex-top.txt" - # Block and mutex profiling are sampling-disabled until enabled; the - # test binary enables them under -blockprofile/-mutexprofile, but - # rates stay small. Zero tops are normal for short runs. - done - done - echo "profiles written under $PROFILES" -fi - -echo -echo "artifacts: bench-header.txt bench-$baseSHA.txt bench-$candSHA.txt benchstat-$baseSHA-vs-$candSHA.txt ${PROFILES:+$PROFILES/}" diff --git a/mise-tasks/test/bench b/mise-tasks/test/bench new file mode 100755 index 0000000..16b173c --- /dev/null +++ b/mise-tasks/test/bench @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +#MISE description="Run benchmarks, optionally comparing against another revision" +#USAGE flag "-b --base " help="Compare the working tree against this revision with benchstat" +#USAGE flag "-c --count " default="10" +#USAGE flag "-t --time