From 0c4f20d0f243af740aeab01e92e369a6158c4dd3 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 21:25:26 +0530 Subject: [PATCH 1/5] ci(perf): GOMEMLIMIT with GOGC=off, pinned toolchain, cachegrind I1, matrix mode [W0-15] Perf-gate methodology fixes from checkpoint C0: - Every GOGC=off measurement also sets GOMEMLIMIT (default 64MiB, shared in scripts/perf/lib.sh; PERF_GOMEMLIMIT / the gomemlimit dispatch input override it): ab.sh primary arms and instruction counts, profile.sh, and the competitor harness (cmd/publish -gomemlimit, recorded in configs[].gomemlimit). Default-GC arms unset it. - perf-gate.yml and bench-compare.yml build with one pinned Go release, PERF_GO_VERSION 1.26.9, check-latest off, GOTOOLCHAIN=local. - Profile mode runs cachegrind cache simulation (32 KiB 8-way L1i) of the gap queries and lists the functions with the most L1i misses. - New matrix dispatch mode: interleaved A/B over the full benchmark matrix generated by scripts/perf/matrix-suite.sh. - Every artifact records the runner CPU model; the gate report names it and states that only in-job ratios are compared; the competitor harness summary leads with GoSQLX/peer time ratios (-ratios). - setupTestServer in pkg/lsp discards output so benchmarks that publish diagnostics stop timing a growing buffer (W0-06 follow-up). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- .github/workflows/bench-compare.yml | 44 ++- .github/workflows/perf-gate.yml | 83 ++++-- CHANGELOG.md | 28 ++ Taskfile.yml | 2 +- benchmarks/compare/README.md | 10 +- benchmarks/compare/SCHEMA.md | 4 +- benchmarks/compare/cmd/publish/main.go | 180 ++++++++++-- benchmarks/compare/cmd/publish/main_test.go | 71 ++++- pkg/lsp/handler_bench_test.go | 21 +- scripts/perf/ab.sh | 21 +- scripts/perf/gate/config_test.go | 287 ++++++++++++++++++++ scripts/perf/gate/gate.go | 18 ++ scripts/perf/gate/gate_test.go | 24 ++ scripts/perf/lib.sh | 77 ++++++ scripts/perf/matrix-suite.sh | 17 ++ scripts/perf/profile.sh | 86 +++++- 16 files changed, 889 insertions(+), 84 deletions(-) create mode 100644 scripts/perf/gate/config_test.go create mode 100644 scripts/perf/lib.sh create mode 100755 scripts/perf/matrix-suite.sh diff --git a/.github/workflows/bench-compare.yml b/.github/workflows/bench-compare.yml index 19beeae01..9b0c46b6c 100644 --- a/.github/workflows/bench-compare.yml +++ b/.github/workflows/bench-compare.yml @@ -8,9 +8,15 @@ name: Competitor Benchmarks # one-pass benchmark smoke run. # - workflow_dispatch and release tags: a full -count run that publishes the # results JSON, the raw benchmark output and a benchstat summary as an -# artifact. Shared runners are noisy; the JSON records the CPU so a run is -# never presented without its hardware. Commit the artifact to +# artifact. Shared runners are noisy and differ in CPU model from job to +# job; the JSON and env.txt record the CPU so a run is never presented +# without its hardware, and the job summary compares GoSQLX with each peer +# by in-job time ratios rather than absolute ns/op. Commit the artifact to # benchmarks/compare/results/ (and website/src/data/) in a PR to publish it. +# +# Both jobs build with PERF_GO_VERSION, the release perf-gate.yml also pins +# (scripts/perf/gate/config_test.go keeps them equal); GOGC=off runs set +# GOMEMLIMIT (cmd/publish -gomemlimit, default as in scripts/perf/lib.sh). on: pull_request: @@ -58,6 +64,8 @@ defaults: env: CGO_ENABLED: '1' GOTOOLCHAIN: local + # One Go release for every perf measurement; perf-gate.yml pins the same. + PERF_GO_VERSION: '1.26.9' jobs: test: @@ -70,8 +78,8 @@ jobs: - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: - go-version-file: benchmarks/compare/go.mod - check-latest: true + go-version: ${{ env.PERF_GO_VERSION }} + check-latest: false cache-dependency-path: benchmarks/compare/go.sum - name: Verify go.mod is tidy @@ -102,8 +110,8 @@ jobs: - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: - go-version-file: benchmarks/compare/go.mod - check-latest: true + go-version: ${{ env.PERF_GO_VERSION }} + check-latest: false cache-dependency-path: benchmarks/compare/go.sum - name: Run harness @@ -118,27 +126,43 @@ jobs: *[!0-9a-z.]*|'') echo "benchtime must look like 200ms or 100x, got '$BENCHTIME'" >&2; exit 1 ;; esac mkdir -p "$RUNNER_TEMP/bench" + { + echo "cpu model: $(. ../../scripts/perf/lib.sh && perf_cpu_model)" + echo "go: $(go version)" + echo "kernel: $(uname -srm)" + echo "commit: $(git rev-parse HEAD)" + echo + lscpu + } > "$RUNNER_TEMP/bench/env.txt" # The matrix: -cpu 4 and -cpu 1, each with GOGC at its default and - # off, so mutator CPU can be told apart from GC. The first entry - # (cpu4, GOGC default) feeds the schema v1 fields of the JSON. + # off (with GOMEMLIMIT), so mutator CPU can be told apart from GC. + # The first entry (cpu4, GOGC default) feeds the schema v1 fields. go run ./cmd/publish -count="$COUNT" -benchtime="$BENCHTIME" \ -cpus=4,1 -gogc=default,off \ -out "$RUNNER_TEMP/bench/latest.json" \ - -raw "$RUNNER_TEMP/bench/latest.txt" + -raw "$RUNNER_TEMP/bench/latest.txt" \ + -ratios "$RUNNER_TEMP/bench/ratios.md" - name: benchstat summary run: | { echo '## Competitor benchmarks' + echo + cat "$RUNNER_TEMP/bench/ratios.md" + echo + echo '### Absolute numbers (this job and this CPU only)' for f in "$RUNNER_TEMP"/bench/latest*.txt; do name=$(basename "$f" .txt) go run golang.org/x/perf/cmd/benchstat@v0.0.0-20260908200009-22c9c6c9d4da \ "$f" > "$RUNNER_TEMP/bench/benchstat-$name.txt" echo - echo "### $name" + echo "
$name" + echo echo '```' cat "$RUNNER_TEMP/bench/benchstat-$name.txt" echo '```' + echo + echo '
' done } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/perf-gate.yml b/.github/workflows/perf-gate.yml index 233652e5a..20625ddca 100644 --- a/.github/workflows/perf-gate.yml +++ b/.github/workflows/perf-gate.yml @@ -10,9 +10,20 @@ name: Perf Gate # scripts/perf/gate judges the result; the report goes to the job summary, # a sticky comment (same-repo PRs only) and an artifact. # - Nightly on main: the same gate between main now and main 24 hours ago. -# - workflow_dispatch: mode "gate" with explicit base/head revisions, or mode -# "profile": pprof (-cpu 1, GOGC=off, -cpu 4) of the gap queries and -# perf stat counters for one revision (input to W0-14). +# - workflow_dispatch: mode "gate" with explicit base/head revisions; mode +# "matrix": the same interleaved A/B over the full benchmark matrix +# (scripts/perf/matrix-suite.sh, about 900 benchmarks; default 5 rounds of +# 100ms, roughly an hour; base = head gives an A/A of the whole matrix); or +# mode "profile": pprof (-cpu 1, GOGC=off, -cpu 4) of the gap queries, perf +# stat counters where the PMU allows, and cachegrind cache simulation (L1i +# included) for one revision (input to W0-14). +# +# Methodology (W0-15): one pinned Go release (PERF_GO_VERSION, shared with +# bench-compare.yml; GOTOOLCHAIN=local stops go.mod from switching it); every +# GOGC=off run also sets GOMEMLIMIT (scripts/perf/lib.sh, input "gomemlimit"); +# every artifact and report names the runner CPU model, and reports compare +# base and head of the same job by ratio only, because hosted runners differ +# in CPU model from job to job. # # SHADOW MODE: the gate reports and never fails a PR. After the shadow period, # set PERF_GATE_ENFORCE to 'true' below; a regression then fails the job @@ -34,9 +45,9 @@ on: workflow_dispatch: inputs: mode: - description: 'gate: A/B of base vs head; profile: CPU profiles and perf counters of head' + description: 'gate: A/B of base vs head over the gated suite; matrix: the same A/B over the full benchmark matrix; profile: CPU profiles, perf counters and cachegrind of head' type: choice - options: [gate, profile] + options: [gate, matrix, profile] default: gate base: description: 'gate mode: base revision (default origin/main)' @@ -47,11 +58,15 @@ on: required: false default: '' rounds: - description: 'gate mode: interleaved rounds' + description: 'gate/matrix mode: interleaved rounds (default 10; matrix 5)' required: false - default: '10' + default: '' benchtime: - description: 'gate mode: -test.benchtime per run; profile mode: per profile' + description: 'gate/matrix mode: -test.benchtime per run (default 200ms; matrix 100ms); profile mode: per profile (default 5s)' + required: false + default: '' + gomemlimit: + description: 'GOMEMLIMIT of the GOGC=off runs (default: scripts/perf/lib.sh; "off" leaves it unset)' required: false default: '' @@ -59,19 +74,24 @@ permissions: contents: read concurrency: - group: perf-gate-${{ github.event.pull_request.number || github.ref }}-${{ inputs.mode || 'gate' }} + group: perf-gate-${{ github.event.pull_request.number || github.ref }}-${{ inputs.mode || 'gate' }}-${{ inputs.gomemlimit }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} env: GOTOOLCHAIN: local + # The one Go release every perf measurement uses; bench-compare.yml pins + # the same (scripts/perf/gate/config_test.go keeps them equal). 1.26.9 is + # the toolchain of the recorded C0 baselines. + PERF_GO_VERSION: '1.26.9' + PERF_GOMEMLIMIT: ${{ inputs.gomemlimit }} PERF_GATE_ENFORCE: 'false' jobs: gate: name: A/B perf gate (shadow) - if: github.event_name != 'workflow_dispatch' || inputs.mode == 'gate' + if: github.event_name != 'workflow_dispatch' || inputs.mode != 'profile' runs-on: ubuntu-latest - timeout-minutes: 30 + timeout-minutes: ${{ inputs.mode == 'matrix' && 240 || 30 }} steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: @@ -82,8 +102,8 @@ jobs: - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: - go-version-file: go.mod - check-latest: true + go-version: ${{ env.PERF_GO_VERSION }} + check-latest: false cache-dependency-path: go.sum - name: Install instruction counters (best effort) @@ -121,20 +141,30 @@ jobs: - name: Run perf gate id: gate - continue-on-error: ${{ env.PERF_GATE_ENFORCE != 'true' }} - timeout-minutes: 24 + continue-on-error: ${{ env.PERF_GATE_ENFORCE != 'true' || inputs.mode == 'matrix' }} + timeout-minutes: ${{ inputs.mode == 'matrix' && 230 || 24 }} env: BASE: ${{ steps.refs.outputs.base }} - ROUNDS: ${{ inputs.rounds || '10' }} - BENCHTIME: ${{ inputs.benchtime || '200ms' }} + MODE: ${{ inputs.mode || 'gate' }} + ROUNDS: ${{ inputs.rounds }} + BENCHTIME: ${{ inputs.benchtime }} ACCEPTED: ${{ contains(github.event.pull_request.labels.*.name, 'perf-regression-accepted') }} run: | set -o pipefail - flags=(-G -r "$ROUNDS" -t "$BENCHTIME" -o "$RUNNER_TEMP/perf-gate") - [ "$PERF_GATE_ENFORCE" = true ] && flags+=(-e) - [ "$ACCEPTED" = true ] && flags+=(-A) mkdir -p "$RUNNER_TEMP/perf-gate" - scripts/perf/ab.sh "${flags[@]}" "$BASE" 2>&1 | tee "$RUNNER_TEMP/perf-gate.log" + if [ "$MODE" = matrix ]; then + # The full matrix: base vs head interleaved like the gate, without + # the A/A arm and instruction counts (too slow at this size). It + # reports and never fails. + scripts/perf/matrix-suite.sh > "$RUNNER_TEMP/matrix-suite.txt" + cp "$RUNNER_TEMP/matrix-suite.txt" "$RUNNER_TEMP/perf-gate/" + flags=(-r "${ROUNDS:-5}" -t "${BENCHTIME:-100ms}" -s "$RUNNER_TEMP/matrix-suite.txt") + else + flags=(-G -r "${ROUNDS:-10}" -t "${BENCHTIME:-200ms}") + [ "$PERF_GATE_ENFORCE" = true ] && flags+=(-e) + [ "$ACCEPTED" = true ] && flags+=(-A) + fi + scripts/perf/ab.sh "${flags[@]}" -o "$RUNNER_TEMP/perf-gate" "$BASE" 2>&1 | tee "$RUNNER_TEMP/perf-gate.log" - name: Job summary if: always() @@ -148,6 +178,8 @@ jobs: echo '' echo '## Perf gate: ERROR (shadow mode: report only)' echo + echo "Runner CPU: \`$(. scripts/perf/lib.sh && perf_cpu_model)\`." + echo echo "The gate did not produce a report; see the \"Run perf gate\" step log of ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}. Last lines:" echo echo '```' @@ -226,14 +258,17 @@ jobs: - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: - go-version-file: go.mod - check-latest: true + go-version: ${{ env.PERF_GO_VERSION }} + check-latest: false cache-dependency-path: go.sum - - name: Install perf (best effort) + - name: Install perf and valgrind (best effort) continue-on-error: true run: | sudo apt-get update -qq + # valgrind first: cachegrind's cache simulation is the only L1i data + # on hosted runners, whose virtualised PMU usually lacks perf events. + sudo apt-get install -y -qq --no-install-recommends valgrind >/dev/null sudo apt-get install -y -qq --no-install-recommends linux-tools-common "linux-tools-$(uname -r)" >/dev/null sudo sysctl -w kernel.perf_event_paranoid=-1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 84e85635b..056403f35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **Perf measurement methodology** (W0-15; benchmark tooling only, no library + behaviour change): + - Every `GOGC=off` measurement now also sets `GOMEMLIMIT` (default `64MiB`; + override with `PERF_GOMEMLIMIT` or the `gomemlimit` dispatch input, `off` + unsets it): `scripts/perf/ab.sh` (so `task bench:ab` and `task bench:gate`), + `scripts/perf/profile.sh` and the competitor harness's `cmd/publish` + (`-gomemlimit`, recorded as `configs[].gomemlimit` in the results JSON). + Without a limit a GC-off heap only grows, so allocations land on fresh pages + whose first-touch faults were timed and profiled; the first benchmark of + each test binary absorbed most of them. GOGC=off timings are therefore not + comparable with runs made before this change; base and head of one run + always use the same setting. + - `perf-gate.yml` and `bench-compare.yml` build with one pinned Go release, + `PERF_GO_VERSION` 1.26.9 (the toolchain of the recorded baselines); the + gate used to take 1.26.1 from `go.mod`. + - `perf-gate.yml` profile mode adds cachegrind cache simulation (32 KiB + 8-way L1i, the Zen 3/4/5 geometry) of the gap queries: instructions, L1i + and L1d misses per operation and the functions with the most L1i misses. + - New `matrix` dispatch mode in `perf-gate.yml`: the interleaved A/B over the + full benchmark matrix (`scripts/perf/matrix-suite.sh`), report only. + - Every perf artifact and report names the runner CPU model, and the reports + compare by in-job ratio only: the gate report says so, and the competitor + harness summary leads with GoSQLX/peer time ratios (`cmd/publish -ratios`) + instead of absolute ns/op. + - The LSP benchmark server (`setupTestServer`) discards its output, so + benchmarks that publish diagnostics no longer time a growing buffer. + ### Fixed - **AST**: `AlterTableStatement.SQL()` now separates multiple ALTER TABLE actions with commas, matching the compact formatter and preserving action boundaries when re-parsed. - **Playground**: the GoSQLX WebAssembly module now runs in a Web Worker, so diff --git a/Taskfile.yml b/Taskfile.yml index 79735d1f3..640160332 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -185,7 +185,7 @@ tasks: - go run golang.org/x/perf/cmd/benchstat@v0.0.0-20260908200009-22c9c6c9d4da results/latest.txt bench:ab: - desc: "Interleaved A/B of the working tree against BASE (GOGC=off, -cpu 1, ABBA, benchstat alpha 0.01). Vars: BASE (default origin/main), ROUNDS (10), BENCHTIME (200ms), BENCH='./pkg/gosqlx=^BenchmarkParseE2E$' (default: the gated suite)" + desc: "Interleaved A/B of the working tree against BASE (GOGC=off with GOMEMLIMIT, -cpu 1, ABBA, benchstat alpha 0.01). Vars: BASE (default origin/main), ROUNDS (10), BENCHTIME (200ms), BENCH='./pkg/gosqlx=^BenchmarkParseE2E$' (default: the gated suite)" cmds: - scripts/perf/ab.sh -r {{.ROUNDS}} -t {{.BENCHTIME}} {{if .BENCH}}-b '{{.BENCH}}'{{end}} {{.BASE}} vars: diff --git a/benchmarks/compare/README.md b/benchmarks/compare/README.md index 4d4ac5e97..c559aaaa6 100644 --- a/benchmarks/compare/README.md +++ b/benchmarks/compare/README.md @@ -16,7 +16,9 @@ this directory. raw output is kept in `results/latest.txt` so you can re-summarise it with `benchstat`. - **Run matrix:** `-cpu 4` and `-cpu 1`, each with GOGC at its default and with - `GOGC=off`, to separate mutator CPU from GC cost. The first configuration + `GOGC=off`, to separate mutator CPU from GC cost. `GOGC=off` runs also set + `GOMEMLIMIT=64MiB` (`-gomemlimit`, same default as `scripts/perf/lib.sh`), so + the heap reuses pages instead of timing first-touch page faults. The first configuration (`cpu4-gogc-default`) feeds the v1 fields; the others are in `cells` of the v2 document and in `results/latest-.txt`. Compare configurations of one run, not absolute numbers across machines. @@ -160,4 +162,8 @@ benchmark on every change to this directory. Running it manually (`workflow_dispatch`) or pushing a release tag performs a full run and uploads the JSON, the raw output of every configuration and a benchstat summary per configuration as an artifact. The smoke run covers `-cpu 1,4,10`; the publish job -runs the full `-cpu 4,1` x `GOGC=default,off` matrix. +runs the full `-cpu 4,1` x `GOGC=default,off` matrix. Both jobs build with the Go +release pinned in `PERF_GO_VERSION` (the perf gate pins the same one). The job +summary leads with GoSQLX/peer time ratios (`-ratios`) and names the runner CPU, +which is also in the artifact's `env.txt`: hosted runners differ in CPU model from +job to job, so compare ratios across runs, never absolute ns/op. diff --git a/benchmarks/compare/SCHEMA.md b/benchmarks/compare/SCHEMA.md index 099003ba7..1484ab5b4 100644 --- a/benchmarks/compare/SCHEMA.md +++ b/benchmarks/compare/SCHEMA.md @@ -13,7 +13,7 @@ v1 can ignore everything under "v2 additions". | `generatedAt`, `commit`, `dirty`, `goVersion` | provenance | | `hardware` | `{cpu, goos, goarch, numCPU}` of the machine that ran it | | `stage` | always `"e2e"` for the v1 view | -| `method` | `{command, count, statistic, rawOutput}` of the **first** configuration. `command` is a quoted, copy-pasteable `go test` line (run it in `benchmarks/compare`) that reproduces `results[]`: `BenchmarkParse` only, with a `GOGC=off ` prefix only when that configuration sets it. | +| `method` | `{command, count, statistic, rawOutput}` of the **first** configuration. `command` is a quoted, copy-pasteable `go test` line (run it in `benchmarks/compare`) that reproduces `results[]`: `BenchmarkParse` only, with a `GOGC=off GOMEMLIMIT=64MiB ` prefix only when that configuration sets GOGC=off. | | `caveat` | the like-for-like caveat the website prints | | `parsers[]` | the v1 parsers only: `gosqlx`, `pg_query_go`, `vitess`, `tidb`, `xwb1989` | | `queries[]` | the 11 portable queries `{id, name, sql}` | @@ -33,7 +33,7 @@ The `-cpu 1` and `GOGC=off` data is in `cells[]`. |---|---| | `peers[]` | every library under test, same shape as `parsers[]` (`id, name, module, version, dialect, cgo, license, legacy, peerDialects`). `peerDialects` lists the corpus dialects the library is a like-for-like peer for. `gosqlx_norelease` is the GoSQLX variant that skips `ReleaseAST`. | | `ormQueries[]` | the ORM-style corpus `{id, name, shape, sql}`; `shape` is quoted, placeholders, lowercase, ddl, nesting or multistatement | -| `configs[]` | the run matrix, first entry primary: `{id, cpu, gogc, command, rawOutput}`. `cpu` is the `-cpu` value (0 = go test default), `gogc` is `default` or `off`. `id` looks like `cpu4-gogc-off`. `command` is the quoted `go test` line of that run (all selected families). | +| `configs[]` | the run matrix, first entry primary: `{id, cpu, gogc, gomemlimit, command, rawOutput}`. `cpu` is the `-cpu` value (0 = go test default), `gogc` is `default` or `off`. `gomemlimit` is the `GOMEMLIMIT` of a `gogc: off` configuration (default `64MiB`, set with `-gomemlimit`; omitted for default GC or `-gomemlimit=off`): without a limit a GC-off heap lands every allocation on a fresh page and times its page fault. `id` looks like `cpu4-gogc-off`. `command` is the quoted `go test` line of that run (all selected families). | | `cells[]` | one row per (benchmark cell, group, implementation, configuration) | | `acceptance[]` | `{dialect, parser, accepted, total, rate}` over all of `testdata/realworld/`, legacy parsers included | | `commonAccept[]` | `{dialect, parsers, total, accepted, setSha256}` summarising `commonaccept/.txt` | diff --git a/benchmarks/compare/cmd/publish/main.go b/benchmarks/compare/cmd/publish/main.go index 9e48cef25..4e9c75338 100644 --- a/benchmarks/compare/cmd/publish/main.go +++ b/benchmarks/compare/cmd/publish/main.go @@ -29,6 +29,7 @@ import ( "flag" "fmt" "io" + "math" "os" "os/exec" "path/filepath" @@ -46,6 +47,13 @@ import ( const schemaVersion = 2 +// defaultGOMEMLIMIT is the GOMEMLIMIT of the GOGC=off configurations. Without +// a limit a GOGC=off heap only grows, so every allocation lands on a fresh +// page and its first-touch fault is timed with the parser; the limit lets the +// collector run rarely and reuse pages. scripts/perf/lib.sh uses the same +// default (a root-module test keeps them equal) and explains the value. +const defaultGOMEMLIMIT = "64MiB" + // gosqlxModule is the module path of the library under test; every variant // of it (gosqlx, gosqlx_norelease) reports the commit being measured. const gosqlxModule = "github.com/ajitpratap0/GoSQLX" @@ -82,11 +90,14 @@ type Report struct { // ConfigV is one point of the run matrix. type ConfigV struct { - ID string `json:"id"` - CPU int `json:"cpu"` // -cpu value; 0 means go test's default (GOMAXPROCS) - GOGC string `json:"gogc"` // "default" or "off" - Command string `json:"command"` - RawOutput string `json:"rawOutput"` + ID string `json:"id"` + CPU int `json:"cpu"` // -cpu value; 0 means go test's default (GOMAXPROCS) + GOGC string `json:"gogc"` // "default" or "off" + // GOMEMLIMIT is the memory limit of a GOGC=off configuration (empty for + // default GC, and when -gomemlimit=off). + GOMEMLIMIT string `json:"gomemlimit,omitempty"` + Command string `json:"command"` + RawOutput string `json:"rawOutput"` } // CellV is the median of all samples of one benchmark cell under one @@ -195,6 +206,8 @@ func run() error { families := flag.String("families", "all", "comma-separated benchmark families to run ("+strings.Join(allFamilies, ",")+"), or all; the v1 fields need Parse, so Parse alone gives a cheap v1-only publish") out := flag.String("out", filepath.Join("results", "latest.json"), "results JSON path") raw := flag.String("raw", filepath.Join("results", "latest.txt"), "raw go test -bench output path of the first configuration (benchstat input)") + memlimit := flag.String("gomemlimit", defaultGOMEMLIMIT, "GOMEMLIMIT of the GOGC=off configurations, or off to leave it unset") + ratios := flag.String("ratios", "", "also write a Markdown table of GoSQLX/peer time ratios per cell and configuration here") flag.Parse() if *count < 1 { @@ -240,10 +253,13 @@ func run() error { rawPath := rawPathFor(*raw, i, c) args := benchArgs(benchRe, *count, c.CPU, *benchtime) - fmt.Fprintf(os.Stderr, "running [%s]: go %s (GOGC=%s)\n", c.ID, strings.Join(args, " "), c.GOGC) + if c.GOGC == "off" && *memlimit != "off" { + c.GOMEMLIMIT = *memlimit + } + fmt.Fprintf(os.Stderr, "running [%s]: %s\n", c.ID, shellCommand(c.GOGC, *memlimit, args)) var buf bytes.Buffer cmd := exec.Command("go", args...) - cmd.Env = envForGOGC(os.Environ(), c.GOGC) + cmd.Env = envForGOGC(os.Environ(), c.GOGC, *memlimit) cmd.Stdout = io.MultiWriter(&buf, os.Stderr) cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { @@ -253,7 +269,7 @@ func run() error { if err != nil { return fmt.Errorf("%s: %w", c.ID, err) } - c.Command = shellCommand(c.GOGC, args) + c.Command = shellCommand(c.GOGC, *memlimit, args) c.RawOutput = filepath.ToSlash(rawPath) report.Configs = append(report.Configs, c) report.Cells = append(report.Cells, cellResults(data, c.ID)...) @@ -262,7 +278,7 @@ func run() error { primary = data report.Hardware = Hardware{CPU: cpu, GOOS: runtime.GOOS, GOARCH: runtime.GOARCH, NumCPU: runtime.NumCPU()} report.Method = Method{ - Command: shellCommand(c.GOGC, benchArgs(v1BenchRegex, *count, c.CPU, *benchtime)), + Command: shellCommand(c.GOGC, *memlimit, benchArgs(v1BenchRegex, *count, c.CPU, *benchtime)), Count: *count, Statistic: "median", RawOutput: c.RawOutput, @@ -318,6 +334,11 @@ func run() error { if err := writeFile(*out, append(encoded, '\n')); err != nil { return err } + if *ratios != "" { + if err := writeFile(*ratios, []byte(ratioMarkdown(report.Cells, report.Configs, report.Hardware.CPU))); err != nil { + return err + } + } fmt.Fprintf(os.Stderr, "wrote %s and %d raw file(s)\n", *out, len(raws)) return nil } @@ -365,11 +386,15 @@ func benchArgs(benchRe string, count, cpu int, benchtime string) []string { // shellCommand renders a copy-pasteable command line (to be run in // benchmarks/compare). Values with shell metacharacters are single-quoted, -// and GOGC=off is spelled out only for the configuration that sets it. -func shellCommand(gogc string, args []string) string { - parts := make([]string, 0, len(args)+2) +// and GOGC=off (with its GOMEMLIMIT unless that is "off") is spelled out only +// for the configuration that sets it. +func shellCommand(gogc, memlimit string, args []string) string { + parts := make([]string, 0, len(args)+3) if gogc == "off" { parts = append(parts, "GOGC=off") + if memlimit != "off" { + parts = append(parts, "GOMEMLIMIT="+quoteArg(memlimit)) + } } parts = append(parts, "go") for _, a := range args { @@ -446,21 +471,142 @@ func rawPathFor(raw string, i int, c ConfigV) string { return strings.TrimSuffix(raw, ext) + "-" + c.ID + ext } -// envForGOGC returns env with GOGC set for the configuration: removed for -// "default", "off" otherwise. -func envForGOGC(env []string, gogc string) []string { - out := make([]string, 0, len(env)+1) +// envForGOGC returns env with GOGC and GOMEMLIMIT set for the configuration: +// both removed for "default"; GOGC=off plus GOMEMLIMIT=memlimit (unless +// memlimit is "off") for "off". Inherited values never leak through. +func envForGOGC(env []string, gogc, memlimit string) []string { + out := make([]string, 0, len(env)+2) for _, kv := range env { - if !strings.HasPrefix(kv, "GOGC=") { + if !strings.HasPrefix(kv, "GOGC=") && !strings.HasPrefix(kv, "GOMEMLIMIT=") { out = append(out, kv) } } if gogc == "off" { out = append(out, "GOGC=off") + if memlimit != "off" { + out = append(out, "GOMEMLIMIT="+memlimit) + } } return out } +// ratioMarkdown renders the in-job comparison for the job summary: for every +// benchmark family, GoSQLX implementation (parser IDs starting with +// "gosqlx") and peer, the geometric mean over the groups (queries or +// dialects) both ran of GoSQLX ns/op divided by peer ns/op, one column per +// configuration, plus the group count and the worst group of the primary +// configuration. Absolute times are left out on purpose: hosted runners +// differ in CPU model from job to job, so only same-job ratios carry over. +func ratioMarkdown(cells []CellV, configs []ConfigV, cpu string) string { + type key struct{ cell, group, parser, config string } + ns := map[key]float64{} + type pair struct{ cell, gosqlx, peer string } + groups := map[pair]map[string]bool{} + byCell := map[string]map[string]bool{} // cell -> parser IDs + for _, c := range cells { + if c.NsPerOp <= 0 { + continue + } + ns[key{c.Cell, c.Group, c.Parser, c.Config}] = c.NsPerOp + if byCell[c.Cell] == nil { + byCell[c.Cell] = map[string]bool{} + } + byCell[c.Cell][c.Parser] = true + } + for _, c := range cells { + if !strings.HasPrefix(c.Parser, "gosqlx") { + continue + } + for peer := range byCell[c.Cell] { + if strings.HasPrefix(peer, "gosqlx") { + continue + } + p := pair{c.Cell, c.Parser, peer} + if groups[p] == nil { + groups[p] = map[string]bool{} + } + groups[p][c.Group] = true + } + } + pairs := make([]pair, 0, len(groups)) + for p := range groups { + pairs = append(pairs, p) + } + family := func(c string) int { + if i := slices.Index(allFamilies, c); i >= 0 { + return i + } + return len(allFamilies) + } + sort.Slice(pairs, func(i, j int) bool { + a, b := pairs[i], pairs[j] + if fa, fb := family(a.cell), family(b.cell); fa != fb { + return fa < fb + } + if a.cell != b.cell { + return a.cell < b.cell + } + if a.gosqlx != b.gosqlx { + return a.gosqlx < b.gosqlx + } + return a.peer < b.peer + }) + + var b strings.Builder + b.WriteString("### Ratios: GoSQLX time / peer time (< 1 means GoSQLX is faster)\n\n") + fmt.Fprintf(&b, "Runner CPU: `%s`. Every cell ran in this job on this CPU, so the ratios compare across runs; "+ + "the absolute ns/op in the JSON and raw files hold for this CPU and this job only. "+ + "Each value is the geometric mean over the groups (queries or dialects) both sides ran.\n\n", cpu) + b.WriteString("| cell | GoSQLX | peer |") + for _, c := range configs { + fmt.Fprintf(&b, " %s |", c.ID) + } + b.WriteString(" groups | worst group |\n|---|---|---|") + for range configs { + b.WriteString("---:|") + } + b.WriteString("---:|---|\n") + for _, p := range pairs { + gs := make([]string, 0, len(groups[p])) + for g := range groups[p] { + gs = append(gs, g) + } + sort.Strings(gs) + fmt.Fprintf(&b, "| %s | %s | %s |", p.cell, p.gosqlx, p.peer) + n, worst, worstG := 0, 0.0, "" + for i, c := range configs { + sum, k := 0.0, 0 + for _, g := range gs { + a, okA := ns[key{p.cell, g, p.gosqlx, c.ID}] + z, okZ := ns[key{p.cell, g, p.peer, c.ID}] + if !okA || !okZ { + continue + } + r := a / z + sum += math.Log(r) + k++ + if i == 0 && r > worst { + worst, worstG = r, g + } + } + if i == 0 { + n = k + } + if k == 0 { + b.WriteString(" |") + continue + } + fmt.Fprintf(&b, " %.2f |", math.Exp(sum/float64(k))) + } + if worstG != "" { + fmt.Fprintf(&b, " %d | %s %.2f |\n", n, worstG, worst) + } else { + fmt.Fprintf(&b, " %d | |\n", n) + } + } + return b.String() +} + // cellResults reduces samples to medians, in a stable order. func cellResults(data map[cellKey]*samples, config string) []CellV { keys := make([]cellKey, 0, len(data)) diff --git a/benchmarks/compare/cmd/publish/main_test.go b/benchmarks/compare/cmd/publish/main_test.go index 4e3d41810..8bc9ddbcb 100644 --- a/benchmarks/compare/cmd/publish/main_test.go +++ b/benchmarks/compare/cmd/publish/main_test.go @@ -153,15 +153,24 @@ func TestParseMatrix(t *testing.T) { } func TestEnvForGOGC(t *testing.T) { - base := []string{"PATH=/bin", "GOGC=50"} - if got := envForGOGC(base, "default"); slices.ContainsFunc(got, func(s string) bool { return strings.HasPrefix(s, "GOGC=") }) { - t.Errorf("default must drop an inherited GOGC, got %v", got) - } - got := envForGOGC(base, "off") + base := []string{"PATH=/bin", "GOGC=50", "GOMEMLIMIT=9GiB"} + isGC := func(s string) bool { return strings.HasPrefix(s, "GOGC=") || strings.HasPrefix(s, "GOMEMLIMIT=") } + if got := envForGOGC(base, "default", defaultGOMEMLIMIT); slices.ContainsFunc(got, isGC) { + t.Errorf("default must drop an inherited GOGC and GOMEMLIMIT, got %v", got) + } + // GOGC=off always comes with a memory limit, so the heap stops growing + // onto fresh pages whose first-touch faults would be timed (W0-15). + got := envForGOGC(base, "off", defaultGOMEMLIMIT) if !slices.Contains(got, "GOGC=off") || slices.Contains(got, "GOGC=50") { t.Errorf("off must replace GOGC, got %v", got) } - if len(base) != 2 || base[1] != "GOGC=50" { + if !slices.Contains(got, "GOMEMLIMIT="+defaultGOMEMLIMIT) || slices.Contains(got, "GOMEMLIMIT=9GiB") { + t.Errorf("off must set GOMEMLIMIT=%s in place of the inherited one, got %v", defaultGOMEMLIMIT, got) + } + if got := envForGOGC(base, "off", "off"); slices.ContainsFunc(got, func(s string) bool { return strings.HasPrefix(s, "GOMEMLIMIT=") }) { + t.Errorf("-gomemlimit=off must leave GOMEMLIMIT unset, got %v", got) + } + if len(base) != 3 || base[1] != "GOGC=50" { t.Error("envForGOGC must not modify its input") } } @@ -309,20 +318,20 @@ func TestMethodCommandIsCopyPasteable(t *testing.T) { } for _, c := range cfgs { for _, re := range []string{v1BenchRegex, "^Benchmark(Parse|ParseORM|Lex)$"} { - cmd := shellCommand(c.GOGC, benchArgs(re, 10, c.CPU, "200ms")) + cmd := shellCommand(c.GOGC, defaultGOMEMLIMIT, benchArgs(re, 10, c.CPU, "200ms")) if bad := unquotedMeta(cmd); bad != "" { t.Errorf("%q has unquoted shell metacharacters %q", cmd, bad) } if strings.Contains(cmd, "GOGC=default") { t.Errorf("%q uses GOGC=default, which is not a GOGC value", cmd) } - if (c.GOGC == "off") != strings.HasPrefix(cmd, "GOGC=off ") { - t.Errorf("%q: GOGC=off prefix must appear exactly for the off configuration", cmd) + if (c.GOGC == "off") != strings.HasPrefix(cmd, "GOGC=off GOMEMLIMIT="+defaultGOMEMLIMIT+" ") { + t.Errorf("%q: the GOGC=off GOMEMLIMIT prefix must appear exactly for the off configuration", cmd) } } } want := "go test -run='^$' -bench='^BenchmarkParse$' -benchmem -count=10 -cpu=1 -benchtime=200ms ." - if got := shellCommand("default", benchArgs(v1BenchRegex, 10, 1, "200ms")); got != want { + if got := shellCommand("default", defaultGOMEMLIMIT, benchArgs(v1BenchRegex, 10, 1, "200ms")); got != want { t.Errorf("v1 command = %q, want %q", got, want) } } @@ -351,3 +360,45 @@ func TestNoReleaseVariantGetsCommitVersion(t *testing.T) { } } } + +// TestRatioMarkdown: the job summary compares GoSQLX with each peer by the +// ratio of their times in the same job, geomean over the groups both ran, +// and names the runner CPU; it never prints absolute ns/op, which do not +// carry across jobs (hosted runners differ in CPU model). +func TestRatioMarkdown(t *testing.T) { + cfgs := []ConfigV{{ID: "cpu4-gogc-default"}, {ID: "cpu1-gogc-off"}} + cell := func(c, g, p, cfg string, ns float64) CellV { + return CellV{Cell: c, Group: g, Parser: p, Config: cfg, NsPerOp: ns, Samples: 10} + } + cells := []CellV{ + cell("Parse", "join", "gosqlx", "cpu4-gogc-default", 12345), + cell("Parse", "join", "vitess", "cpu4-gogc-default", 10288), + cell("Parse", "aggregate", "gosqlx", "cpu4-gogc-default", 13000), + cell("Parse", "aggregate", "vitess", "cpu4-gogc-default", 10000), + cell("Parse", "join", "tidb", "cpu4-gogc-default", 24690), + cell("Parse", "join", "gosqlx", "cpu1-gogc-off", 900), + cell("Parse", "join", "vitess", "cpu1-gogc-off", 1000), + cell("Lex", "join", "gosqlx_scratch", "cpu4-gogc-default", 3700), + cell("Lex", "join", "vitess", "cpu4-gogc-default", 1000), + } + md := ratioMarkdown(cells, cfgs, "AMD EPYC 7763 64-Core Processor") + for _, want := range []string{ + "Runner CPU: `AMD EPYC 7763 64-Core Processor`", + "| Parse | gosqlx | vitess | 1.25 | 0.90 | 2 | aggregate 1.30 |", + "| Parse | gosqlx | tidb | 0.50 | | 1 | join 0.50 |", + "| Lex | gosqlx_scratch | vitess | 3.70 | | 1 | join 3.70 |", + "| cell | GoSQLX | peer | cpu4-gogc-default | cpu1-gogc-off |", + } { + if !strings.Contains(md, want) { + t.Errorf("ratio table lacks %q:\n%s", want, md) + } + } + for _, abs := range []string{"12345", "10288", "24690"} { + if strings.Contains(md, abs) { + t.Errorf("ratio table prints the absolute time %s:\n%s", abs, md) + } + } + if i, j := strings.Index(md, "| Parse |"), strings.Index(md, "| Lex |"); i < 0 || j < 0 || i > j { + t.Errorf("rows must follow the family order (Parse before Lex):\n%s", md) + } +} diff --git a/pkg/lsp/handler_bench_test.go b/pkg/lsp/handler_bench_test.go index 46f630bde..3a4c77a16 100644 --- a/pkg/lsp/handler_bench_test.go +++ b/pkg/lsp/handler_bench_test.go @@ -513,13 +513,11 @@ LIMIT 10` } } -// setupTestServer creates a test server with mock I/O +// setupTestServer creates a test server with empty input whose output +// (responses and publishDiagnostics notifications) is discarded, so +// benchmarks do not time an ever-growing output buffer. func setupTestServer() *Server { - mock := &mockReadWriter{ - input: bytes.NewBuffer(nil), - output: bytes.NewBuffer(nil), - } - return NewServer(mock.input, mock.output, log.New(io.Discard, "", 0)) + return NewServer(bytes.NewBuffer(nil), io.Discard, log.New(io.Discard, "", 0)) } // generateLargeSQLDocument generates a SQL document with the specified number of statements @@ -613,3 +611,14 @@ func BenchmarkServer_SendMessage(b *testing.B) { server.SendNotification("window/logMessage", params) } } + +// TestSetupTestServerDiscardsOutput: the benchmarks (and the W0-06 LSP +// matrix) build their server with setupTestServer, and validateDocument sends +// a publishDiagnostics message on every call. Writing those into a growing +// buffer would time buffer growth and copying that scale with b.N and inflate +// B/op, so the output must go to io.Discard. No caller reads it back. +func TestSetupTestServerDiscardsOutput(t *testing.T) { + if w := setupTestServer().writer; w != io.Discard { + t.Fatalf("setupTestServer writes to %T, want io.Discard", w) + } +} diff --git a/scripts/perf/ab.sh b/scripts/perf/ab.sh index f6dd0c21f..8b327bbbc 100755 --- a/scripts/perf/ab.sh +++ b/scripts/perf/ab.sh @@ -8,7 +8,9 @@ # revisions with the same toolchain, in the same job. # - Primary metric: GOGC=off GOMAXPROCS=1 (-test.cpu 1), pinned to one CPU # with taskset when it exists. GC-free mutator time is low-variance and -# ranks queries like the -cpu 4 headline does. +# ranks queries like the -cpu 4 headline does. GOMEMLIMIT (default 64MiB, +# see scripts/perf/lib.sh) bounds the heap so the runs reuse pages instead +# of paying first-touch page faults. # - Arms run interleaved, one -test.count 1 pass per arm per round, in # ABBA order (two arms) or rotating through every permutation (three). # - Optional A/A control (-a): a second, interleaved run of the base binary. @@ -17,6 +19,8 @@ # else cachegrind, else macOS `/usr/bin/time -l`, at two iteration counts; # the difference divided by the extra iterations is instructions/op. # - allocs/op and B/op come from the primary runs (GOGC=off: no pool drains). +# - env.txt and the report name the CPU model: hosted runners differ from +# job to job, so only the in-job base/head ratios are reported. # # Usage: scripts/perf/ab.sh [options] BASE [HEAD] # BASE git revision of the baseline @@ -39,6 +43,7 @@ # PIN_CPU CPU for taskset (default: the last online CPU; "none" disables pinning) # BENCHSTAT benchstat command (default: benchstat on PATH, else the pinned version via go run) # IR_SAMPLES samples per perfdriver case, median taken (default 3, every backend) +# PERF_GOMEMLIMIT GOMEMLIMIT of the GOGC=off runs (default 64MiB; "off" unsets it) # # Output: DIR/report.md (also printed), DIR/status (pass, regression, # inconclusive), DIR/raw/*.txt (raw benchmark output), DIR/env.txt. @@ -48,6 +53,12 @@ set -euo pipefail BENCHSTAT_VERSION=v0.0.0-20260908200009-22c9c6c9d4da +# shellcheck source=scripts/perf/lib.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" +gcoff_env=() +while IFS= read -r kv; do gcoff_env+=("$kv"); done < <(perf_gcoff_env) +memlimit=$(perf_gomemlimit) + usage() { sed -n '2,/^set -euo/p' "$0" | sed -e '$d' -e 's/^# \{0,1\}//'; } die() { echo "ab.sh: $*" >&2; exit "${2:-3}"; } log() { echo "ab.sh: $*" >&2; } @@ -190,9 +201,11 @@ fi echo "date: $(date -u +%Y-%m-%dT%H:%M:%SZ)" echo "base: $base_sha" echo "head: ${head_sha:-working tree of $(git -C "$root" rev-parse HEAD)}" + echo "cpu model: $(perf_cpu_model)" echo "go: $(go version)" echo "kernel: $(uname -srm)" echo "pinning: $pin_desc" + echo "gcoff env: ${gcoff_env[*]}" echo "clocksource: $(cat /sys/devices/system/clocksource/clocksource0/current_clocksource 2>/dev/null || echo n/a)" echo "load: $(uptime | sed 's/.*load average[s]*: //')" echo "instr count: ${ir_backend:-none}${ir_note:+ ($ir_note)}" @@ -215,7 +228,7 @@ run_arm() { # arm config outfile case $arm in base | base2) bin=$out/bin/base tree=$base_tree ;; head) bin=$out/bin/head tree=$head_tree ;; esac for i in "${!pkgs[@]}"; do local pkg=${pkgs[$i]} re=${regexes[$i]} envs - if [ "$config" = gcoff ]; then envs=(GOGC=off GOMAXPROCS=1); else envs=(-u GOGC GOMAXPROCS=1); fi + if [ "$config" = gcoff ]; then envs=("${gcoff_env[@]}" GOMAXPROCS=1); else envs=(-u GOGC -u GOMEMLIMIT GOMAXPROCS=1); fi (cd "$tree/$pkg" && env "${envs[@]}" ${pin[@]+"${pin[@]}"} "$bin/$(slug "$pkg").test" \ -test.run '^$' -test.bench "$re" -test.benchmem -test.count 1 \ -test.benchtime "$benchtime" -test.cpu 1) >"$out/run.log" 2>&1 || @@ -258,7 +271,7 @@ count_instructions() { # arm case n -> prints the count case $arm in base) tree=$base_tree ;; head) tree=$head_tree ;; esac # The environment is set outside the counter so that the counted # process is perfdriver itself, not env (valgrind does not follow exec). - local envs=(GOGC=off GOMAXPROCS=1 GODEBUG=asyncpreemptoff=1) + local envs=("${gcoff_env[@]}" GOMAXPROCS=1 GODEBUG=asyncpreemptoff=1) local drv=("$out/bin/$arm/perfdriver" -case "$c" -n "$n") case $ir_backend in perf) @@ -333,7 +346,7 @@ judge() { # extra gate flags... local args=(-base "$out/raw/gcoff-base.txt" -head "$out/raw/gcoff-head.txt" -md "$out/report.md" -status "$out/status" -env "$out/env.txt" -benchstat "$out/benchstat.txt" -base-ref "$base_label" -head-ref "$head_label" - -settings "$rounds interleaved rounds, benchtime $benchtime, -cpu 1, $pin_desc.") + -settings "$rounds interleaved rounds, benchtime $benchtime, -cpu 1, GOMEMLIMIT $memlimit, $pin_desc.") [ $aa -eq 1 ] && args+=(-aa "$out/raw/gcoff-base2.txt") [ $sec -eq 1 ] && args+=(-sec-base "$out/raw/gcon-base.txt" -sec-head "$out/raw/gcon-head.txt") if [ $ir -eq 1 ]; then diff --git a/scripts/perf/gate/config_test.go b/scripts/perf/gate/config_test.go new file mode 100644 index 000000000..ec55accd4 --- /dev/null +++ b/scripts/perf/gate/config_test.go @@ -0,0 +1,287 @@ +// Copyright 2026 GoSQLX Authors +// SPDX-License-Identifier: Apache-2.0 + +package main + +// Tests of the perf tooling's configuration (W0-15): the toolchain pin of the +// perf workflows, GOMEMLIMIT next to every GOGC=off measurement, the shell +// helpers in scripts/perf/lib.sh and the full-matrix suite. They read the +// repository files, so they run from scripts/perf/gate. + +import ( + "bufio" + "os" + "os/exec" + "path/filepath" + "regexp" + "runtime" + "sort" + "strings" + "testing" +) + +const repoRoot = "../../.." + +func readRepoFile(t *testing.T, rel string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(repoRoot, filepath.FromSlash(rel))) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +var perfWorkflows = []string{".github/workflows/perf-gate.yml", ".github/workflows/bench-compare.yml"} + +// TestPerfWorkflowsPinOneGoToolchain: perf numbers from the gate and from the +// competitor harness are only comparable when both are built by the same Go +// release (C0 found the gate on 1.26.1 from go.mod and the harness on 1.26.9). +// Each workflow pins PERF_GO_VERSION to the same exact release, every +// setup-go step uses it without check-latest, and GOTOOLCHAIN=local stops a +// go.mod from switching toolchains behind the pin. +func TestPerfWorkflowsPinOneGoToolchain(t *testing.T) { + pinRe := regexp.MustCompile(`(?m)^\s+PERF_GO_VERSION:\s*'?([0-9]+\.[0-9]+\.[0-9]+)'?\s*$`) + pins := map[string]string{} + for _, wf := range perfWorkflows { + src := readRepoFile(t, wf) + m := pinRe.FindAllStringSubmatch(src, -1) + if len(m) != 1 { + t.Fatalf("%s: want exactly one PERF_GO_VERSION: 'X.Y.Z' in the workflow env, found %d", wf, len(m)) + } + pins[wf] = m[0][1] + if !regexp.MustCompile(`(?m)^\s+GOTOOLCHAIN:\s*local\s*$`).MatchString(src) { + t.Errorf("%s: GOTOOLCHAIN must be local so go.mod cannot switch toolchains", wf) + } + if strings.Contains(src, "go-version-file") { + t.Errorf("%s: setup-go must use the pinned PERF_GO_VERSION, not go-version-file", wf) + } + steps := strings.Split(src, "uses: actions/setup-go@")[1:] + if len(steps) == 0 { + t.Fatalf("%s: no setup-go step", wf) + } + for i, step := range steps { + with := step + if j := strings.Index(with, "- name:"); j >= 0 { + with = with[:j] + } + if !strings.Contains(with, "go-version: ${{ env.PERF_GO_VERSION }}") { + t.Errorf("%s: setup-go step %d does not use go-version: ${{ env.PERF_GO_VERSION }}", wf, i+1) + } + if !strings.Contains(with, "check-latest: false") { + t.Errorf("%s: setup-go step %d must set check-latest: false", wf, i+1) + } + } + } + if a, b := pins[perfWorkflows[0]], pins[perfWorkflows[1]]; a != b { + t.Errorf("perf workflows pin different toolchains: %s=%s, %s=%s", perfWorkflows[0], a, perfWorkflows[1], b) + } +} + +// TestGCOffMeasurementsSetGOMEMLIMIT: every place a perf script or workflow +// turns the collector off also bounds the heap, so first-touch page faults do +// not pollute timings and profiles (C0 methodology finding 2). +func TestGCOffMeasurementsSetGOMEMLIMIT(t *testing.T) { + files := []string{"scripts/perf/ab.sh", "scripts/perf/profile.sh", "Taskfile.yml"} + files = append(files, perfWorkflows...) + gcoff := regexp.MustCompile(`GOGC[=:]\s*'?off`) + for _, f := range files { + sc := bufio.NewScanner(strings.NewReader(readRepoFile(t, f))) + for n := 1; sc.Scan(); n++ { + line := strings.TrimSpace(sc.Text()) + // Comments and Taskfile descriptions only talk about GOGC=off. + if strings.HasPrefix(line, "#") || strings.HasPrefix(line, "desc:") || !gcoff.MatchString(line) { + continue + } + // gcoff_env is the array scripts fill from perf_gcoff_env. + if !strings.Contains(line, "GOMEMLIMIT") && !strings.Contains(line, "gcoff_env") { + t.Errorf("%s:%d sets GOGC=off without GOMEMLIMIT (use perf_gcoff_env from scripts/perf/lib.sh): %s", f, n, line) + } + } + } + // The competitor harness lives in its own module; its default must match. + lib := readRepoFile(t, "scripts/perf/lib.sh") + m := regexp.MustCompile(`(?m)^PERF_GOMEMLIMIT_DEFAULT=(\S+)$`).FindStringSubmatch(lib) + if m == nil { + t.Fatal("scripts/perf/lib.sh: no PERF_GOMEMLIMIT_DEFAULT") + } + pub := readRepoFile(t, "benchmarks/compare/cmd/publish/main.go") + if want := `defaultGOMEMLIMIT = "` + m[1] + `"`; !strings.Contains(pub, want) { + t.Errorf("benchmarks/compare/cmd/publish: want %s, the scripts/perf/lib.sh default", want) + } +} + +// bashLib runs a snippet with scripts/perf/lib.sh sourced. +func bashLib(t *testing.T, env []string, script string) string { + t.Helper() + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not installed") + } + cmd := exec.Command("bash", "-c", `set -euo pipefail; . "$1"; `+script, "bash", filepath.Join(repoRoot, "scripts", "perf", "lib.sh")) + cmd.Env = append(os.Environ(), env...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + return string(out) +} + +func TestGCOffEnvSetsTheMemoryLimit(t *testing.T) { + for _, tc := range []struct { + name string + env []string + want string + }{ + {"default", []string{"PERF_GOMEMLIMIT="}, "GOGC=off\nGOMEMLIMIT=64MiB\n"}, + {"override", []string{"PERF_GOMEMLIMIT=1GiB"}, "GOGC=off\nGOMEMLIMIT=1GiB\n"}, + {"off unsets an inherited limit", []string{"PERF_GOMEMLIMIT=off"}, "-u\nGOMEMLIMIT\nGOGC=off\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := bashLib(t, tc.env, "perf_gcoff_env"); got != tc.want { + t.Errorf("perf_gcoff_env = %q, want %q", got, tc.want) + } + // The arguments must work as given: env(1) applies them. + got := bashLib(t, append(tc.env, "GOMEMLIMIT=inherited"), `a=(); while IFS= read -r kv; do a+=("$kv"); done < <(perf_gcoff_env); env "${a[@]}" sh -c 'echo "GOGC=$GOGC GOMEMLIMIT=${GOMEMLIMIT:-unset}"'`) + wantLimit := strings.TrimPrefix(strings.Split(strings.TrimSpace(tc.want), "\n")[1], "GOMEMLIMIT=") + if strings.HasPrefix(tc.want, "-u") { + wantLimit = "unset" + } + if want := "GOGC=off GOMEMLIMIT=" + wantLimit + "\n"; got != want { + t.Errorf("env with perf_gcoff_env: got %q, want %q", got, want) + } + }) + } +} + +func TestCPUModelIsRecorded(t *testing.T) { + got := strings.TrimSpace(bashLib(t, nil, "perf_cpu_model")) + if got == "" || strings.HasPrefix(got, "unknown") && runtime.GOOS == "linux" { + t.Fatalf("perf_cpu_model = %q, want a model name", got) + } + if !strings.HasSuffix(got, ")") { + t.Errorf("perf_cpu_model = %q, want the architecture in parentheses", got) + } +} + +// valgrindSummary is cachegrind's stderr summary with --cache-sim=yes +// (valgrind 3.22, as installed on ubuntu-24.04 runners). +const valgrindSummary = `==4242== Cachegrind, a high-precision tracing profiler +==4242== +==4242== I refs: 2,149,394 +==4242== I1 misses: 1,276 +==4242== LLi misses: 1,232 +==4242== I1 miss rate: 0.06% +==4242== LLi miss rate: 0.06% +==4242== +==4242== D refs: 758,433 (542,105 rd + 216,328 wr) +==4242== D1 misses: 14,357 ( 12,198 rd + 2,159 wr) +==4242== LLd misses: 9,032 ( 7,920 rd + 1,112 wr) +==4242== D1 miss rate: 1.9% ( 2.3% + 1.0% ) +==4242== LLd miss rate: 1.2% ( 1.5% + 0.5% ) +==4242== +==4242== LL refs: 15,633 ( 13,474 rd + 2,159 wr) +==4242== LL misses: 10,264 ( 9,152 rd + 1,112 wr) +==4242== LL miss rate: 0.4% ( 0.3% + 0.5% ) +` + +func TestCachegrindCountsParseTheSummary(t *testing.T) { + dir := t.TempDir() + write := func(name, s string) string { + p := filepath.Join(dir, name) + if err := os.WriteFile(p, []byte(s), 0o644); err != nil { + t.Fatal(err) + } + return p + } + full := write("full.log", valgrindSummary) + if got := bashLib(t, nil, `perf_cachegrind_counts "`+full+`"`); got != "2149394 1276 1232 758433 14357 9032\n" { + t.Errorf("cache-sim summary: got %q", got) + } + // --cache-sim=no (the perf gate's instruction counts): only I refs. + irOnly := write("ir.log", "==7== I refs: 123,456,789\n") + if got := bashLib(t, nil, `perf_cachegrind_counts "`+irOnly+`"`); got != "123456789 0 0 0 0 0\n" { + t.Errorf("Ir-only summary: got %q", got) + } + empty := write("empty.log", "valgrind: failed to start tool\n") + if got := bashLib(t, nil, `if perf_cachegrind_counts "`+empty+`"; then echo parsed; else echo failed; fi`); got != "failed\n" { + t.Errorf("a log without I refs must fail, got %q", got) + } +} + +// TestMatrixSuiteCoversEveryBenchmarkPackage: the full-matrix mode of the +// perf gate runs the suite scripts/perf/matrix-suite.sh prints, which must +// name every package with benchmarks that `task bench:matrix` runs (./pkg/... +// and ./wasm/internal/... except the CGO-only cbinding), each once, with the +// regex ".". The expected set is computed here independently of the script. +func TestMatrixSuiteCoversEveryBenchmarkPackage(t *testing.T) { + want := map[string]bool{} + benchRe := regexp.MustCompile(`(?m)^func Benchmark\w*\(`) + for _, top := range []string{"pkg", "wasm/internal"} { + err := filepath.WalkDir(filepath.Join(repoRoot, top), func(p string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() || !strings.HasSuffix(p, "_test.go") { + return err + } + src, err := os.ReadFile(p) + if err != nil { + return err + } + if benchRe.Match(src) { + rel, err := filepath.Rel(repoRoot, filepath.Dir(p)) + if err != nil { + return err + } + if rel = filepath.ToSlash(rel); rel != "pkg/cbinding" { + want["./"+rel] = true + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + } + if !want["./pkg/gosqlx"] || !want["./pkg/sql/tokenizer"] { + t.Fatalf("benchmark package walk found %v, which lacks pkg/gosqlx or pkg/sql/tokenizer", want) + } + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not installed") + } + out, err := exec.Command("bash", filepath.Join(repoRoot, "scripts", "perf", "matrix-suite.sh"), repoRoot).CombinedOutput() + if err != nil { + t.Fatalf("matrix-suite.sh: %v\n%s", err, out) + } + got := map[string]bool{} + sc := bufio.NewScanner(strings.NewReader(string(out))) + for sc.Scan() { + f := strings.Fields(sc.Text()) + if len(f) == 0 || strings.HasPrefix(f[0], "#") { + continue + } + if len(f) != 2 || f[1] != "." { + t.Errorf("matrix suite: want \" .\", got %q", sc.Text()) + continue + } + if got[f[0]] { + t.Errorf("matrix suite: %s listed twice", f[0]) + } + got[f[0]] = true + } + var missing, extra []string + for p := range want { + if !got[p] { + missing = append(missing, p) + } + } + for p := range got { + if !want[p] { + extra = append(extra, p) + } + } + sort.Strings(missing) + sort.Strings(extra) + if len(missing) > 0 { + t.Errorf("matrix suite misses benchmark packages: %v", missing) + } + if len(extra) > 0 { + t.Errorf("matrix suite lists packages without benchmarks: %v", extra) + } +} diff --git a/scripts/perf/gate/gate.go b/scripts/perf/gate/gate.go index 3b0394d5e..4e75b3318 100644 --- a/scripts/perf/gate/gate.go +++ b/scripts/perf/gate/gate.go @@ -421,6 +421,8 @@ func Markdown(res Result, cfg Config, meta Meta) string { } b.WriteString("\n\n") } + fmt.Fprintf(&b, "Runner CPU: `%s`. Base and head ran interleaved in this job, so the tables show their ratios only; "+ + "absolute timings (raw files in the artifact) hold for this CPU and this job alone and are never compared across jobs.\n\n", runnerCPU(meta.Env)) if len(res.Reasons) > 0 { for _, r := range res.Reasons { fmt.Fprintf(&b, "- %s\n", r) @@ -512,6 +514,22 @@ func Markdown(res Result, cfg Config, meta Meta) string { return b.String() } +// runnerCPU extracts the CPU model from the environment block: the +// "cpu model:" line scripts/perf/lib.sh writes, else lscpu's "Model name:", +// else "unknown". +func runnerCPU(env string) string { + for _, prefix := range []string{"cpu model:", "Model name:"} { + for _, line := range strings.Split(env, "\n") { + if v, ok := strings.CutPrefix(strings.TrimSpace(line), prefix); ok { + if v = strings.TrimSpace(v); v != "" { + return v + } + } + } + } + return "unknown" +} + func codeList(names []string) string { q := make([]string, len(names)) for i, n := range names { diff --git a/scripts/perf/gate/gate_test.go b/scripts/perf/gate/gate_test.go index de80e7102..8b8ccf7b8 100644 --- a/scripts/perf/gate/gate_test.go +++ b/scripts/perf/gate/gate_test.go @@ -481,3 +481,27 @@ func TestUnequalSampleCountsSkipTheGeomeanP(t *testing.T) { t.Fatalf("verdict = %s (%v), want pass", res.Verdict, res.Reasons) } } + +// Hosted runners hand out different CPU models from job to job (C0 saw an +// EPYC 7763, 9V74 and 9V45 in one checkpoint), so the report names the CPU +// it ran on and says that only the in-job ratios are comparable. +func TestReportNamesTheRunnerCPU(t *testing.T) { + s := suite() + res := evaluate(t, s, s, s, DefaultConfig()) + for _, tc := range []struct{ name, env, want string }{ + {"cpu model line", "date: x\ncpu model: AMD EPYC 7763 64-Core Processor (x86_64)\ngo: go1.26.9\n", "AMD EPYC 7763 64-Core Processor (x86_64)"}, + {"lscpu only", "go: go1.26.9\n\nArchitecture: x86_64\nModel name: AMD EPYC 9V45 96-Core Processor\n", "AMD EPYC 9V45 96-Core Processor"}, + {"no environment", "", "unknown"}, + } { + t.Run(tc.name, func(t *testing.T) { + md := Markdown(res, DefaultConfig(), Meta{BaseRef: "aaaa", HeadRef: "bbbb", Env: tc.env}) + want := "Runner CPU: `" + tc.want + "`" + if !strings.Contains(md, want) { + t.Fatalf("report does not name the runner CPU, want %q in:\n%s", want, md) + } + if !strings.Contains(md, "ratios") { + t.Errorf("report does not say that only in-job ratios are compared:\n%s", md) + } + }) + } +} diff --git a/scripts/perf/lib.sh b/scripts/perf/lib.sh new file mode 100644 index 000000000..30e2e3cf8 --- /dev/null +++ b/scripts/perf/lib.sh @@ -0,0 +1,77 @@ +# lib.sh - settings and helpers shared by the perf scripts (ab.sh, profile.sh). +# Source it; it defines functions and one default and runs nothing. +# +# GOMEMLIMIT next to GOGC=off +# Every GOGC=off measurement also sets GOMEMLIMIT (PERF_GOMEMLIMIT, default +# below; "off" leaves the limit unset). With GOGC=off and no limit the heap +# only grows, so every allocated byte lands on a fresh page and pays a +# first-touch page fault plus kernel zeroing. That cost is not mutator work, +# scales with B/op and lands on whichever benchmark first grows the heap in a +# process: in the C0 A/A run (EPYC 9V45) ParseE2E/simple_select, the first +# benchmark of its binary, gained nothing from GOGC=off while every other +# query gained 21-25%, and the GOGC=off profiles spent 6-11% in page-touching +# memclr. With a limit the collector runs only when the heap reaches it and +# the freed pages are reused. The limit must sit below what one 200ms +# benchmark run allocates (about 100 MB for simple_select), or it changes +# nothing; each collection of the tiny live heap costs about 0.5 ms of one +# CPU, so 64MiB keeps the collector to about 1% of a GOMAXPROCS=1 run. +# The default-GC arms unset GOMEMLIMIT so an inherited value cannot leak in. +# benchmarks/compare/cmd/publish uses the same default (a test keeps them +# equal). +PERF_GOMEMLIMIT_DEFAULT=64MiB + +# perf_gomemlimit prints the GOMEMLIMIT that GOGC=off runs use, or "off". +perf_gomemlimit() { + printf '%s\n' "${PERF_GOMEMLIMIT:-$PERF_GOMEMLIMIT_DEFAULT}" +} + +# perf_gcoff_env prints the env(1) arguments of a GOGC=off measurement, one +# per line: GOGC=off and GOMEMLIMIT=, or, when the limit is off, +# "-u" "GOMEMLIMIT" first (env options must precede assignments) and then +# GOGC=off. Read it into an array with a while-read loop (macOS ships bash +# 3.2, which has no mapfile). +perf_gcoff_env() { + local limit + limit=$(perf_gomemlimit) + if [ "$limit" = off ]; then + printf '%s\n' -u GOMEMLIMIT GOGC=off + else + printf '%s\n' GOGC=off "GOMEMLIMIT=$limit" + fi +} + +# perf_cpu_model prints the CPU model of this machine and its architecture, +# for example "AMD EPYC 7763 64-Core Processor (x86_64)". Hosted runners hand +# out different CPU models from one job to the next, so every artifact records +# it and absolute timings are only compared within one job. +perf_cpu_model() { + local m="" + if [ -r /proc/cpuinfo ]; then + m=$(sed -n 's/^model name[[:space:]]*:[[:space:]]*//p' /proc/cpuinfo | head -n 1) + fi + if [ -z "$m" ] && command -v lscpu >/dev/null 2>&1; then + m=$(lscpu | sed -n 's/^Model name:[[:space:]]*//p' | head -n 1) + fi + if [ -z "$m" ] && [ "$(uname -s)" = Darwin ]; then + m=$(sysctl -n machdep.cpu.brand_string 2>/dev/null || true) + fi + m=$(printf '%s' "$m" | sed -e 's/[[:space:]]\{1,\}/ /g' -e 's/^ //' -e 's/ $//') + printf '%s (%s)\n' "${m:-unknown}" "$(uname -m)" +} + +# perf_cachegrind_counts reads the summary that `valgrind --tool=cachegrind` +# writes to stderr and prints "Ir I1mr ILmr Dr D1mr DLmr" as plain integers +# (0 for a counter the summary lacks; with --cache-sim=no only Ir is there). +# It fails when the summary has no "I refs" line. +perf_cachegrind_counts() { + local f=$1 key ir + ir=$(sed -nE 's/.*[[:space:]]I[[:space:]]+refs:[[:space:]]+([0-9,]+).*/\1/p' "$f" | head -n 1 | tr -d ,) + [ -n "$ir" ] || return 1 + printf '%s' "$ir" + for key in 'I1[[:space:]]+misses' 'LLi[[:space:]]+misses' 'D[[:space:]]+refs' 'D1[[:space:]]+misses' 'LLd[[:space:]]+misses'; do + local v + v=$(sed -nE "s/.*[[:space:]]${key}:[[:space:]]+([0-9,]+).*/\\1/p" "$f" | head -n 1 | tr -d ,) + printf ' %s' "${v:-0}" + done + printf '\n' +} diff --git a/scripts/perf/matrix-suite.sh b/scripts/perf/matrix-suite.sh new file mode 100755 index 000000000..2a7affc6e --- /dev/null +++ b/scripts/perf/matrix-suite.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +# matrix-suite.sh - print the full benchmark matrix as a perf-gate suite file +# (the format of scripts/perf/gate-suite.txt) for the "matrix" dispatch mode of +# .github/workflows/perf-gate.yml: every package under ./pkg and +# ./wasm/internal that has benchmarks, with the regex "." (the packages +# `task bench:matrix` runs), except pkg/cbinding, which needs CGO +# (`task test:cbinding` covers it). The list is generated, so a package that +# gains benchmarks joins the matrix without editing a file. +# +# Usage: scripts/perf/matrix-suite.sh [REPO_ROOT] > suite.txt +# scripts/perf/ab.sh -s suite.txt BASE +set -euo pipefail +root=${1:-$(git rev-parse --show-toplevel)} +cd "$root" +echo "# Full benchmark matrix, generated by scripts/perf/matrix-suite.sh" +grep -rlE --include='*_test.go' '^func Benchmark[A-Za-z0-9_]*\(' pkg wasm/internal | + sed 's#/[^/]*$##' | sort -u | grep -vx 'pkg/cbinding' | sed -e 's#^#./#' -e 's#$# .#' diff --git a/scripts/perf/profile.sh b/scripts/perf/profile.sh index 7561987c3..02a3ba1a8 100755 --- a/scripts/perf/profile.sh +++ b/scripts/perf/profile.sh @@ -6,17 +6,34 @@ # For each query it records a pprof CPU profile of BenchmarkParseE2E/ in # three configurations, and a `go tool pprof -top` listing of each: # cpu1 GOMAXPROCS=1, default GC (pinned with taskset when available) -# gcoff GOMAXPROCS=1, GOGC=off (the perf gate's primary metric) +# gcoff GOMAXPROCS=1, GOGC=off with GOMEMLIMIT (the perf gate's primary +# metric; the limit, default 64MiB from scripts/perf/lib.sh, keeps +# first-touch page faults out of the profile) # cpu4 GOMAXPROCS=4, default GC (the competitor-harness CI shape) # When `perf stat` works it also counts cycles, instructions, branch and # front-end/i-cache events over cmd/perfdriver -case e2e/ (events the -# machine lacks show as ). +# machine lacks show as ). Hosted runners have no hardware +# counters, so when valgrind is installed it also simulates the caches with +# cachegrind over cmd/perfdriver -case e2e/: a 32 KiB 8-way L1i (the L1i of +# Zen 3, Zen 4 and Zen 5), a 32 KiB 8-way L1d and a 32 MiB 16-way LL, the same +# geometry on every runner, so its per-operation counts compare across jobs. +# Two iteration counts are run and differenced, so start-up cancels out. # # Usage: scripts/perf/profile.sh [-o DIR] [-t BENCHTIME] [-q "join aggregate complex insert"] # Run it from a checkout of the revision to profile. Output: DIR/{env.txt, -# *.pprof, top-*.txt, perfstat-*.txt, gosqlx.test, perfdriver, summary.md}. +# *.pprof, top-*.txt, perfstat-*.txt, cachegrind-*.out, cg-top-*.txt, +# gosqlx.test, perfdriver, summary.md}. env.txt and summary.md name the CPU +# model: absolute timings hold for that CPU only. +# Environment: PERF_GOMEMLIMIT (GOMEMLIMIT of the GOGC=off runs; "off" unsets it), +# CG_ITERS ("N1 N2", perfdriver iterations of the two cachegrind runs, default "200 2200"). set -euo pipefail +# shellcheck source=scripts/perf/lib.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" +gcoff_env=() +while IFS= read -r kv; do gcoff_env+=("$kv"); done < <(perf_gcoff_env) +cpu_model=$(perf_cpu_model) + out="" benchtime=3s queries="join aggregate complex insert" while getopts ":o:t:q:h" opt; do case $opt in @@ -43,30 +60,43 @@ perf_ok=0 if command -v perf >/dev/null 2>&1 && perf stat -x, -e instructions:u -- true 2>&1 | grep -Eq '^[0-9]+,'; then perf_ok=1 fi +cg_ok=0 +if command -v valgrind >/dev/null 2>&1 && valgrind --tool=cachegrind --help >/dev/null 2>&1; then + cg_ok=1 +fi +read -r cg_n1 cg_n2 <<<"${CG_ITERS:-200 2200}" +case "$cg_n1:$cg_n2" in *[!0-9:]* | :* | *:) echo "profile.sh: CG_ITERS must be two integers, got '${CG_ITERS:-}'" >&2; exit 2 ;; esac +[ "$cg_n2" -gt "$cg_n1" ] || { echo "profile.sh: CG_ITERS: N2 must exceed N1" >&2; exit 2; } +cg_sim=(--cache-sim=yes --I1=32768,8,64 --D1=32768,8,64 --LL=33554432,16,64) { echo "date: $(date -u +%Y-%m-%dT%H:%M:%SZ)" echo "revision: $(git -C "$root" rev-parse HEAD)" + echo "cpu model: $cpu_model" echo "go: $(go version)" + echo "gcoff env: ${gcoff_env[*]}" echo "kernel: $(uname -srm)" echo "clocksource: $(cat /sys/devices/system/clocksource/clocksource0/current_clocksource 2>/dev/null || echo n/a)" echo "perf stat: $([ $perf_ok -eq 1 ] && echo available || echo unavailable)" + echo "cachegrind: $([ $cg_ok -eq 1 ] && echo "$(valgrind --version) ${cg_sim[*]}, $cg_n1 and $cg_n2 iterations" || echo unavailable)" if command -v lscpu >/dev/null 2>&1; then echo; lscpu; fi } >"$out/env.txt" { echo "## Parse profiles of the gap queries" echo - echo "Revision \`$(git -C "$root" rev-parse --short=12 HEAD)\`, benchtime $benchtime. Profiles, the test binary and \`perf stat\` output are in the artifact." + echo "Revision \`$(git -C "$root" rev-parse --short=12 HEAD)\`, benchtime $benchtime, GOGC=off runs with \`${gcoff_env[*]}\`. Profiles, the test binary, \`perf stat\` and cachegrind output are in the artifact." + echo + echo "Runner CPU: \`$cpu_model\`. Absolute ns/op and profile shares hold for this CPU only; compare runs by ratio or by in-job A/B, never by absolute time across jobs." echo } >"$out/summary.md" for q in $queries; do for cfg in cpu1 gcoff cpu4; do case $cfg in - cpu1) envs=(-u GOGC GOMAXPROCS=1) cpu=1 p=(${pin[@]+"${pin[@]}"}) ;; - gcoff) envs=(GOGC=off GOMAXPROCS=1) cpu=1 p=(${pin[@]+"${pin[@]}"}) ;; - cpu4) envs=(-u GOGC GOMAXPROCS=4) cpu=4 p=() ;; + cpu1) envs=(-u GOGC -u GOMEMLIMIT GOMAXPROCS=1) cpu=1 p=(${pin[@]+"${pin[@]}"}) ;; + gcoff) envs=("${gcoff_env[@]}" GOMAXPROCS=1) cpu=1 p=(${pin[@]+"${pin[@]}"}) ;; + cpu4) envs=(-u GOGC -u GOMEMLIMIT GOMAXPROCS=4) cpu=4 p=() ;; esac echo "profile.sh: $q $cfg" >&2 (cd "$root/pkg/gosqlx" && env "${envs[@]}" ${p[@]+"${p[@]}"} "$out/gosqlx.test" \ @@ -79,7 +109,7 @@ for q in $queries; do (cd "$root" && perf stat -x, -o "$out/perfstat-$q.txt" \ -e cycles:u,instructions:u,branches:u,branch-misses:u \ -e L1-icache-load-misses,iTLB-load-misses,stalled-cycles-frontend,L1-dcache-load-misses \ - -- env GOGC=off GOMAXPROCS=1 GODEBUG=asyncpreemptoff=1 ${pin[@]+"${pin[@]}"} \ + -- env "${gcoff_env[@]}" GOMAXPROCS=1 GODEBUG=asyncpreemptoff=1 ${pin[@]+"${pin[@]}"} \ "$out/perfdriver" -case "e2e/$q" -n 200000) || echo "profile.sh: perf stat failed for $q" >&2 fi { @@ -97,4 +127,44 @@ done if [ $perf_ok -eq 0 ]; then echo "\`perf stat\` is not usable on this machine, so there are no hardware counters." >>"$out/summary.md" fi + +# cachegrind_run q n: one simulated run of perfdriver; prints its counts. +cachegrind_run() { + local q=$1 n=$2 log=$out/cachegrind-$q-$n.log + (cd "$root" && env "${gcoff_env[@]}" GOMAXPROCS=1 GODEBUG=asyncpreemptoff=1 \ + valgrind --tool=cachegrind "${cg_sim[@]}" --cachegrind-out-file="$out/cachegrind-$q-$n.out" \ + "$out/perfdriver" -case "e2e/$q" -n "$n") >/dev/null 2>"$log" || return 1 + perf_cachegrind_counts "$log" +} + +if [ $cg_ok -eq 1 ]; then + { + echo "### Simulated caches (cachegrind, per operation)" + echo + echo "cmd/perfdriver -case e2e/, ${gcoff_env[*]} GOMAXPROCS=1, \`${cg_sim[*]}\`; counts are (run of $cg_n2 - run of $cg_n1) / $((cg_n2 - cg_n1)) operations. Simulated, so they do not depend on the runner CPU." + echo + echo "| query | Ir/op | I1 misses/op | I1 miss % | LLi misses/op | D1 misses/op | LLd misses/op |" + echo "|---|---:|---:|---:|---:|---:|---:|" + } >>"$out/summary.md" + for q in $queries; do + echo "profile.sh: $q cachegrind" >&2 + if ! c1=$(cachegrind_run "$q" "$cg_n1") || ! c2=$(cachegrind_run "$q" "$cg_n2"); then + echo "| $q | cachegrind failed (see cachegrind-$q-*.log) | | | | | |" >>"$out/summary.md" + continue + fi + awk -v q="$q" -v d=$((cg_n2 - cg_n1)) -v a="$c1" -v b="$c2" 'BEGIN { + split(a, x, " "); split(b, y, " ") + for (i = 1; i <= 6; i++) v[i] = (y[i] - x[i]) / d + rate = v[1] > 0 ? 100 * v[2] / v[1] : 0 + printf "| %s | %.0f | %.2f | %.4f | %.2f | %.2f | %.2f |\n", q, v[1], v[2], rate, v[3], v[5], v[6] + }' >>"$out/summary.md" + # Hottest functions by simulated L1i misses (best effort: cg_annotate's + # options differ between valgrind releases). + cg_annotate --show=Ir,I1mr,ILmr --sort=I1mr "$out/cachegrind-$q-$cg_n2.out" >"$out/cg-top-$q.txt" 2>&1 || + cg_annotate "$out/cachegrind-$q-$cg_n2.out" >"$out/cg-top-$q.txt" 2>&1 || true + done + echo >>"$out/summary.md" +else + echo "valgrind is not installed, so there is no simulated cache data." >>"$out/summary.md" +fi cat "$out/summary.md" From 53149c188b2bec6012c3808fcf4be773ac10175e Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:49:55 +0530 Subject: [PATCH 2/5] fix(perf): cachegrind runs in profile.sh never started [W0-15] The words of one `local` are expanded before its assignments, so `local q=$1 n=$2 log=...-$n.log` read an unset $n and set -u aborted every cachegrind run (the first profile dispatch reported "cachegrind failed" for all queries). Split the declaration and add an end-to-end test that runs profile.sh with a stub valgrind and checks the per-operation table. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- scripts/perf/gate/config_test.go | 64 ++++++++++++++++++++++++++++++++ scripts/perf/profile.sh | 6 ++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/scripts/perf/gate/config_test.go b/scripts/perf/gate/config_test.go index ec55accd4..d04c0cc7c 100644 --- a/scripts/perf/gate/config_test.go +++ b/scripts/perf/gate/config_test.go @@ -285,3 +285,67 @@ func TestMatrixSuiteCoversEveryBenchmarkPackage(t *testing.T) { t.Errorf("matrix suite lists packages without benchmarks: %v", extra) } } + +// TestProfileCachegrindTable runs scripts/perf/profile.sh end to end with a +// stub valgrind on PATH (macOS and most dev machines have none) and checks +// the per-operation table: the counts of the two runs are differenced and +// divided by the extra iterations, so start-up cost cancels. The stub's +// counts are 1e6 + 5000n instructions and 100 + 7n L1i misses for n +// operations. +func TestProfileCachegrindTable(t *testing.T) { + if testing.Short() { + t.Skip("builds and runs the gosqlx test binary") + } + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not installed") + } + stubDir := t.TempDir() + stub := `#!/usr/bin/env bash +n=0 out="" +for a in "$@"; do + case $prev in -n) n=$a ;; esac + case $a in --cachegrind-out-file=*) out=${a#*=} ;; --help|--version) echo valgrind-stub; exit 0 ;; esac + prev=$a +done +[ -n "$out" ] && : > "$out" +{ + echo "==1== I refs: $((1000000 + 5000 * n))" + echo "==1== I1 misses: $((100 + 7 * n))" + echo "==1== LLi misses: $((50 + n))" + echo "==1== D refs: $((400000 + 2000 * n)) (1 rd + 1 wr)" + echo "==1== D1 misses: $((300 + 3 * n)) ( 1 rd + 1 wr)" + echo "==1== LLd misses: $((200 + 2 * n)) ( 1 rd + 1 wr)" +} >&2 +` + if err := os.WriteFile(filepath.Join(stubDir, "valgrind"), []byte(stub), 0o755); err != nil { + t.Fatal(err) + } + out := t.TempDir() + root, err := filepath.Abs(repoRoot) + if err != nil { + t.Fatal(err) + } + cmd := exec.Command("bash", filepath.Join(root, "scripts", "perf", "profile.sh"), "-o", out, "-t", "1x", "-q", "join") + cmd.Dir = root + cmd.Env = append(os.Environ(), "PATH="+stubDir+string(os.PathListSeparator)+os.Getenv("PATH"), "CG_ITERS=10 110") + if b, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("profile.sh: %v\n%s", err, b) + } + summary, err := os.ReadFile(filepath.Join(out, "summary.md")) + if err != nil { + t.Fatal(err) + } + if want := "| join | 5000 | 7.00 | 0.1400 | 1.00 | 3.00 | 2.00 |"; !strings.Contains(string(summary), want) { + t.Errorf("summary lacks the cachegrind row %q:\n%s", want, summary) + } + if !strings.Contains(string(summary), "Runner CPU: `") { + t.Errorf("summary does not name the runner CPU:\n%s", summary) + } + env, err := os.ReadFile(filepath.Join(out, "env.txt")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(env), "gcoff env: GOGC=off GOMEMLIMIT=") { + t.Errorf("env.txt does not record the GOGC=off environment:\n%s", env) + } +} diff --git a/scripts/perf/profile.sh b/scripts/perf/profile.sh index 02a3ba1a8..d98a8022b 100755 --- a/scripts/perf/profile.sh +++ b/scripts/perf/profile.sh @@ -130,7 +130,10 @@ fi # cachegrind_run q n: one simulated run of perfdriver; prints its counts. cachegrind_run() { - local q=$1 n=$2 log=$out/cachegrind-$q-$n.log + # Two statements: the words of one `local` are expanded before any of + # its assignments, so $n would still be unset in log=... + local q=$1 n=$2 + local log=$out/cachegrind-$q-$n.log (cd "$root" && env "${gcoff_env[@]}" GOMAXPROCS=1 GODEBUG=asyncpreemptoff=1 \ valgrind --tool=cachegrind "${cg_sim[@]}" --cachegrind-out-file="$out/cachegrind-$q-$n.out" \ "$out/perfdriver" -case "e2e/$q" -n "$n") >/dev/null 2>"$log" || return 1 @@ -139,6 +142,7 @@ cachegrind_run() { if [ $cg_ok -eq 1 ]; then { + echo echo "### Simulated caches (cachegrind, per operation)" echo echo "cmd/perfdriver -case e2e/, ${gcoff_env[*]} GOMAXPROCS=1, \`${cg_sim[*]}\`; counts are (run of $cg_n2 - run of $cg_n1) / $((cg_n2 - cg_n1)) operations. Simulated, so they do not depend on the runner CPU." From 35a036eca8c341b77b6139fafaf6869c35ef4f7f Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:54:20 +0530 Subject: [PATCH 3/5] fix(perf): GOMEMLIMIT for bench:matrix, document the method, label matrix reports [W0-15] Review fixes after merging #917 (W0-12): - task bench:matrix sets GOMEMLIMIT=64MiB next to GOGC=off; the GOMEMLIMIT lint now understands YAML env blocks with the keys on separate lines. - docs/PERFORMANCE_METHODOLOGY.md documents GOMEMLIMIT and its effects (GC share in time and Ir/op, pool drains), the pinned PERF_GO_VERSION, the matrix and profile (cachegrind) dispatch modes and ratio-only reports. - scripts/perf/baseline keeps GOGC=off without GOMEMLIMIT on purpose (it gates absolute allocs/op and B/op, exact only without pool drains), drops an inherited GOGC/GOMEMLIMIT, and is covered by the lint through an explicit, file-restricted exemption marker. - ab.sh comment on allocs/op corrected: the limit lets GC and pool drains run; allocs/op stays exact for the gated suite only. - Matrix dispatch: job "Matrix A/B (report only)", report heading "Matrix A/B: VERDICT (report only, not a gate verdict)" via ab.sh -T and gate -report-only/-title. - PERF_GOMEMLIMIT / the gomemlimit input are validated up front (perf_check_gomemlimit) instead of every benchmark binary panicking. - ab.sh die() no longer appends the exit code to its message. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- .github/workflows/perf-gate.yml | 17 +++++-- Taskfile.yml | 5 +- docs/PERFORMANCE_METHODOLOGY.md | 21 +++++++-- scripts/perf/ab.sh | 22 +++++++-- scripts/perf/baseline/baseline_test.go | 11 +++++ scripts/perf/baseline/main.go | 27 ++++++++++- scripts/perf/gate/config_test.go | 65 ++++++++++++++++++++++++-- scripts/perf/gate/gate.go | 12 ++++- scripts/perf/gate/gate_test.go | 17 +++++++ scripts/perf/gate/main.go | 12 ++++- scripts/perf/lib.sh | 20 ++++++++ scripts/perf/profile.sh | 1 + 12 files changed, 206 insertions(+), 24 deletions(-) diff --git a/.github/workflows/perf-gate.yml b/.github/workflows/perf-gate.yml index 20625ddca..23eec74f2 100644 --- a/.github/workflows/perf-gate.yml +++ b/.github/workflows/perf-gate.yml @@ -88,7 +88,8 @@ env: jobs: gate: - name: A/B perf gate (shadow) + # A matrix dispatch is a report, not a gate verdict: name it so. + name: ${{ inputs.mode == 'matrix' && 'Matrix A/B (report only)' || 'A/B perf gate (shadow)' }} if: github.event_name != 'workflow_dispatch' || inputs.mode != 'profile' runs-on: ubuntu-latest timeout-minutes: ${{ inputs.mode == 'matrix' && 240 || 30 }} @@ -99,6 +100,11 @@ jobs: fetch-depth: 0 persist-credentials: false + - name: Validate inputs + # A GOMEMLIMIT the runtime rejects would make every benchmark binary + # panic at startup; fail here with a clear input error instead. + run: . scripts/perf/lib.sh && perf_check_gomemlimit + - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: @@ -158,7 +164,7 @@ jobs: # reports and never fails. scripts/perf/matrix-suite.sh > "$RUNNER_TEMP/matrix-suite.txt" cp "$RUNNER_TEMP/matrix-suite.txt" "$RUNNER_TEMP/perf-gate/" - flags=(-r "${ROUNDS:-5}" -t "${BENCHTIME:-100ms}" -s "$RUNNER_TEMP/matrix-suite.txt") + flags=(-r "${ROUNDS:-5}" -t "${BENCHTIME:-100ms}" -s "$RUNNER_TEMP/matrix-suite.txt" -T 'Matrix A/B') else flags=(-G -r "${ROUNDS:-10}" -t "${BENCHTIME:-200ms}") [ "$PERF_GATE_ENFORCE" = true ] && flags+=(-e) @@ -170,13 +176,18 @@ jobs: if: always() env: PR_NUMBER: ${{ github.event.pull_request.number }} + MODE: ${{ inputs.mode || 'gate' }} run: | out="$RUNNER_TEMP/perf-gate" mkdir -p "$out" if [ ! -f "$out/report.md" ]; then { echo '' - echo '## Perf gate: ERROR (shadow mode: report only)' + if [ "$MODE" = matrix ]; then + echo '## Matrix A/B: ERROR (report only, not a gate verdict)' + else + echo '## Perf gate: ERROR (shadow mode: report only)' + fi echo echo "Runner CPU: \`$(. scripts/perf/lib.sh && perf_cpu_model)\`." echo diff --git a/Taskfile.yml b/Taskfile.yml index 8cf3c1ad3..2ca83ea5b 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -203,9 +203,10 @@ tasks: bench:matrix: set: [pipefail] - desc: "Run the whole benchmark matrix once (every pkg benchmark except cbinding, which needs CGO: see bench:interop; -benchmem, -cpu 1, GOGC=off) and write it to OUT (default: .bench/matrix.txt, gitignored). Vars: BENCHTIME (default 100ms), OUT, FILTER (default .)" + desc: "Run the whole benchmark matrix once (every pkg benchmark except cbinding, which needs CGO: see bench:interop; -benchmem, -cpu 1, GOGC=off with GOMEMLIMIT=64MiB so the heap stays bounded and GC-off timings are not dominated by first-touch page faults) and write it to OUT (default: .bench/matrix.txt, gitignored). Vars: BENCHTIME (default 100ms), OUT, FILTER (default .)" env: GOGC: 'off' + GOMEMLIMIT: '64MiB' cmds: - mkdir -p "$(dirname '{{.OUT}}')" - go test -run '^$' -bench '{{.FILTER}}' -benchmem -benchtime {{.BENCHTIME}} -cpu 1 $(go list ./pkg/... ./wasm/internal/... | grep -v /cbinding) 2>&1 | tee {{.OUT}} @@ -235,7 +236,7 @@ tasks: TMP_BIN: '{{.TMPDIR | default "/tmp"}}/gosqlx-pgo.test' bench:profile: - desc: "CPU profiles (-cpu 1, GOGC=off, -cpu 4) of the gap queries plus perf stat counters when available (Vars: BENCHTIME, default 3s)" + desc: "CPU profiles (-cpu 1, GOGC=off with GOMEMLIMIT, -cpu 4) of the gap queries plus perf stat counters when available (Vars: BENCHTIME, default 3s)" cmds: - scripts/perf/profile.sh -t {{.BENCHTIME}} vars: diff --git a/docs/PERFORMANCE_METHODOLOGY.md b/docs/PERFORMANCE_METHODOLOGY.md index 600063376..3996b2e35 100644 --- a/docs/PERFORMANCE_METHODOLOGY.md +++ b/docs/PERFORMANCE_METHODOLOGY.md @@ -14,7 +14,7 @@ How GoSQLX measures performance, how a pull request is judged, and what we may c Most functionality-by-dialect cells have a benchmark; lint, security and LSP are pending (#910). List them with `go test -list Benchmark ./pkg/...`. Run the whole matrix once with: ```bash -task bench:matrix # all pkg benchmarks, -cpu 1, GOGC=off -> .bench/matrix.txt +task bench:matrix # all pkg benchmarks, -cpu 1, GOGC=off GOMEMLIMIT=64MiB -> .bench/matrix.txt task bench:matrix FILTER='^BenchmarkParseRW$' BENCHTIME=200ms ``` @@ -29,7 +29,19 @@ task bench:ab BENCH='./pkg/gosqlx=^BenchmarkParseE2E$' # one benchmark task bench:gate # exactly what CI runs, locally ``` -`scripts/perf/ab.sh` builds test binaries for both revisions with the same toolchain and runs them interleaved in ABBA order for at least 10 rounds. The primary metric uses `GOGC=off GOMAXPROCS=1` (`-cpu 1`, pinned to one CPU where `taskset` exists): garbage-collection-free mutator time has low variance and ranks queries the same way the `-cpu 4` headline does. A secondary default-GC `-cpu 1` arm (`-g`, enabled by `bench:gate`) is reported for information only; `-cpu 4` is measured only by `task bench:profile` and the competitor harness. Results go through `benchstat` at alpha 0.01. The benchmarks that are gated are listed in `scripts/perf/gate-suite.txt`. +`scripts/perf/ab.sh` builds test binaries for both revisions with the same toolchain and runs them interleaved in ABBA order for at least 10 rounds. The primary metric uses `GOGC=off GOMEMLIMIT=64MiB GOMAXPROCS=1` (`-cpu 1`, pinned to one CPU where `taskset` exists): almost garbage-collection-free mutator time has low variance and ranks queries the same way the `-cpu 4` headline does. A secondary default-GC `-cpu 1` arm (`-g`, enabled by `bench:gate`) is reported for information only; `-cpu 4` is measured only by `task bench:profile` and the competitor harness. Results go through `benchstat` at alpha 0.01. The benchmarks that are gated are listed in `scripts/perf/gate-suite.txt`. + +**Why GOMEMLIMIT.** With `GOGC=off` and no limit the heap only grows, so every allocated byte lands on a fresh page and pays a first-touch page fault and kernel zeroing. That cost is not mutator work, scales with B/op and lands on whichever benchmark first grows the heap in a process (in the amd64 checkpoint the first benchmark of a binary gained nothing from `GOGC=off` while the others gained 21-25%). With a limit the collector runs only when the heap reaches it and the pages are reused. Every `GOGC=off` measurement (`ab.sh`, `profile.sh`, `task bench:matrix`, the competitor harness) therefore also sets `GOMEMLIMIT`, default `64MiB` from `scripts/perf/lib.sh`; override it with `PERF_GOMEMLIMIT` (or the `gomemlimit` dispatch input), and `off` unsets it. Consequences: + +- Timings include a small GC share (each collection of the tiny live heap costs about 0.5 ms, about 1% of a run), and so do instructions/op: `perfdriver` and cachegrind run with the same limit, so Ir/op is no longer pure mutator work but carries a GC share proportional to B/op (about 0.25% for `join`). +- Each collection drains the `sync.Pool`s. allocs/op stays exact for the gated suite (identical in every A/A and PR run so far) but varies for large-input and low-iteration benchmarks (`ParseRW/*/large`, `Handler_LargeDocument`, `*SustainedLoad*`), which are not gated. +- `GOGC=off` timings recorded before this change are not comparable with later ones; base and head of one run always share the setting. + +`scripts/perf/baseline` (below) is the deliberate exception: it gates absolute allocs/op and B/op only, so it runs `GOGC=off` with no limit, where no collection drains a pool and the counts are exact for every benchmark. + +**Toolchain.** `perf-gate.yml` and `bench-compare.yml` build with one pinned Go release, `PERF_GO_VERSION` (1.26.9, the toolchain of the recorded baselines), with `GOTOOLCHAIN=local` so `go.mod` cannot switch it. Numbers from the gate and the competitor harness are only comparable when built by the same release. + +**Ratios only.** Hosted runners hand out different CPU models from job to job. Every artifact and report names the runner CPU, and reports compare base and head of the same job by ratio only; absolute ns/op hold for that CPU and job alone. The competitor harness summary likewise leads with GoSQLX/peer time ratios (`cmd/publish -ratios`). Tips: run on an idle machine, close other benchmarks, and re-run once if the A/A control (below) reports noise. Never weaken a test or a threshold to make a result pass. @@ -46,6 +58,8 @@ Tips: run on an idle machine, close other benchmarks, and re-run once if the A/A **A/A control.** Each run also compares the base binary against a second copy of itself. If more than 10% of benchmarks move by more than 3% in that A/A comparison, the machine is too noisy: the job re-runs once, and a second noisy result is **inconclusive**, which is neutral, not a failure. +**Other dispatch modes** of `perf-gate.yml`: `matrix` runs the same interleaved A/B over the whole benchmark matrix (`scripts/perf/matrix-suite.sh`, about 900 benchmarks, 5 rounds of 100ms by default, roughly an hour; base = head gives an A/A of the whole matrix) without the A/A arm and instruction counts. It is a report, not a gate: its job is named "Matrix A/B (report only)" and its heading says so. `profile` records pprof profiles, `perf stat` counters where the PMU allows, and cachegrind cache simulation (32 KiB 8-way L1i, the Zen 3/4/5 geometry) of the gap queries: instructions, L1i and L1d misses per operation and the functions with the most L1i misses. + **Verdicts:** `pass`, `regression`, `noisy` (re-run), `inconclusive`. The gate starts in shadow mode (it reports but never fails); enforcement is switched on after the shadow period. A maintainer can accept a known regression with the `perf-regression-accepted` label plus a written justification in the PR body. ## Baseline files @@ -71,7 +85,7 @@ Deviation from PLAN 2.7: the baseline files do not store instructions/op (Ir). I ## Profiling and PGO ```bash -task bench:profile # pprof at -cpu 1 / GOGC=off / -cpu 4 for the gap queries +task bench:profile # pprof at -cpu 1 / GOGC=off (with GOMEMLIMIT) / -cpu 4 for the gap queries task pgo:refresh # regenerate cmd/gosqlx/default.pgo from the parse benchmarks ``` @@ -93,6 +107,7 @@ Public performance statements (README, website, release notes) follow these rule ## Reference - `scripts/perf/ab.sh`, `scripts/perf/gate/`, `scripts/perf/gate-suite.txt`: A/B runner, verdict logic, gated suite +- `scripts/perf/lib.sh`: shared settings (the `GOMEMLIMIT` default, the runner CPU model) - `scripts/perf/baseline/`: baseline recorder and checker - `.github/workflows/perf-gate.yml`: CI gate; `.github/workflows/bench-compare.yml`: competitor runs - `benchmarks/compare/README.md`: competitor harness diff --git a/scripts/perf/ab.sh b/scripts/perf/ab.sh index 8b327bbbc..d761dea52 100755 --- a/scripts/perf/ab.sh +++ b/scripts/perf/ab.sh @@ -18,7 +18,12 @@ # - Optional instruction counts (-i): cmd/perfdriver under `perf stat`, # else cachegrind, else macOS `/usr/bin/time -l`, at two iteration counts; # the difference divided by the extra iterations is instructions/op. -# - allocs/op and B/op come from the primary runs (GOGC=off: no pool drains). +# - allocs/op and B/op come from the primary runs. GOMEMLIMIT lets the +# collector (and with it sync.Pool drains) run about once per limit's +# worth of allocation, so allocs/op is exact for the gated suite +# (identical in every A/A and PR run so far) but not for large-input or +# low-N benchmarks (ParseRW/*/large, Handler_LargeDocument, +# *SustainedLoad*); see the open question on the allocs rule in PR #921. # - env.txt and the report name the CPU model: hosted runners differ from # job to job, so only the in-job base/head ratios are reported. # @@ -38,12 +43,15 @@ # -G gate mode: -a -g -i, and re-measure once when the A/A is noisy # -e enforce: exit 1 on a regression (default: report only) # -A the regression is accepted (perf-regression-accepted label) +# -T TITLE report-only comparison (not a gate): heading "TITLE: VERDICT +# (report only, not a gate verdict)"; exclusive with -e # -h show this help # Environment: # PIN_CPU CPU for taskset (default: the last online CPU; "none" disables pinning) # BENCHSTAT benchstat command (default: benchstat on PATH, else the pinned version via go run) # IR_SAMPLES samples per perfdriver case, median taken (default 3, every backend) -# PERF_GOMEMLIMIT GOMEMLIMIT of the GOGC=off runs (default 64MiB; "off" unsets it) +# PERF_GOMEMLIMIT GOMEMLIMIT of the GOGC=off runs (default 64MiB; "off" unsets +# it; otherwise digits with an optional B/KiB/MiB/GiB/TiB suffix) # # Output: DIR/report.md (also printed), DIR/status (pass, regression, # inconclusive), DIR/raw/*.txt (raw benchmark output), DIR/env.txt. @@ -60,12 +68,12 @@ while IFS= read -r kv; do gcoff_env+=("$kv"); done < <(perf_gcoff_env) memlimit=$(perf_gomemlimit) usage() { sed -n '2,/^set -euo/p' "$0" | sed -e '$d' -e 's/^# \{0,1\}//'; } -die() { echo "ab.sh: $*" >&2; exit "${2:-3}"; } +die() { echo "ab.sh: $1" >&2; exit "${2:-3}"; } log() { echo "ab.sh: $*" >&2; } -out="" rounds=10 benchtime=200ms suite="" aa=0 sec=0 ir=0 gate=0 enforce=0 accepted=0 +out="" rounds=10 benchtime=200ms suite="" aa=0 sec=0 ir=0 gate=0 enforce=0 accepted=0 title="" adhoc=() -while getopts ":o:r:t:s:b:agiGeAh" opt; do +while getopts ":o:r:t:s:b:T:agiGeAh" opt; do case $opt in o) out=$OPTARG ;; r) rounds=$OPTARG ;; @@ -78,6 +86,7 @@ while getopts ":o:r:t:s:b:agiGeAh" opt; do G) gate=1 aa=1 sec=1 ir=1 ;; e) enforce=1 ;; A) accepted=1 ;; + T) title=$OPTARG ;; h) usage; exit 0 ;; :) die "-$OPTARG needs a value" 2 ;; *) die "unknown option -$OPTARG (see -h)" 2 ;; @@ -86,6 +95,8 @@ done shift $((OPTIND - 1)) [ $# -ge 1 ] && [ $# -le 2 ] || { usage >&2; exit 2; } case $rounds in '' | *[!0-9]* | 0) die "-r must be a positive integer, got '$rounds'" 2 ;; esac +[ -z "$title" ] || [ $enforce -eq 0 ] || die "-T (report only) and -e are exclusive" 2 +perf_check_gomemlimit || exit 2 root=$(git rev-parse --show-toplevel) base_ref=$1 @@ -358,6 +369,7 @@ judge() { # extra gate flags... fi [ $enforce -eq 1 ] && args+=(-enforce) [ $accepted -eq 1 ] && args+=(-accepted) + [ -n "$title" ] && args+=(-report-only -title "$title") (cd "$gate_src" && go run ./scripts/perf/gate "${args[@]}" "$@") } diff --git a/scripts/perf/baseline/baseline_test.go b/scripts/perf/baseline/baseline_test.go index 3dbaf2d46..89a18c6d2 100644 --- a/scripts/perf/baseline/baseline_test.go +++ b/scripts/perf/baseline/baseline_test.go @@ -264,3 +264,14 @@ func TestUpdateFromInputDoesNotStampLocalToolchain(t *testing.T) { t.Fatalf("external provenance not preserved: go_version=%q comment=%q", f.GoVersion, f.Comment) } } + +// TestGCOffEnvHasNoMemoryLimit: the baseline gates absolute allocs/op, which +// stay exact only when no collection drains the pools, so it runs GOGC=off +// with no GOMEMLIMIT and drops any inherited GOGC or GOMEMLIMIT. +func TestGCOffEnvHasNoMemoryLimit(t *testing.T) { + got := gcOffEnv([]string{"PATH=/bin", "GOGC=100", "GOMEMLIMIT=64MiB", "GOMEMLIMITX=keep"}) + want := []string{"PATH=/bin", "GOMEMLIMITX=keep", "GOGC=off"} + if strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("gcOffEnv = %q, want %q", got, want) + } +} diff --git a/scripts/perf/baseline/main.go b/scripts/perf/baseline/main.go index 3f4009b82..cc5a9e20d 100644 --- a/scripts/perf/baseline/main.go +++ b/scripts/perf/baseline/main.go @@ -36,7 +36,7 @@ const externalFileComment = "ns_per_op is trend-only. bytes_per_op and allocs_pe "Recorded from external benchmark output (-input); go_version is the toolchain declared with -go-version. Regenerate with: task bench:baseline:update" const fileComment = "ns_per_op is trend-only. bytes_per_op and allocs_per_op are gated by scripts/perf/baseline " + - "(allocs/op must not rise, B/op may rise at most 2 percent). Measured with GOGC=off, -cpu 1, medians. " + + "(allocs/op must not rise, B/op may rise at most 2 percent). Measured with GOGC=off and no GOMEMLIMIT (no GC, so no pool drains), -cpu 1, medians. " + "Regenerate with: task bench:baseline:update" func main() { @@ -168,6 +168,29 @@ func run(args []string, stdout, stderr io.Writer) error { return nil } +// gcOffEnv returns environ with GOGC=off and without GOGC or GOMEMLIMIT +// settings of its own. +// +// Unlike scripts/perf/ab.sh (W0-15), the baseline deliberately sets no +// GOMEMLIMIT: it gates absolute allocs/op and B/op only, and with no +// collection at all no sync.Pool is drained mid-run, so those counts are +// exact for every benchmark, large inputs and low iteration counts included. +// A memory limit lets the collector run and drain the pools, which makes +// allocs/op of large-input benchmarks vary from run to run. Its ns/op are +// trend-only and include first-touch page faults; never compare them with +// the A/B gate's timings. An inherited GOMEMLIMIT is dropped for the same +// reason. +func gcOffEnv(environ []string) []string { + env := make([]string, 0, len(environ)+1) + for _, kv := range environ { + if strings.HasPrefix(kv, "GOGC=") || strings.HasPrefix(kv, "GOMEMLIMIT=") { + continue + } + env = append(env, kv) + } + return append(env, "GOGC=off") // perf:no-gomemlimit (see above) +} + // measure runs every line of the suite file and returns the concatenated // `go test` output. func measure(suite, benchtime string, count int, log io.Writer) ([]byte, error) { @@ -190,7 +213,7 @@ func measure(suite, benchtime string, count int, log io.Writer) ([]byte, error) fmt.Fprintf(log, "baseline: %s %s\n", parts[0], parts[1]) cmd := exec.Command("go", "test", "-run", "^$", "-bench", parts[1], "-benchmem", "-benchtime", benchtime, "-count", fmt.Sprint(count), "-cpu", "1", parts[0]) - cmd.Env = append(os.Environ(), "GOGC=off") + cmd.Env = gcOffEnv(os.Environ()) cmd.Stderr = log out, err := cmd.Output() if err != nil { diff --git a/scripts/perf/gate/config_test.go b/scripts/perf/gate/config_test.go index d04c0cc7c..f9b0edb41 100644 --- a/scripts/perf/gate/config_test.go +++ b/scripts/perf/gate/config_test.go @@ -81,19 +81,35 @@ func TestPerfWorkflowsPinOneGoToolchain(t *testing.T) { // turns the collector off also bounds the heap, so first-touch page faults do // not pollute timings and profiles (C0 methodology finding 2). func TestGCOffMeasurementsSetGOMEMLIMIT(t *testing.T) { - files := []string{"scripts/perf/ab.sh", "scripts/perf/profile.sh", "Taskfile.yml"} + files := []string{"scripts/perf/ab.sh", "scripts/perf/profile.sh", "Taskfile.yml", "scripts/perf/baseline/main.go"} files = append(files, perfWorkflows...) + // The baseline recorder gates absolute allocs/op and B/op only and runs + // without a limit on purpose (no GC, so no pool drains): see gcOffEnv in + // scripts/perf/baseline. Only files listed here may carry the marker. + exempt := map[string]bool{"scripts/perf/baseline/main.go": true} + const marker = "perf:no-gomemlimit" gcoff := regexp.MustCompile(`GOGC[=:]\s*'?off`) for _, f := range files { + var lines []string sc := bufio.NewScanner(strings.NewReader(readRepoFile(t, f))) - for n := 1; sc.Scan(); n++ { - line := strings.TrimSpace(sc.Text()) + for sc.Scan() { + lines = append(lines, sc.Text()) + } + for i, raw := range lines { + n := i + 1 + line := strings.TrimSpace(raw) // Comments and Taskfile descriptions only talk about GOGC=off. - if strings.HasPrefix(line, "#") || strings.HasPrefix(line, "desc:") || !gcoff.MatchString(line) { + if strings.HasPrefix(line, "#") || strings.HasPrefix(line, "//") || strings.HasPrefix(line, "desc:") || !gcoff.MatchString(line) { + continue + } + if strings.Contains(line, marker) { + if !exempt[f] { + t.Errorf("%s:%d: %s is reserved for the baseline recorder: %s", f, n, marker, line) + } continue } // gcoff_env is the array scripts fill from perf_gcoff_env. - if !strings.Contains(line, "GOMEMLIMIT") && !strings.Contains(line, "gcoff_env") { + if !strings.Contains(line, "GOMEMLIMIT") && !strings.Contains(line, "gcoff_env") && !yamlSiblingKey(lines, i, "GOMEMLIMIT:") { t.Errorf("%s:%d sets GOGC=off without GOMEMLIMIT (use perf_gcoff_env from scripts/perf/lib.sh): %s", f, n, line) } } @@ -110,6 +126,27 @@ func TestGCOffMeasurementsSetGOMEMLIMIT(t *testing.T) { } } +// yamlSiblingKey reports whether the YAML mapping that holds lines[i] (the +// contiguous lines at its indentation) also has a key starting with key, as +// in a Taskfile or workflow env block with GOGC: 'off' and GOMEMLIMIT on +// separate lines. +func yamlSiblingKey(lines []string, i int, key string) bool { + indent := func(l string) int { return len(l) - len(strings.TrimLeft(l, " ")) } + want := indent(lines[i]) + for _, step := range []int{-1, 1} { + for j := i + step; j >= 0 && j < len(lines); j += step { + l := lines[j] + if strings.TrimSpace(l) == "" || indent(l) != want { + break + } + if strings.HasPrefix(strings.TrimSpace(l), key) { + return true + } + } + } + return false +} + // bashLib runs a snippet with scripts/perf/lib.sh sourced. func bashLib(t *testing.T, env []string, script string) string { t.Helper() @@ -152,6 +189,24 @@ func TestGCOffEnvSetsTheMemoryLimit(t *testing.T) { } } +// TestGOMEMLIMITIsValidated: a value the Go runtime rejects would make every +// benchmark binary panic at startup, so the scripts refuse it up front. +func TestGOMEMLIMITIsValidated(t *testing.T) { + for _, v := range []string{"", "off", "64MiB", "1GiB", "512KiB", "4TiB", "100B", "67108864"} { + bashLib(t, []string{"PERF_GOMEMLIMIT=" + v}, "perf_check_gomemlimit") + } + for _, v := range []string{"64Mi", "64MB", "64KB", "6.4MiB", "-1", "abc", "64 MiB", "MiB"} { + cmd := exec.Command("bash", "-c", `. "$1"; perf_check_gomemlimit`, "bash", filepath.Join(repoRoot, "scripts", "perf", "lib.sh")) + cmd.Env = append(os.Environ(), "PERF_GOMEMLIMIT="+v) + out, err := cmd.CombinedOutput() + if err == nil { + t.Errorf("PERF_GOMEMLIMIT=%q accepted", v) + } else if !strings.Contains(string(out), "PERF_GOMEMLIMIT must be") { + t.Errorf("PERF_GOMEMLIMIT=%q: no clear message: %s", v, out) + } + } +} + func TestCPUModelIsRecorded(t *testing.T) { got := strings.TrimSpace(bashLib(t, nil, "perf_cpu_model")) if got == "" || strings.HasPrefix(got, "unknown") && runtime.GOOS == "linux" { diff --git a/scripts/perf/gate/gate.go b/scripts/perf/gate/gate.go index 4e75b3318..a57e7eacc 100644 --- a/scripts/perf/gate/gate.go +++ b/scripts/perf/gate/gate.go @@ -389,7 +389,8 @@ func pval(c Cmp) string { // Meta is free-form context printed above the tables. type Meta struct { BaseRef, HeadRef string - Mode string // "shadow" or "enforce" + Mode string // "shadow", "enforce" or "report" (a comparison that is not a gate, e.g. the matrix) + Title string // report heading; default "Perf gate" Accepted bool // perf-regression-accepted label present Settings string // one line: rounds, benchtime, pinning Env string // lscpu / go version block @@ -413,7 +414,14 @@ func Markdown(res Result, cfg Config, meta Meta) string { mode = "enforced, regression accepted by the perf-regression-accepted label" } } - fmt.Fprintf(&b, "## Perf gate: %s (%s)\n\n", verdict, mode) + if meta.Mode == "report" { + mode = "report only, not a gate verdict" + } + title := meta.Title + if title == "" { + title = "Perf gate" + } + fmt.Fprintf(&b, "## %s: %s (%s)\n\n", title, verdict, mode) if meta.BaseRef != "" || meta.HeadRef != "" { fmt.Fprintf(&b, "Base `%s`, head `%s`.", meta.BaseRef, meta.HeadRef) if meta.Settings != "" { diff --git a/scripts/perf/gate/gate_test.go b/scripts/perf/gate/gate_test.go index 8b8ccf7b8..d09eff0bf 100644 --- a/scripts/perf/gate/gate_test.go +++ b/scripts/perf/gate/gate_test.go @@ -505,3 +505,20 @@ func TestReportNamesTheRunnerCPU(t *testing.T) { }) } } + +// TestReportOnlyHeadingIsNotAGateVerdict: a matrix comparison must not read +// as a gate verdict in a job summary. +func TestReportOnlyHeadingIsNotAGateVerdict(t *testing.T) { + s := suite() + res := evaluate(t, s, s, nil, DefaultConfig()) + md := Markdown(res, DefaultConfig(), Meta{Mode: "report", Title: "Matrix A/B"}) + if !strings.Contains(md, "## Matrix A/B: PASS (report only, not a gate verdict)") { + t.Fatalf("report-only heading missing:\n%s", md) + } + if strings.Contains(md, "Perf gate:") || strings.Contains(md, "shadow mode") { + t.Fatalf("report-only report reads like a gate verdict:\n%s", md) + } + if md := Markdown(res, DefaultConfig(), Meta{Mode: "shadow"}); !strings.Contains(md, "## Perf gate: PASS (shadow mode") { + t.Fatalf("default heading changed:\n%s", md) + } +} diff --git a/scripts/perf/gate/main.go b/scripts/perf/gate/main.go index aec67d034..4d08bc9d7 100644 --- a/scripts/perf/gate/main.go +++ b/scripts/perf/gate/main.go @@ -73,6 +73,8 @@ func realMain(args []string, stdout io.Writer) (int, error) { envPath = fs.String("env", "", "environment description file to embed") bstatPath = fs.String("benchstat", "", "benchstat output file to embed") irNote = fs.String("ir-note", "", "why instruction counts were not measured") + title = fs.String("title", "", "report heading (default \"Perf gate\")") + reportOnly = fs.Bool("report-only", false, "the run is a comparison, not a gate (heading says so); exclusive with -enforce") ) fs.Float64Var(&cfg.Alpha, "alpha", cfg.Alpha, "significance level") fs.Float64Var(&cfg.TimePct, "time-pct", cfg.TimePct, "per-benchmark time threshold, %") @@ -88,6 +90,9 @@ func realMain(args []string, stdout io.Writer) (int, error) { if fs.NArg() > 0 { return 0, fmt.Errorf("unexpected arguments: %v", fs.Args()) } + if *reportOnly && *enforce { + return 0, errors.New("-report-only and -enforce are exclusive") + } if *basePath == "" || *headPath == "" { return 0, errors.New("-base and -head are required") } @@ -117,9 +122,12 @@ func realMain(args []string, stdout io.Writer) (int, error) { return 0, errors.New("no benchmark results in -base or -head") } - meta := Meta{BaseRef: *baseRef, HeadRef: *headRef, Settings: *settings, Accepted: *accepted, IrNote: *irNote, Mode: "shadow"} - if *enforce { + meta := Meta{BaseRef: *baseRef, HeadRef: *headRef, Settings: *settings, Accepted: *accepted, IrNote: *irNote, Mode: "shadow", Title: *title} + switch { + case *enforce: meta.Mode = "enforce" + case *reportOnly: + meta.Mode = "report" } var err error if meta.Env, err = readOptional(*envPath); err != nil { diff --git a/scripts/perf/lib.sh b/scripts/perf/lib.sh index 30e2e3cf8..dc18df47d 100644 --- a/scripts/perf/lib.sh +++ b/scripts/perf/lib.sh @@ -25,6 +25,26 @@ perf_gomemlimit() { printf '%s\n' "${PERF_GOMEMLIMIT:-$PERF_GOMEMLIMIT_DEFAULT}" } +# perf_check_gomemlimit fails with a message on stderr unless the limit is +# "off" or a value the Go runtime accepts (digits, optional B, KiB, MiB, GiB +# or TiB suffix). A bad value would otherwise make every benchmark binary +# panic at startup. +perf_check_gomemlimit() { + local limit + limit=$(perf_gomemlimit) + case $limit in + off) return 0 ;; + '' | *[!0-9A-Za-z]*) ;; + *) + local n=$limit + case $n in *B) n=${n%B} n=${n%[KMGT]i} ;; esac + case $n in '' | *[!0-9]*) ;; *) return 0 ;; esac + ;; + esac + echo "PERF_GOMEMLIMIT must be off or digits with an optional B/KiB/MiB/GiB/TiB suffix (e.g. 64MiB), got '$limit'" >&2 + return 1 +} + # perf_gcoff_env prints the env(1) arguments of a GOGC=off measurement, one # per line: GOGC=off and GOMEMLIMIT=, or, when the limit is off, # "-u" "GOMEMLIMIT" first (env options must precede assignments) and then diff --git a/scripts/perf/profile.sh b/scripts/perf/profile.sh index d98a8022b..2e4e0ded4 100755 --- a/scripts/perf/profile.sh +++ b/scripts/perf/profile.sh @@ -30,6 +30,7 @@ set -euo pipefail # shellcheck source=scripts/perf/lib.sh . "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh" +perf_check_gomemlimit || exit 2 gcoff_env=() while IFS= read -r kv; do gcoff_env+=("$kv"); done < <(perf_gcoff_env) cpu_model=$(perf_cpu_model) From 4424940563bffeeb9b072d2a491eabcd4462511d Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:06:28 +0530 Subject: [PATCH 4/5] fix(ci): validate gomemlimit before checkout in perf-gate [W0-15] CodeQL flagged the new "Validate inputs" step (actions/cache-poisoning): it ran scripts/perf/lib.sh from the checked-out, dispatch-chosen ref. The check is now inlined and runs before checkout; a test runs the inlined block against perf_check_gomemlimit so the two accept the same values. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- .github/workflows/perf-gate.yml | 25 ++++++++++++++++----- scripts/perf/gate/config_test.go | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/.github/workflows/perf-gate.yml b/.github/workflows/perf-gate.yml index 23eec74f2..101050221 100644 --- a/.github/workflows/perf-gate.yml +++ b/.github/workflows/perf-gate.yml @@ -94,17 +94,32 @@ jobs: runs-on: ubuntu-latest timeout-minutes: ${{ inputs.mode == 'matrix' && 240 || 30 }} steps: + - name: Validate inputs + # A GOMEMLIMIT the runtime rejects would make every benchmark binary + # panic at startup; fail here with a clear input error instead. Same + # rule as perf_check_gomemlimit in scripts/perf/lib.sh (a test keeps + # them equal), inlined so it runs before any checked-out code. + env: + LIMIT: ${{ inputs.gomemlimit }} + run: | + n=$LIMIT + case $n in *B) n=${n%B} n=${n%[KMGT]i} ;; esac + case $LIMIT in + '' | off) ;; + *[!0-9A-Za-z]*) bad=1 ;; + *) case $n in '' | *[!0-9]*) bad=1 ;; esac ;; + esac + if [ -n "${bad:-}" ]; then + echo "::error::gomemlimit must be off or digits with an optional B/KiB/MiB/GiB/TiB suffix (e.g. 64MiB), got '$LIMIT'" + exit 1 + fi + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: ref: ${{ inputs.head || '' }} fetch-depth: 0 persist-credentials: false - - name: Validate inputs - # A GOMEMLIMIT the runtime rejects would make every benchmark binary - # panic at startup; fail here with a clear input error instead. - run: . scripts/perf/lib.sh && perf_check_gomemlimit - - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: diff --git a/scripts/perf/gate/config_test.go b/scripts/perf/gate/config_test.go index f9b0edb41..b0e0f8dfe 100644 --- a/scripts/perf/gate/config_test.go +++ b/scripts/perf/gate/config_test.go @@ -207,6 +207,44 @@ func TestGOMEMLIMITIsValidated(t *testing.T) { } } +// TestWorkflowGOMEMLIMITCheckMatchesLib: perf-gate.yml validates the +// gomemlimit input inline, before checkout; it must accept exactly what +// perf_check_gomemlimit accepts. +func TestWorkflowGOMEMLIMITCheckMatchesLib(t *testing.T) { + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not installed") + } + lines := strings.Split(readRepoFile(t, ".github/workflows/perf-gate.yml"), "\n") + var script []string + for i, l := range lines { + if strings.TrimSpace(l) != "- name: Validate inputs" { + continue + } + j := i + 1 + for ; j < len(lines) && strings.TrimSpace(lines[j]) != "run: |"; j++ { + } + for j++; j < len(lines) && strings.TrimSpace(lines[j]) != ""; j++ { + script = append(script, strings.TrimPrefix(lines[j], " ")) + } + break + } + if len(script) == 0 { + t.Fatal("perf-gate.yml: no \"Validate inputs\" run block") + } + lib := filepath.Join(repoRoot, "scripts", "perf", "lib.sh") + for _, v := range []string{"", "off", "64MiB", "1GiB", "100B", "67108864", "64Mi", "64MB", "6.4MiB", "-1", "abc", "64 MiB", "MiB", "B"} { + wf := exec.Command("bash", "-c", strings.Join(script, "\n")) + wf.Env = append(os.Environ(), "LIMIT="+v) + wfOK := wf.Run() == nil + lc := exec.Command("bash", "-c", `. "$1"; perf_check_gomemlimit 2>/dev/null`, "bash", lib) + lc.Env = append(os.Environ(), "PERF_GOMEMLIMIT="+v) + libOK := lc.Run() == nil + if wfOK != libOK { + t.Errorf("gomemlimit %q: workflow accepts=%v, lib.sh accepts=%v", v, wfOK, libOK) + } + } +} + func TestCPUModelIsRecorded(t *testing.T) { got := strings.TrimSpace(bashLib(t, nil, "perf_cpu_model")) if got == "" || strings.HasPrefix(got, "unknown") && runtime.GOOS == "linux" { From b1156f3775f96ef376b87efaa3cc16c7ca4ed9bb Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:32:03 +0530 Subject: [PATCH 5/5] test(perf): lint GOGC=off per shell segment, reject PiB/EiB limits [W0-15] The GOMEMLIMIT lint checked the whole line, so the one-line if/else in ab.sh run_arm let the default-GC branch's `-u GOMEMLIMIT` cover a bare GOGC=off in the gcoff branch. Check each GOGC=off token's own segment (delimited by ( ) ; | &) and pin the shapes in TestGCOffSegmentLint. Add 1PiB/1EiB to the GOMEMLIMIT validation lists so a [KMGTP] suffix mutation is caught, and point the ab.sh allocs caveat at docs/PERFORMANCE_METHODOLOGY.md instead of the PR. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- scripts/perf/ab.sh | 2 +- scripts/perf/gate/config_test.go | 53 ++++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/scripts/perf/ab.sh b/scripts/perf/ab.sh index d761dea52..a027dae03 100755 --- a/scripts/perf/ab.sh +++ b/scripts/perf/ab.sh @@ -23,7 +23,7 @@ # worth of allocation, so allocs/op is exact for the gated suite # (identical in every A/A and PR run so far) but not for large-input or # low-N benchmarks (ParseRW/*/large, Handler_LargeDocument, -# *SustainedLoad*); see the open question on the allocs rule in PR #921. +# *SustainedLoad*); docs/PERFORMANCE_METHODOLOGY.md covers this caveat. # - env.txt and the report name the CPU model: hosted runners differ from # job to job, so only the in-job base/head ratios are reported. # diff --git a/scripts/perf/gate/config_test.go b/scripts/perf/gate/config_test.go index b0e0f8dfe..3bf8364b7 100644 --- a/scripts/perf/gate/config_test.go +++ b/scripts/perf/gate/config_test.go @@ -88,7 +88,6 @@ func TestGCOffMeasurementsSetGOMEMLIMIT(t *testing.T) { // scripts/perf/baseline. Only files listed here may carry the marker. exempt := map[string]bool{"scripts/perf/baseline/main.go": true} const marker = "perf:no-gomemlimit" - gcoff := regexp.MustCompile(`GOGC[=:]\s*'?off`) for _, f := range files { var lines []string sc := bufio.NewScanner(strings.NewReader(readRepoFile(t, f))) @@ -99,7 +98,7 @@ func TestGCOffMeasurementsSetGOMEMLIMIT(t *testing.T) { n := i + 1 line := strings.TrimSpace(raw) // Comments and Taskfile descriptions only talk about GOGC=off. - if strings.HasPrefix(line, "#") || strings.HasPrefix(line, "//") || strings.HasPrefix(line, "desc:") || !gcoff.MatchString(line) { + if strings.HasPrefix(line, "#") || strings.HasPrefix(line, "//") || strings.HasPrefix(line, "desc:") || !gcoffToken.MatchString(line) { continue } if strings.Contains(line, marker) { @@ -108,8 +107,7 @@ func TestGCOffMeasurementsSetGOMEMLIMIT(t *testing.T) { } continue } - // gcoff_env is the array scripts fill from perf_gcoff_env. - if !strings.Contains(line, "GOMEMLIMIT") && !strings.Contains(line, "gcoff_env") && !yamlSiblingKey(lines, i, "GOMEMLIMIT:") { + if !gcoffSegmentsBounded(line) && !yamlSiblingKey(lines, i, "GOMEMLIMIT:") { t.Errorf("%s:%d sets GOGC=off without GOMEMLIMIT (use perf_gcoff_env from scripts/perf/lib.sh): %s", f, n, line) } } @@ -126,6 +124,49 @@ func TestGCOffMeasurementsSetGOMEMLIMIT(t *testing.T) { } } +// gcoffSegmentsBounded reports whether every GOGC=off on line sits in a +// shell segment (delimited by ( ) ; | &) that also names GOMEMLIMIT or the +// gcoff_env array scripts fill from perf_gcoff_env. Checking the segment, not +// the whole line, keeps a one-line if/else honest: a GOMEMLIMIT in one branch +// must not cover a bare GOGC=off in the other. +func gcoffSegmentsBounded(line string) bool { + const seps = "();|&" + for _, loc := range gcoffToken.FindAllStringIndex(line, -1) { + start := strings.LastIndexAny(line[:loc[0]], seps) + 1 + end := len(line) + if j := strings.IndexAny(line[loc[1]:], seps); j >= 0 { + end = loc[1] + j + } + seg := line[start:end] + if !strings.Contains(seg, "GOMEMLIMIT") && !strings.Contains(seg, "gcoff_env") { + return false + } + } + return true +} + +var gcoffToken = regexp.MustCompile(`GOGC[=:]\s*'?off`) + +// TestGCOffSegmentLint pins the lint above on the shapes it must tell apart, +// including the one-line if/else of run_arm in scripts/perf/ab.sh. +func TestGCOffSegmentLint(t *testing.T) { + for _, tc := range []struct { + line string + ok bool + }{ + {`if [ "$config" = gcoff ]; then envs=("${gcoff_env[@]}" GOMAXPROCS=1); else envs=(-u GOGC -u GOMEMLIMIT GOMAXPROCS=1); fi`, true}, + {`if [ "$config" = gcoff ]; then envs=(GOGC=off GOMAXPROCS=1); else envs=(-u GOGC -u GOMEMLIMIT GOMAXPROCS=1); fi`, false}, + {`envs=(GOMEMLIMIT=64MiB GOGC=off GOMAXPROCS=1)`, true}, + {`env GOGC=off go test ./... && env GOMEMLIMIT=64MiB go test ./...`, false}, + {`env GOGC=off GOMEMLIMIT=64MiB go test ./...`, true}, + {`GOGC: 'off'`, false}, + } { + if got := gcoffSegmentsBounded(tc.line); got != tc.ok { + t.Errorf("gcoffSegmentsBounded(%q) = %v, want %v", tc.line, got, tc.ok) + } + } +} + // yamlSiblingKey reports whether the YAML mapping that holds lines[i] (the // contiguous lines at its indentation) also has a key starting with key, as // in a Taskfile or workflow env block with GOGC: 'off' and GOMEMLIMIT on @@ -195,7 +236,7 @@ func TestGOMEMLIMITIsValidated(t *testing.T) { for _, v := range []string{"", "off", "64MiB", "1GiB", "512KiB", "4TiB", "100B", "67108864"} { bashLib(t, []string{"PERF_GOMEMLIMIT=" + v}, "perf_check_gomemlimit") } - for _, v := range []string{"64Mi", "64MB", "64KB", "6.4MiB", "-1", "abc", "64 MiB", "MiB"} { + for _, v := range []string{"64Mi", "64MB", "64KB", "6.4MiB", "-1", "abc", "64 MiB", "MiB", "1PiB", "1EiB"} { cmd := exec.Command("bash", "-c", `. "$1"; perf_check_gomemlimit`, "bash", filepath.Join(repoRoot, "scripts", "perf", "lib.sh")) cmd.Env = append(os.Environ(), "PERF_GOMEMLIMIT="+v) out, err := cmd.CombinedOutput() @@ -232,7 +273,7 @@ func TestWorkflowGOMEMLIMITCheckMatchesLib(t *testing.T) { t.Fatal("perf-gate.yml: no \"Validate inputs\" run block") } lib := filepath.Join(repoRoot, "scripts", "perf", "lib.sh") - for _, v := range []string{"", "off", "64MiB", "1GiB", "100B", "67108864", "64Mi", "64MB", "6.4MiB", "-1", "abc", "64 MiB", "MiB", "B"} { + for _, v := range []string{"", "off", "64MiB", "1GiB", "100B", "67108864", "64Mi", "64MB", "6.4MiB", "-1", "abc", "64 MiB", "MiB", "B", "4TiB", "1PiB", "1EiB"} { wf := exec.Command("bash", "-c", strings.Join(script, "\n")) wf.Env = append(os.Environ(), "LIMIT="+v) wfOK := wf.Run() == nil