Skip to content

ci(perf): GOMEMLIMIT with GOGC=off, pinned toolchain, cachegrind I1, matrix mode [W0-15] - #921

Open
ajitpratap0 wants to merge 9 commits into
mainfrom
perf/w0-15-gate-methodology
Open

ajitpratap0 wants to merge 9 commits into
mainfrom
perf/w0-15-gate-methodology

Conversation

@ajitpratap0

@ajitpratap0 ajitpratap0 commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Summary

Perf-gate methodology fixes from checkpoint C0 ("Methodology fixes", 2.4). Tooling and benchmark harness only; no library code changes.

  1. GOMEMLIMIT next to every GOGC=off measurement (default 64MiB, one definition in the new scripts/perf/lib.sh; override with PERF_GOMEMLIMIT or the new gomemlimit dispatch input, off unsets it). Applied in ab.sh (primary arms and perfdriver instruction counts), profile.sh (gcoff profiles, perf stat) and the competitor harness cmd/publish (-gomemlimit, recorded as configs[].gomemlimit, shown in command). Default-GC arms now unset GOMEMLIMIT so an inherited value cannot leak in. Why 64MiB: in the C0 A/A run, ParseE2E/simple_select, the first benchmark of its binary, gained nothing from GOGC=off while every other query gained 21-25%. That first benchmark grows the fresh heap and pays the first-touch page faults (ParseRW/postgresql/medium, the first of its line, showed ±23% spread). The limit has to sit below what one 200 ms run allocates (about 100 MB) or it changes nothing. Each collection of the tiny live heap costs about 0.5 ms (gctrace), so 64MiB keeps the collector to about 1% of a GOMAXPROCS=1 run. The CI A/A runs below compare off, 64MiB and 256MiB.

  2. cachegrind cache simulation in profile mode: --cache-sim=yes --I1=32768,8,64 --D1=32768,8,64 --LL=33554432,16,64 (the Zen 3/4/5 L1i geometry, so counts compare across runner CPUs) over cmd/perfdriver -case e2e/<q>, two iteration counts differenced. The summary shows Ir, I1 and LLi misses per op and the I1 miss rate. cg-top-<q>.txt lists functions by I1 misses. The profile job now installs valgrind.

  3. One pinned Go toolchain: perf-gate.yml and bench-compare.yml both use PERF_GO_VERSION: '1.26.9' (check-latest: false, GOTOOLCHAIN=local). The gate used to resolve 1.26.1 from go.mod.

  4. Full-matrix dispatch mode (mode=matrix): the same interleaved A/B (no A/A arm, no Ir) over every benchmark package that task bench:matrix runs. scripts/perf/matrix-suite.sh generates the list (about 900 benchmarks), so new benchmark packages join without a file edit. The defaults are 5 rounds of 100 ms, roughly an hour, with a job timeout of 240 min. It only reports. With base = head it is a whole-matrix A/A. It was cheap to add, so it is in.

  5. CPU model in every artifact, ratio-only reports: env.txt of the gate, matrix and profile gets a cpu model: line, and bench-compare gets a new env.txt. The gate report now opens with Runner CPU: ... and says only in-job ratios are compared. The profile summary names the CPU. The bench-compare job summary leads with a GoSQLX/peer time-ratio table (cmd/publish -ratios: geomean per cell × peer × config, plus the worst group). Absolute benchstat output moves into collapsed sections labelled "this job and this CPU only".

  6. W0-06 follow-up: setupTestServer (used by the existing LSP benchmarks and by W0-06's BenchmarkLSP matrix in test(bench): lint, security and LSP benchmark matrix [W0-06] #910) now writes to io.Discard instead of a growing bytes.Buffer.

  7. Review fixes after merging feat(bench): per-arch baselines, matrix/pgo tasks, methodology doc [W0-12] #917 (W0-12) (commits 35a036ec and 44249405, merge b0a3df4a):

    • task bench:matrix (added by feat(bench): per-arch baselines, matrix/pgo tasks, methodology doc [W0-12] #917) sets GOMEMLIMIT: '64MiB' next to GOGC: 'off'; its desc says why. The GOMEMLIMIT lint (TestGCOffMeasurementsSetGOMEMLIMIT) now reads YAML env blocks whose keys sit on separate lines; deleting the new line makes it fail again (mutation checked).
    • docs/PERFORMANCE_METHODOLOGY.md documents the primary metric as GOGC=off GOMEMLIMIT=64MiB GOMAXPROCS=1, why the limit is there and what it costs (GC share in time and in instructions/op, pool drains), the pinned PERF_GO_VERSION 1.26.9, the matrix and profile (cachegrind) dispatch modes, and ratio-only reporting.
    • Baseline tool decision: exempt, documented. scripts/perf/baseline keeps GOGC=off with no GOMEMLIMIT on purpose. It gates absolute allocs/op and B/op only, and those are exact for every benchmark only when no collection drains a sync.Pool; its ns/op are trend-only. It now also drops an inherited GOGC/GOMEMLIMIT (gcOffEnv, TestGCOffEnvHasNoMemoryLimit), and the file comment it writes says "no GOMEMLIMIT". The lint now scans scripts/perf/baseline/main.go too; the exemption is an explicit perf:no-gomemlimit marker that the test allows only in that file. amd64.json/arm64.json are not re-recorded (orchestrator's call).
    • scripts/perf/ab.sh comment corrected: the limit lets the collector and pool drains run, so allocs/op is exact for the gated suite but not for large-input or low-N benchmarks (documented in docs/PERFORMANCE_METHODOLOGY.md).
    • Instructions/op is no longer pure mutator work: perfdriver and cachegrind run with the limit too, so Ir/op carries a small GC share proportional to B/op (about 0.25% for join). Stated in the CHANGELOG and the methodology doc.
    • Matrix dispatch is labelled as a report: job name Matrix A/B (report only), heading Matrix A/B: VERDICT (report only, not a gate verdict) (new ab.sh -T, gate -report-only -title; -T with -e is refused). TestReportOnlyHeadingIsNotAGateVerdict.
    • PERF_GOMEMLIMIT/the gomemlimit input are validated up front (perf_check_gomemlimit in lib.sh: off or digits with an optional B/KiB/MiB/GiB/TiB suffix, the forms the runtime accepts), in ab.sh/profile.sh, and in a new "Validate inputs" workflow step. That step is inlined and runs before checkout (commit 44249405): the first version sourced lib.sh from the dispatch-chosen checkout and CodeQL flagged it as actions/cache-poisoning (high). TestGOMEMLIMITIsValidated and TestWorkflowGOMEMLIMITCheckMatchesLib (runs the inlined block against perf_check_gomemlimit for 14 values; a mutation of the workflow block is caught).
    • ab.sh's die no longer appends the exit code to its message.
  8. Second review round (commit b1156f37, merge of origin/main a9a2d60d bringing in perf(tokenizer): read the clock only when metrics are enabled [W1-02] #923):

    • The GOMEMLIMIT lint now checks each GOGC=off token's own shell segment (delimited by ( ) ; | &), not the whole line. Before, 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. TestGCOffSegmentLint pins the shapes, including that exact line.
    • 1PiB and 1EiB (suffixes the runtime rejects) are in the reject lists of TestGOMEMLIMITIsValidated and TestWorkflowGOMEMLIMITCheckMatchesLib (now 17 values).
    • The ab.sh header points the allocs/op caveat at docs/PERFORMANCE_METHODOLOGY.md instead of this PR.

Mutations checked (each makes go test ./scripts/perf/... fail, then reverted): dropping the Taskfile GOMEMLIMIT; reverting the ab.sh perfdriver env; reverting the profile.sh gcoff env; breaking the workflow validate block; dropping pkg/lsp from matrix-suite; reverting the ab.sh run_arm gcoff arm to envs=(GOGC=off GOMAXPROCS=1) (caught by TestGCOffMeasurementsSetGOMEMLIMIT at ab.sh:242 since b1156f37); widening the lib.sh suffix to [KMGTP]i (caught by both GOMEMLIMIT validation tests via 1PiB since b1156f37).

Tests (written first, each seen failing before the fix; TestProfileCachegrindTable was added after the first profile dispatch caught the local bug):

  • scripts/perf/gate/config_test.go:
    • both perf workflows pin the same PERF_GO_VERSION, with no go-version-file and check-latest: false on every setup-go;
    • no GOGC=off line in ab.sh, profile.sh, Taskfile or the perf workflows lacks GOMEMLIMIT;
    • the lib.sh default equals the cmd/publish default;
    • perf_gcoff_env output works through env(1), including off unsetting an inherited limit;
    • perf_cpu_model is non-empty;
    • perf_cachegrind_counts parses a valgrind 3.22 summary, an Ir-only summary, and fails without I refs;
    • matrix-suite.sh covers exactly the benchmark packages found by an independent walk. A mutation that drops pkg/lsp is caught.
  • gate_test.go TestReportNamesTheRunnerCPU: the cpu model: line, the lscpu fallback and unknown.
  • cmd/publish: TestEnvForGOGC (off sets GOMEMLIMIT and replaces an inherited one, default drops both), TestMethodCommandIsCopyPasteable (off configs carry the GOGC=off GOMEMLIMIT=64MiB prefix), and TestRatioMarkdown (geomean ratios, worst group, CPU line, no absolute ns).
  • pkg/lsp TestSetupTestServerDiscardsOutput.

Test Plan

Current head a9a2d60d (second review round b1156f37 + merge of origin/main d29c6446):

  • task check: pass (EXIT 0)
  • task test:cbinding: pass
  • go test -count=1 ./pkg/compatibility/...: pass
  • go run honnef.co/go/tools/cmd/staticcheck@latest ./scripts/perf/gate ./scripts/perf/baseline: clean
  • go test -count=1 ./scripts/perf/... and benchmarks/compare: go test -count=1 ./cmd/publish: pass
  • Workflow wiring at the final head (cheap dispatches):
    • Matrix mode, base = head, 1 round of 10 ms (run 41339919142, EPYC 9V74): success. Job name Matrix A/B (report only), heading ## Matrix A/B: REGRESSION (report only, not a gate verdict), Runner CPU line, go1.26.9. The REGRESSION label in this A/A comes only from "allocs/op increased in 31 benchmark(s)": at 10 ms and one round N is tiny, so the documented low-N allocs/op variance under the limit shows in more benchmarks than the 52/907 seen at 50 ms. Matrix mode is report-only; nothing fails.
    • bench-compare dispatch, count=1, benchtime 10ms (run 41339943725, EPYC 7763): both jobs success; the artifact contains env.txt (cpu model: line, go1.26.9) and ratios.md (Runner CPU line, GoSQLX/peer ratio table over the four configs).

Earlier heads:
After merging origin/main (b0a3df4a) and the review fixes (35a036ec, 44249405; gate re-run on each):

  • task check: pass (EXIT 0)
  • task test:cbinding: pass
  • go test -count=1 ./pkg/compatibility/...: pass
  • go run honnef.co/go/tools/cmd/staticcheck@latest ./scripts/perf/gate/ ./scripts/perf/baseline/: clean
  • go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 .github/workflows/perf-gate.yml .github/workflows/bench-compare.yml: clean
  • task bench:matrix FILTER='^BenchmarkParseE2E$/simple' BENCHTIME=1x: runs; scripts/perf/ab.sh -T x -e and PERF_GOMEMLIMIT=64Mi scripts/perf/{ab,profile}.sh exit 2 with a clear message.

Before the merge (head 1b970a29):

  • task check: pass
  • task test:cbinding: pass
  • go test -count=1 ./pkg/compatibility/...: pass
  • go run honnef.co/go/tools/cmd/staticcheck@latest ./scripts/perf/gate/ ./pkg/lsp/, and in benchmarks/compare: ./cmd/publish/: clean. go vet ./... and go test -race ./cmd/publish/ in benchmarks/compare: pass
  • go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 -shellcheck= .github/workflows/perf-gate.yml .github/workflows/bench-compare.yml: clean
  • Local smoke: scripts/perf/ab.sh -r 2 -t 20ms -i -b './pkg/gosqlx=^BenchmarkParseE2E$/^join$' origin/main → env.txt has cpu model: Apple M4 (arm64) and gcoff env: GOGC=off GOMEMLIMIT=64MiB, and the report opens with the Runner CPU line. scripts/perf/profile.sh -t 50ms -q join → summary names the CPU and the GOGC=off env; cachegrind is reported unavailable (no valgrind on macOS).
  • Local LSP A/B: interleaved, 8 rounds, GOGC=off, M4 shared, indicative only. W0-06's bench_matrix_test.go was added to both trees.
                                │    base     │               head                │
                                │   sec/op    │   sec/op     vs base              │
LSP/diagnostics/1KB/postgresql    21.65µ ± 7%   20.96µ ± 2%       ~ (p=0.645 n=8)
LSP/diagnostics/30KB/postgresql   792.1µ ± 2%   805.1µ ± 3%       ~ (p=0.382 n=8)
LSP/editCycle/1KB/postgresql      77.86µ ± 6%   77.68µ ± 9%       ~ (p=0.721 n=8)
LSP/editCycle/30KB/postgresql     4.403m ± 5%   4.306m ± 3%       ~ (p=0.065 n=8)
geomean                           276.9µ        274.1µ       -1.00%

                                │     base     │                head                │
                                │     B/op     │     B/op      vs base              │
LSP/diagnostics/1KB/postgresql    36.48Ki ± 1%   35.98Ki ± 0%  -1.38% (p=0.000 n=8)
LSP/diagnostics/30KB/postgresql   2.210Mi ± 0%   2.210Mi ± 0%  -0.01% (p=0.000 n=8)
LSP/editCycle/1KB/postgresql      110.5Ki ± 0%   110.1Ki ± 0%  -0.37% (p=0.000 n=8)
LSP/editCycle/30KB/postgresql     6.039Mi ± 0%   6.039Mi ± 0%  -0.01% (p=0.000 n=8)
geomean                           487.4Ki        485.2Ki       -0.45%

B/op of diagnostics/1KB drops 1.4% because the output buffer no longer grows. Time is not significant on this shared machine.

CI (amd64): perf-gate shadow verdict on this PR

Current head a9a2d60d (after the second review round and the merge of origin/main d29c6446): run 41339917373, EPYC 7763: PASS (shadow). Gated suite is now 32 benchmarks (includes #923's additions). Geomean -0.04% (p=0.631), A/A control 0 of 32 moved more than 3%, allocs/op identical for all 32, B/op unchanged, instructions/op within ±0.12% for every case. One benchmark faster (Normalize/complex -3.03%, A/A -2.39%: noise, this PR changes no library code). All checks green.

Previous head:
Head 44249405 (after the origin/main merge and review fixes): run 41337507444, merge commit da504728 on base 11f55dd7 (#917), EPYC 9V74: PASS (shadow). No gated metric moved past its threshold. Geomean +0.13% (p=0.853), A/A control 2 of 26 benchmarks moved more than 3% (under the 10% noise limit; ParseRW/sqlserver/medium +4.66% vs A/A +4.83%, ParseE2E/complex A/A +3.36%), allocs/op identical for all 26, B/op unchanged, instructions/op geomean +0.03%. All other checks are green. The first Race Detector attempt timed out in pkg/transpiler (panic: test timed out after 2m0s while TestTranspileDeepChainLinear was just starting, on a slow runner where pkg/sql/tokenizer took 112 s). The test takes 2.3 s locally under -race and this PR changes no library code. One re-run of the failed job passed.

Earlier runs: Run 41333597464 at merge commit 1b970a2, base 69a5a728, on an EPYC 9V74: PASS. No gated metric moved past its threshold. Geomean -0.19% (p=0.25), A/A control 0 of 26 benchmarks moved more than 3%, allocs/op unchanged everywhere, and the instructions/op table is clean. That is the expected result, because the PR changes no library code. The first commit's PR run, 41323785943 on an EPYC 9V45, was also PASS (geomean +0.10%, A/A 0 of 26). Both reports open with the new Runner CPU line and record GOMEMLIMIT 64MiB in the settings.

A/A dispatch runs (proof)

GOMEMLIMIT choice: gate mode A/A (base = head = 0c4f20d0, 10 rounds, 200 ms). The runners are different CPUs, so I compare the in-job ratio of each benchmark's GOGC=off time to its default-GC time. The first benchmark of each binary should look like the others if page faults no longer pollute it.

run CPU GOMEMLIMIT ParseE2E/simple_select gcoff/gcon (rest of ParseE2E) ParseRW/postgresql/medium gcoff/gcon (rest of ParseRW) mean CV of gcoff samples A/A >3%
41323691165 EPYC 9V45 unset (old behaviour) 0.985 (0.74-0.82) 0.883 (0.70-0.78) 3.3% 1 of 26
41323693616 Xeon 8573C 64MiB 0.868 (0.77-0.85) 0.744 (0.71-0.77) 1.6% 0 of 26
41323696807 EPYC 7763 256MiB 0.934 (0.73-0.79) 0.818 (0.70-0.73) 1.2% 0 of 26
  • With no limit, the first benchmark reproduces the C0 pollution: it is 21-26% slower than its siblings relative to default GC.
  • 64MiB removes it for ParseRW. For simple_select it leaves +6%: that benchmark's calibration runs still fault in the first ~50 MB.
  • 256MiB, which sits above what a 200 ms run allocates, removes only about a third of the pollution.
  • CVs come from three different CPU models and are indicative only.
  • Cost of 64MiB: the collector now runs about every 50 MB of allocation, about 0.5 ms of CPU per cycle on M4 gctrace. The GOGC=off metric therefore carries a small (~1%) GC share proportional to B/op.

Full matrix: GOGC=off without a limit cannot run at all. Matrix-mode A/A run 41330996082 used gomemlimit=off, 2 rounds of 50 ms. The runner was killed with "The runner has received a shutdown signal" (exit 143) 2.7 min into round 1, twice (attempt 1 and the rerun). The behaviour is consistent with memory exhaustion: the heap grows without bound across the large-input benchmarks. The same matrix with 64MiB completed twice: runs 41323701845 and 41330998379, 907 benchmarks, 693 s for 2 rounds of 50 ms, on EPYC 7763. The default 5 rounds of 100 ms should take about 45-60 min, inside the 240 min timeout.

Matrix A/A finding: the matrix report says "allocs/op increased in 4 benchmarks" in an A/A. allocs/op is not exactly deterministic for large-input, low-N benchmarks: ParseRW/*/large (e.g. 11154 vs 11156), Handler_LargeDocument, the *SustainedLoad* and TokenizerThroughputScaling benchmarks. 52 of 907 benchmarks varied. Matrix mode only reports, so nothing fails. The gated suite's allocs/op were identical in every A/A and PR run above. I could not get a no-limit matrix baseline (see above), so I cannot tell how much of this comes from GC under the limit draining pools. Listed as an open question.

Profile mode (run 41330929067 at 53149c18, EPYC 9V74). The first profile dispatch (41323699342) exposed a bug: a local expansion bug in profile.sh stopped every cachegrind run. The fix is in the second commit, with an end-to-end test that drives profile.sh through a stub valgrind.

Simulated caches, per operation (32 KiB 8-way L1i/L1d, 32 MiB LL):

query Ir/op I1 misses/op I1 miss % LLi misses/op D1 misses/op
join 58666 1224 2.09 0 153
aggregate 40192 1052 2.62 0 99
complex 155265 1837 1.18 0 765
insert 28385 540 1.90 0 23
  • cg-top-complex.txt: parser.go functions take 16.4% of all I1 misses (parsePrimaryExpression 2.3%, parseFunctionCall 2.0%).
  • The GOGC=off profiles now show memclrNoHeapPointers at 2.8% (join) and 1.3% (complex), and heapSetTypeSmallHeader is out of the top 40. C0's GOGC=off profile had 5.6% + 5.9% (join) and 5.0% + 3.2% (complex).

Toolchain: every run above logs go version go1.26.9 linux/amd64.

Breaking Changes

None to the library. For the perf tooling:

  • GOGC=off timings from the gate, bench:ab and the competitor harness are not comparable with runs made before this change. Base and head of one run always share the setting.
  • cmd/publish's internal envForGOGC/shellCommand gained a memory-limit argument.
  • Results JSON configs[] gains an optional gomemlimit field (additive).

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

…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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@vercel

vercel Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
gosqlx-website Ready Ready Preview Nov 2, 2026 9:14am UTC

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 43s —— View job


Review of PR #921: perf-gate methodology (W0-15)

I read the diff for scripts/perf/{lib.sh,ab.sh,profile.sh,matrix-suite.sh}, scripts/perf/gate/gate.go and pkg/lsp/handler_bench_test.go. I did not run anything. I did not read the workflow YAML, cmd/publish or the new tests in depth, so this review does not cover them.

Overall: the change is well structured. Putting the GOMEMLIMIT default in one lib.sh and tying it to the cmd/publish default with a test is the right approach. The rationale comments are clear and the changes stay inside the tooling.

Things I checked that look correct

  • env(1) option ordering: perf_gcoff_env puts -u GOMEMLIMIT before the GOGC=off assignment when the limit is off. env -u options have to come before assignments, so this is right.
  • bash 3.2 compatibility: the while read loop avoids mapfile, so it works on macOS.
  • perf_cachegrind_counts: the column order (Ir, I1mr, ILmr, Dr, D1mr, DLmr) matches the awk indices in profile.sh: v[1], v[2], v[3], v[5], v[6]. The [[:space:]] prefix before I keeps it from matching the ==pid== prefix.
  • Cachegrind differencing: subtracting the 200-iteration run from the 2200-iteration run and dividing by 2000 removes start-up cost. The CG_ITERS validation is sound.
  • LSP benchmarks: switching setupTestServer to io.Discard removes buffer-growth cost that scaled with b.N. The B/op drop in your numbers matches that.

Suggestions

  1. Memory limit versus live heap (scripts/perf/lib.sh). A fixed 64MiB limit works for the tiny live heaps of the parse benchmarks. In mode=matrix, which runs about 900 benchmarks, some may hold a live heap near or above 64MiB. The 30KB LSP documents and the large-input cases are the likeliest. There the runtime collects continuously. It caps GC at about 50% of CPU, so the limit is far from free, and the measurement then reflects GC cost. That cost would be the same for base and head, so ratios would stay valid but be diluted or noisy. Consider one of these:
    • printing a warning when a run's heap approaches the limit, or
    • letting matrix mode default PERF_GOMEMLIMIT higher, for example 256MiB, which your A/A runs already compare.
  2. Cachegrind under Go (profile.sh).
    • Cachegrind on Go binaries can be flaky. It runs 2 × queries × 2 valgrind runs, each about 50× slower, so the profile job's wall time could grow noticeably. Consider logging the time per run, or a timeout.
    • The cachegrind_run failure path is handled, which is good.
    • The cg_annotate fallback hides option differences between valgrind versions behind || true. That is fine, but the cg-top-*.txt file may then hold an error message instead of a listing. Consider checking for that.
  3. matrix-suite.sh.
    • It matches ^func Benchmark…\( only in *_test.go. It would miss benchmarks defined with different formatting, such as a type-parameterised declaration or an Example-style name, though that is unlikely in practice.
    • It also hardcodes the pkg and wasm/internal roots. Your independent-walk test catches drift inside those roots, but not a new top-level benchmark directory.
  4. runnerCPU (gate.go). If env.txt contains both a cpu model: line and an lscpu block, it correctly prefers the first. In profile.sh the lscpu dump also gets appended, so this works, but the order of the two prefixes is what makes it work. A one-line comment would protect it.
  5. TestSetupTestServerDiscardsOutput. It checks server.writer == io.Discard against the implementation's field name. It is fine as a guard, but it breaks on an unrelated rename of that field. A behavioural check would be sturdier, for example confirming that validateDocument retains no output.
  6. Comparability note. The PR description already says GOGC=off timings before and after this change are not comparable. Please also mention it in the CHANGELOG entry if it isn't there already. I did not check.

Test coverage

Coverage of the shell helpers through Go tests is strong. Testing perf_gcoff_env through real env(1), the matrix-suite.sh mutation check, and the workflow pin checks give good protection against drift. I'd like to see the pending CI A/A results (off, 64MiB, 256MiB) before merge to confirm that 64MiB removes the first-benchmark penalty.

No blocking issues found.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Perf gate: PASS (shadow mode: report only, this check never fails)

Base 68a04b41e6a2, head 7290d254d239. 10 interleaved rounds, benchtime 200ms, -cpu 1, GOMEMLIMIT 64MiB, pinned to CPU 3 (SMT siblings 2-3) with taskset.

Runner CPU: AMD EPYC 7763 64-Core Processor (x86_64). 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.

  • no gated metric moved past its threshold

Rules: time regresses at >= +3% with p < 0.01 (Mann-Whitney U), or a suite geomean >= +1.5% (per-round geomeans, same test); allocs/op must not increase; B/op must not rise more than 2%; instructions/op must not rise more than 1.5%. An A/A control with more than 10% of benchmarks moving over 3% is noisy: re-run once, then inconclusive (neutral).

Gated suite (GOGC=off, GOMAXPROCS=1)

benchmark time p A/A B/op allocs/op status
ParseE2E/simple_select -0.33% 0.344 -0.21% +0.00% 5 → 5 ok
ParseE2E/select_where +1.89% 0.038 +0.10% +0.00% 18 → 18 ok
ParseE2E/join -0.67% 0.384 -1.22% +0.00% 17 → 17 ok
ParseE2E/aggregate +0.28% 0.496 -0.12% +0.00% 18 → 18 ok
ParseE2E/subquery +0.89% 0.940 -0.21% +0.00% 14 → 14 ok
ParseE2E/cte +0.44% 0.853 +1.65% +0.00% 21 → 21 ok
ParseE2E/window -0.39% 0.186 -0.51% +0.00% 11 → 11 ok
ParseE2E/complex -1.07% 0.007 -0.80% +0.00% 50 → 50 ok
ParseE2E/insert +1.07% 0.571 -1.00% +0.00% 17 → 17 ok
ParseE2E/update +0.22% 0.880 -0.49% +0.00% 13 → 13 ok
ParseE2E/delete +0.72% 0.290 +0.29% +0.00% 5 → 5 ok
ParseRW/postgresql/medium -3.14% 0.247 -3.49% -0.00% 648 → 648 ok
ParseRW/mysql/medium -0.65% 0.912 -1.50% +0.00% 368 → 368 ok
ParseRW/mariadb/medium +1.39% 0.105 +1.35% +0.00% 530 → 530 ok
ParseRW/sqlserver/medium +0.05% 0.529 +0.52% +0.00% 937 → 937 ok
ParseRW/oracle/medium -0.26% 0.912 -0.44% +0.00% 1094 → 1094 ok
ParseRW/sqlite/medium -1.31% 0.315 -0.10% +0.00% 348 → 348 ok
ParseRW/snowflake/medium -0.48% 0.579 -3.53% +0.00% 779 → 779 ok
ParseRW/clickhouse/medium +0.85% 0.684 +1.31% +0.00% 384 → 384 ok
ParseFresh/simple_select -0.28% 0.031 -0.23% +0.00% 6 → 6 ok
ParseFresh/join -0.46% 0.190 -0.34% +0.00% 18 → 18 ok
ParseFresh/complex -0.92% 0.004 -0.23% +0.00% 51 → 51 ok
PoolGetPut/serial -0.20% 0.520 -0.49% +0.00% 0 → 0 ok
Tokenize/scratch/join -0.21% 0.224 -0.17% +0.00% 0 → 0 ok
Tokenize/scratch/complex +0.00% 0.623 +0.01% +0.00% 0 → 0 ok
Tokenize/scratch/insert -0.08% 0.147 -0.08% +0.00% 0 → 0 ok
Normalize/simple_select -0.58% 0.212 -0.70% +0.00% 10 → 10 ok
Normalize/join +2.78% <0.001 -1.55% +0.00% 40 → 40 ok
Normalize/complex +0.39% 0.315 +0.15% +0.00% 110 → 110 ok
TransformParse/simple_select -0.09% 0.705 +0.21% +0.00% 5 → 5 ok
TransformParse/join -0.10% 0.481 -0.05% +0.00% 17 → 17 ok
TransformParse/complex -0.43% 0.045 -0.27% +0.00% 50 → 50 ok
geomean -0.03% 0.853 -0.38%

A/A control: 2 of 32 benchmarks moved more than 3%.

Instructions per operation (cmd/perfdriver)

case instructions/op status
e2e/aggregate +0.00% ok
e2e/complex -0.03% ok
e2e/cte +0.00% ok
e2e/delete -0.08% ok
e2e/insert -0.01% ok
e2e/join +0.02% ok
e2e/select_where -0.02% ok
e2e/simple_select +0.02% ok
e2e/subquery -0.03% ok
e2e/update +0.03% ok
e2e/window -0.06% ok
rw/postgresql/medium +0.06% ok
Secondary metric: default GC, GOMAXPROCS=1 (informational)
benchmark time p
ParseE2E/simple_select +0.33% 0.495
ParseE2E/select_where +0.40% 0.393
ParseE2E/join +0.27% 0.353
ParseE2E/aggregate +0.32% 0.623
ParseE2E/subquery -0.04% 0.880
ParseE2E/cte -0.06% 0.970
ParseE2E/window -0.73% 0.280
ParseE2E/complex -0.16% 0.970
ParseE2E/insert +0.50% 0.307
ParseE2E/update -0.61% 0.405
ParseE2E/delete -0.63% 0.820
ParseRW/postgresql/medium -0.41% 0.123
ParseRW/mysql/medium -0.45% 0.436
ParseRW/mariadb/medium -0.28% 0.218
ParseRW/sqlserver/medium -0.68% 0.005
ParseRW/oracle/medium -0.12% 0.971
ParseRW/sqlite/medium +0.13% 0.631
ParseRW/snowflake/medium -0.11% 0.971
ParseRW/clickhouse/medium -0.23% 0.684
ParseFresh/simple_select -0.35% 0.315
ParseFresh/join -0.34% 0.912
ParseFresh/complex -0.95% 0.015
PoolGetPut/serial +0.13% 0.363
Tokenize/scratch/join -0.08% 0.791
Tokenize/scratch/complex +0.14% 0.734
Tokenize/scratch/insert +0.04% 0.789
Normalize/simple_select +0.08% 0.820
Normalize/join +0.89% 0.015
Normalize/complex +0.12% 0.853
TransformParse/simple_select -0.36% 0.449
TransformParse/join +0.35% 0.143
TransformParse/complex -0.82% 0.052
geomean -0.12%
Environment
date:        2026-11-02T09:16:12Z
base:        68a04b41e6a23b15ba97104a1f29aa5452ce98b1
head:        working tree of 7290d254d2397a06576123d4ed8150986f9d94ca
cpu model:   AMD EPYC 7763 64-Core Processor (x86_64)
go:          go version go1.26.9 linux/amd64
kernel:      Linux 7.0.0-1017-azure x86_64
pinning:     pinned to CPU 3 (SMT siblings 2-3) with taskset
gcoff env:   GOGC=off GOMEMLIMIT=64MiB
clocksource: tsc
load:        1.37, 0.38, 0.13
instr count: cachegrind
perf probe:  Error: No supported events found. The instructions:u event is not supported. 

Architecture:                            x86_64
CPU(s):                                  4
Model name:                              AMD EPYC 7763 64-Core Processor
Thread(s) per core:                      2
Core(s) per socket:                      2
Socket(s):                               1
Hypervisor vendor:                       Microsoft
L1d cache:                               64 KiB (2 instances)
L1i cache:                               64 KiB (2 instances)
L2 cache:                                1 MiB (2 instances)
L3 cache:                                32 MiB (1 instance)
benchstat (primary metric)
goos: linux
goarch: amd64
pkg: github.com/ajitpratap0/GoSQLX/pkg/fingerprint
cpu: AMD EPYC 7763 64-Core Processor                
                        │    base     │                head                │
                        │   sec/op    │   sec/op     vs base               │
Normalize/simple_select   2.860µ ± 1%   2.844µ ± 1%       ~ (p=0.217 n=10)
Normalize/join            10.51µ ± 3%   10.80µ ± 1%  +2.78% (p=0.000 n=10)
Normalize/complex         27.35µ ± 3%   27.45µ ± 4%       ~ (p=0.315 n=10)
geomean                   9.368µ        9.448µ       +0.85%

                        │     base     │                 head                  │
                        │     B/op     │     B/op      vs base                 │
Normalize/simple_select   1.016Ki ± 0%   1.016Ki ± 0%       ~ (p=1.000 n=10) ¹
Normalize/join            3.234Ki ± 0%   3.234Ki ± 0%       ~ (p=1.000 n=10) ¹
Normalize/complex         8.945Ki ± 0%   8.945Ki ± 0%       ~ (p=1.000 n=10) ¹
geomean                   3.086Ki        3.086Ki       +0.00%
¹ all samples are equal

                        │    base    │                head                 │
                        │ allocs/op  │ allocs/op   vs base                 │
Normalize/simple_select   10.00 ± 0%   10.00 ± 0%       ~ (p=1.000 n=10) ¹
Normalize/join            40.00 ± 0%   40.00 ± 0%       ~ (p=1.000 n=10) ¹
Normalize/complex         110.0 ± 0%   110.0 ± 0%       ~ (p=1.000 n=10) ¹
geomean                   35.30        35.30       +0.00%
¹ all samples are equal

pkg: github.com/ajitpratap0/GoSQLX/pkg/gosqlx
                          │    base     │                head                │
                          │   sec/op    │   sec/op     vs base               │
ParseE2E/simple_select      1.676µ ± 1%   1.671µ ± 1%       ~ (p=0.342 n=10)
ParseE2E/select_where       3.966µ ± 2%   4.041µ ± 2%       ~ (p=0.034 n=10)
ParseE2E/join               5.993µ ± 2%   5.953µ ± 1%       ~ (p=0.381 n=10)
ParseE2E/aggregate          4.332µ ± 2%   4.344µ ± 1%       ~ (p=0.493 n=10)
ParseE2E/subquery           3.801µ ± 2%   3.835µ ± 5%       ~ (p=0.928 n=10)
ParseE2E/cte                5.286µ ± 3%   5.309µ ± 2%       ~ (p=0.853 n=10)
ParseE2E/window             3.458µ ± 1%   3.445µ ± 2%       ~ (p=0.184 n=10)
ParseE2E/complex            16.34µ ± 1%   16.16µ ± 2%  -1.07% (p=0.007 n=10)
ParseE2E/insert             2.657µ ± 3%   2.685µ ± 3%       ~ (p=0.565 n=10)
ParseE2E/update             2.463µ ± 5%   2.469µ ± 5%       ~ (p=0.868 n=10)
ParseE2E/delete             1.528µ ± 2%   1.539µ ± 1%       ~ (p=0.287 n=10)
ParseRW/postgresql/medium   186.6µ ± 4%   180.8µ ± 3%       ~ (p=0.247 n=10)
ParseRW/mysql/medium        114.3µ ± 2%   113.5µ ± 1%       ~ (p=0.912 n=10)
ParseRW/mariadb/medium      166.3µ ± 3%   168.7µ ± 3%       ~ (p=0.105 n=10)
ParseRW/sqlserver/medium    350.9µ ± 5%   351.1µ ± 4%       ~ (p=0.529 n=10)
ParseRW/oracle/medium       410.0µ ± 1%   408.9µ ± 5%       ~ (p=0.912 n=10)
ParseRW/sqlite/medium       92.12µ ± 3%   90.92µ ± 4%       ~ (p=0.315 n=10)
ParseRW/snowflake/medium    243.9µ ± 3%   242.7µ ± 4%       ~ (p=0.579 n=10)
ParseRW/clickhouse/medium   107.8µ ± 2%   108.7µ ± 3%       ~ (p=0.684 n=10)
ParseFresh/simple_select    10.09µ ± 1%   10.06µ ± 1%       ~ (p=0.027 n=10)
ParseFresh/join             20.54µ ± 1%   20.44µ ± 1%       ~ (p=0.190 n=10)
ParseFresh/complex          36.60µ ± 4%   36.26µ ± 0%  -0.92% (p=0.004 n=10)
PoolGetPut/serial           66.97n ± 1%   66.84n ± 1%       ~ (p=0.515 n=10)
geomean                     15.06µ        15.04µ       -0.11%

                          │     base     │                head                 │
                          │     B/s      │     B/s       vs base               │
ParseE2E/simple_select      14.79Mi ± 1%   14.84Mi ± 1%       ~ (p=0.342 n=10)
ParseE2E/select_where       24.77Mi ± 2%   24.31Mi ± 2%       ~ (p=0.034 n=10)
ParseE2E/join               20.06Mi ± 2%   20.18Mi ± 1%       ~ (p=0.361 n=10)
ParseE2E/aggregate          18.50Mi ± 2%   18.44Mi ± 1%       ~ (p=0.493 n=10)
ParseE2E/subquery           20.58Mi ± 2%   20.39Mi ± 6%       ~ (p=0.928 n=10)
ParseE2E/cte                22.37Mi ± 3%   22.27Mi ± 2%       ~ (p=0.813 n=10)
ParseE2E/window             25.93Mi ± 1%   26.03Mi ± 2%       ~ (p=0.183 n=10)
ParseE2E/complex            22.06Mi ± 1%   22.31Mi ± 2%  +1.10% (p=0.006 n=10)
ParseE2E/insert             27.28Mi ± 3%   26.99Mi ± 3%       ~ (p=0.565 n=10)
ParseE2E/update             21.29Mi ± 5%   21.25Mi ± 5%       ~ (p=0.868 n=10)
ParseE2E/delete             32.46Mi ± 2%   32.22Mi ± 1%       ~ (p=0.280 n=10)
ParseRW/postgresql/medium   19.23Mi ± 4%   19.85Mi ± 3%       ~ (p=0.247 n=10)
ParseRW/mysql/medium        23.68Mi ± 2%   23.84Mi ± 1%       ~ (p=0.928 n=10)
ParseRW/mariadb/medium      21.32Mi ± 3%   21.02Mi ± 3%       ~ (p=0.093 n=10)
ParseRW/sqlserver/medium    18.96Mi ± 5%   18.95Mi ± 4%       ~ (p=0.529 n=10)
ParseRW/oracle/medium       20.44Mi ± 1%   20.50Mi ± 4%       ~ (p=0.926 n=10)
ParseRW/sqlite/medium       22.42Mi ± 3%   22.72Mi ± 4%       ~ (p=0.324 n=10)
ParseRW/snowflake/medium    20.84Mi ± 3%   20.95Mi ± 4%       ~ (p=0.565 n=10)
ParseRW/clickhouse/medium   20.90Mi ± 2%   20.72Mi ± 3%       ~ (p=0.697 n=10)
geomean                     21.70Mi        21.71Mi       +0.03%

                          │      base      │                 head                  │
                          │      B/op      │     B/op      vs base                 │
ParseE2E/simple_select        816.0 ± 0%       816.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/select_where       1.523Ki ± 0%     1.523Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/join               2.305Ki ± 0%     2.305Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/aggregate          1.812Ki ± 0%     1.812Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/subquery           1.828Ki ± 0%     1.828Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/cte                2.336Ki ± 0%     2.336Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/window             1.281Ki ± 0%     1.281Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/complex            6.399Ki ± 0%     6.399Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/insert               824.0 ± 0%       824.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/update               736.0 ± 0%       736.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/delete               400.0 ± 0%       400.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/postgresql/medium   70.57Ki ± 0%     70.57Ki ± 0%       ~ (p=0.132 n=10)
ParseRW/mysql/medium        43.77Ki ± 0%     43.77Ki ± 0%       ~ (p=1.000 n=10)
ParseRW/mariadb/medium      64.99Ki ± 0%     64.99Ki ± 0%       ~ (p=0.787 n=10)
ParseRW/sqlserver/medium    90.64Ki ± 0%     90.64Ki ± 0%       ~ (p=0.910 n=10)
ParseRW/oracle/medium       113.8Ki ± 0%     113.8Ki ± 0%       ~ (p=0.178 n=10)
ParseRW/sqlite/medium       39.94Ki ± 0%     39.94Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/snowflake/medium    83.42Ki ± 0%     83.42Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/clickhouse/medium   37.13Ki ± 0%     37.13Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/simple_select      896.0 ± 0%       896.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/join             2.383Ki ± 0%     2.383Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/complex          6.477Ki ± 0%     6.477Ki ± 0%       ~ (p=1.000 n=10) ¹
PoolGetPut/serial             0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                  ²                 -0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                          │     base      │                 head                 │
                          │   allocs/op   │  allocs/op   vs base                 │
ParseE2E/simple_select       5.000 ± 0%      5.000 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/select_where        18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/join                17.00 ± 0%      17.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/aggregate           18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/subquery            14.00 ± 0%      14.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/cte                 21.00 ± 0%      21.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/window              11.00 ± 0%      11.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/complex             50.00 ± 0%      50.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/insert              17.00 ± 0%      17.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/update              13.00 ± 0%      13.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/delete              5.000 ± 0%      5.000 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/postgresql/medium    648.0 ± 0%      648.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/mysql/medium         368.0 ± 0%      368.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/mariadb/medium       530.0 ± 0%      530.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/sqlserver/medium     937.0 ± 0%      937.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/oracle/medium       1.094k ± 0%     1.094k ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/sqlite/medium        348.0 ± 0%      348.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/snowflake/medium     779.0 ± 0%      779.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/clickhouse/medium    384.0 ± 0%      384.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/simple_select     6.000 ± 0%      6.000 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/join              18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/complex           51.00 ± 0%      51.00 ± 0%       ~ (p=1.000 n=10) ¹
PoolGetPut/serial            0.000 ± 0%      0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                 ²                +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                          │    base     │                head                │
                          │  sec/stmt   │  sec/stmt    vs base               │
ParseRW/postgresql/medium   15.55µ ± 4%   15.07µ ± 3%       ~ (p=0.247 n=10)
ParseRW/mysql/medium        14.28µ ± 2%   14.19µ ± 1%       ~ (p=0.912 n=10)
ParseRW/mariadb/medium      18.48µ ± 3%   18.74µ ± 3%       ~ (p=0.105 n=10)
ParseRW/sqlserver/medium    16.71µ ± 4%   16.72µ ± 4%       ~ (p=0.529 n=10)
ParseRW/oracle/medium       17.08µ ± 1%   17.04µ ± 5%       ~ (p=0.912 n=10)
ParseRW/sqlite/medium       15.35µ ± 3%   15.15µ ± 4%       ~ (p=0.315 n=10)
ParseRW/snowflake/medium    18.76µ ± 3%   18.67µ ± 4%       ~ (p=0.579 n=10)
ParseRW/clickhouse/medium   15.40µ ± 2%   15.54µ ± 3%       ~ (p=0.669 n=10)
geomean                     16.39µ        16.31µ       -0.45%

pkg: github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer
                         │    base     │                head                │
                         │   sec/op    │   sec/op     vs base               │
Tokenize/scratch/join      2.612µ ± 1%   2.606µ ± 1%       ~ (p=0.224 n=10)
Tokenize/scratch/complex   7.092µ ± 1%   7.091µ ± 1%       ~ (p=0.616 n=10)
Tokenize/scratch/insert    1.257µ ± 1%   1.256µ ± 0%       ~ (p=0.145 n=10)
geomean                    2.855µ        2.852µ       -0.10%

                         │     base     │                head                 │
                         │     B/s      │     B/s       vs base               │
Tokenize/scratch/join      46.01Mi ± 1%   46.11Mi ± 1%       ~ (p=0.270 n=10)
Tokenize/scratch/complex   50.83Mi ± 1%   50.84Mi ± 1%       ~ (p=0.617 n=10)
Tokenize/scratch/insert    57.68Mi ± 1%   57.73Mi ± 0%       ~ (p=0.239 n=10)
geomean                    51.29Mi        51.34Mi       +0.10%

                         │     base     │                head                 │
                         │     B/op     │    B/op     vs base                 │
Tokenize/scratch/join      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/complex   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/insert    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                               ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                         │     base     │                head                 │
                         │  allocs/op   │ allocs/op   vs base                 │
Tokenize/scratch/join      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/complex   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/insert    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                               ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

pkg: github.com/ajitpratap0/GoSQLX/pkg/transform
                             │    base     │                head                │
                             │   sec/op    │   sec/op     vs base               │
TransformParse/simple_select   1.666µ ± 1%   1.665µ ± 1%       ~ (p=0.699 n=10)
TransformParse/join            6.341µ ± 1%   6.335µ ± 3%       ~ (p=0.481 n=10)
TransformParse/complex         16.37µ ± 4%   16.30µ ± 1%       ~ (p=0.041 n=10)
geomean                        5.571µ        5.560µ       -0.21%

                             │     base     │                 head                  │
                             │     B/op     │     B/op      vs base                 │
TransformParse/simple_select     816.0 ± 0%     816.0 ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/join            2.305Ki ± 0%   2.305Ki ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/complex         6.398Ki ± 0%   6.398Ki ± 0%       ~ (p=1.000 n=10) ¹
geomean                        2.273Ki        2.273Ki       +0.00%
¹ all samples are equal

                             │    base    │                head                 │
                             │ allocs/op  │ allocs/op   vs base                 │
TransformParse/simple_select   5.000 ± 0%   5.000 ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/join            17.00 ± 0%   17.00 ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/complex         50.00 ± 0%   50.00 ± 0%       ~ (p=1.000 n=10) ¹
geomean                        16.20        16.20       +0.00%
¹ all samples are equal

Run, raw data and benchstat

ajitpratap0 and others added 2 commits November 1, 2026 22:49
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 21s —— View job


Review of PR #921: perf-gate methodology (W0-15)

I read lib.sh, matrix-suite.sh, and the diffs of gate.go and pkg/lsp/handler_bench_test.go. I did not run anything and did not read the workflow YAML, cmd/publish or profile.sh this round. The earlier review's points on those files still stand.

Overall: the change is sound and stays inside the tooling. The CI perf-gate shadow run passes. With GOMEMLIMIT 64MiB, the A/A control shows 0 of 26 benchmarks moving more than 3%. ParseE2E/simple_select A/A is -0.40%, so the first-benchmark penalty looks gone on that run. I found no blocking issues.

Checked and correct

  • perf_gcoff_env: -u GOMEMLIMIT comes before the GOGC=off assignment, which env(1) requires. It also has no mapfile, so it works on bash 3.2.
  • perf_cachegrind_counts: the [[:space:]] prefix avoids matching the ==pid== prefix. Missing counters default to 0, and a missing I refs fails.
  • matrix-suite.sh: it is set -euo pipefail, and the grep -vx filter is sound. If every package were filtered out, the pipeline would fail on grep -v's exit status of 1. That cannot happen in practice.
  • setupTestServer: writing to io.Discard removes the buffer growth. The B/op drop in your numbers matches that.

Suggestions

  1. runnerCPU ordering (gate.go): the cpu model: prefix is tried before Model name:, and that order is what makes the lscpu fallback a fallback. A one-line comment would protect it from a reorder.
  2. TestSetupTestServerDiscardsOutput: it compares server.writer with io.Discard, so it depends on the field name and on NewServer storing the writer unwrapped. If NewServer ever wraps the writer, for example in a mutex-guarded writer, the test fails for a harmless reason. A behavioural check would be sturdier.
  3. Matrix mode and the 64MiB limit: in about 900 benchmarks, some, such as the 30KB LSP documents and large-input cases, may have a live heap near or above 64MiB. The runtime then collects continuously, which dilutes ratios but is the same for base and head. Consider defaulting gomemlimit to 256MiB for mode=matrix, or noting this in the report.
  4. matrix-suite.sh roots: pkg and wasm/internal are hardcoded. A benchmark package in a new top-level directory would be missed. Your independent-walk test only covers those two roots. Widening that test to the whole repo would catch it.
  5. Cachegrind cost (profile.sh): cachegrind over Go binaries runs about 50× slower, and each query gets two runs. A per-run timeout or logged duration would help diagnose slow profile jobs. Also check that cg-top-*.txt does not just contain an error message when the cg_annotate fallback fires.
  6. CHANGELOG: the entry should say that GOGC=off timings from before and after this change are not comparable. The PR body already says it.

Test coverage

Coverage is strong. It includes the workflow pin checks, the GOMEMLIMIT-on-every-GOGC=off check, the lib.sh and cmd/publish default equality test, the env(1) round trip and the matrix-suite.sh mutation test. The remaining CI A/A runs for off, 64MiB and 256MiB would be worth reading before merge, to confirm that 64MiB is the right default.

ajitpratap0 and others added 2 commits November 1, 2026 23:52
Resolves the CHANGELOG.md [Unreleased] conflict with #917 (W0-12) by
keeping both its ### Added block and the W0-15 ### Changed block.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
…trix 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 21s —— View job


Review of PR #921: perf-gate methodology (W0-15)

I read lib.sh and the profile.sh diff in full. I did not run anything. I did not re-read the workflow YAML, cmd/publish or the Go tests, so this adds only a few points to the two earlier reviews. Their remaining suggestions still apply: the runnerCPU ordering comment, a behavioural TestSetupTestServerDiscardsOutput, and the CHANGELOG comparability note.

Overall: the change is sound and stays inside tooling. The shadow gate passed (geomean -0.19%, A/A 0 of 26). I found no blocking issues.

Checked and correct

  • perf_gcoff_env: with the limit off, -u GOMEMLIMIT comes before GOGC=off, which env(1) requires. Reading it with a while read loop keeps it working on macOS bash 3.2.
  • perf_check_gomemlimit: the validation rejects a bad value up front, instead of letting every benchmark binary panic at startup.
  • cachegrind_run: splitting local q=$1 n=$2 from local log=... fixes the earlier expansion bug.
  • Cachegrind columns: the order Ir I1mr ILmr Dr D1mr DLmr matches the awk indices v[1], v[2], v[3], v[5], v[6].
  • Under set -e: if ! c1=$(...) || ! c2=$(...) is safe.

Suggestions (non-blocking)

  1. cg-top-*.txt can hold an error message. In profile.sh the cg_annotate ... || cg_annotate ... || true chain redirects 2>&1 into the file. If both calls fail, the artifact holds only the error text. Consider removing the file when both fail, or writing a one-line "cg_annotate unavailable" note.
  2. No cap on the cachegrind runtime. Each query runs valgrind twice, and the second run does 2200 iterations. timeout around cachegrind_run, or logging the duration per run, would make slow profile jobs easier to diagnose. The cachegrind failed row already covers a non-zero exit.
  3. Hot function names. cg_annotate --show=... --sort=I1mr is valgrind-version dependent, as your comment says. Recording valgrind --version in env.txt already helps. Consider also noting which of the two cg_annotate invocations produced each file.
  4. GC share under the limit in matrix mode. The earlier reviews raised this. Since the matrix A/A showed 52 of 907 benchmarks with varying allocs/op, a short report note in matrix mode would help readers. It could say that allocs/op is not deterministic for large-input benchmarks under a 64MiB limit, and that only the interleaved ratio is meaningful. This would also make it harder to mistake the "allocs/op increased" line for a regression.
  5. perf_cpu_model on mixed-core CPUs. It takes only the first model name line from /proc/cpuinfo. On hybrid or multi-socket machines that is a single representative, which is fine for a label. It is not the model of the pinned CPU.

Test coverage

Coverage is strong. The env(1) round trip, the stub-valgrind end-to-end test for profile.sh, the pin checks and the matrix-suite.sh mutation test are the right guards for tooling that otherwise only fails in CI. A small addition would be a test that cg-top-*.txt is absent or flagged when cg_annotate fails (suggestion 1).

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 18s —— View job


Review of PR #921: perf-gate methodology (W0-15)

I read scripts/perf/lib.sh in full and the cg_annotate section of the profile.sh diff. I did not run tests, and I did not re-read the workflow YAML, cmd/publish or the Go tests. This adds little to the three earlier reviews. Their open points still stand.

Overall: the change is sound and stays inside the tooling. The shadow gate passes (no gated metric moved, A/A 0 of 26). I found no blocking issues.

Checked and correct

  • perf_gcoff_env: -u GOMEMLIMIT comes before GOGC=off, as env(1) requires. It reads back with while read, so it works on bash 3.2.
  • perf_check_gomemlimit: it rejects a bad value up front instead of letting every benchmark binary panic at startup.
  • perf_cachegrind_counts: the [[:space:]] prefix avoids matching the ==pid== prefix. A counter missing from the summary defaults to 0, and a missing I refs line fails.
  • perf_cpu_model: the fallback chain is /proc/cpuinfo, then lscpu, then sysctl, then unknown. That is fine for a label.

Suggestions (non-blocking)

  1. cg-top-*.txt can end up holding only an error message. profile.sh redirects 2>&1 into the file for both cg_annotate attempts, then ends with || true. If both calls fail, the artifact looks like a listing but contains an error. Remove the file when both fail, or write a one-line "cg_annotate unavailable" note. A small test through the stub valgrind would cover it.
  2. Cachegrind runtime. Each query gets two valgrind runs, and the second is 2200 iterations. A timeout, or logging the duration per run, would make slow profile jobs easier to diagnose.
  3. Matrix mode and the 64MiB limit. Some large-input benchmarks may hold a live heap near the limit. The runtime then collects continuously. Base and head are affected equally, so the ratios stay valid but get noisier. Your matrix A/A already showed 52 of 907 benchmarks with varying allocs/op. A short note in the matrix report would stop "allocs/op increased" from being read as a regression. It could say that only the interleaved ratio is meaningful there. Alternatively, default matrix mode to 256MiB.
  4. runnerCPU in gate.go. The cpu model: prefix has to be tried before the Model name: lscpu fallback. A one-line comment would protect that order from a reorder.
  5. TestSetupTestServerDiscardsOutput. It compares server.writer with io.Discard, so it depends on the field name and on NewServer storing the writer unwrapped. A behavioural check would survive harmless refactors.
  6. matrix-suite.sh roots. pkg and wasm/internal are hardcoded. A benchmark package in a new top-level directory would be missed, and the independent-walk test only covers those two roots. Walking the whole repo in the test would catch it.
  7. CHANGELOG. The diff adds a CHANGELOG entry. Confirm it says that GOGC=off timings from before and after this change are not comparable. The PR body already says so.

Test coverage

Coverage is strong for tooling that otherwise only fails in CI. The env(1) round trip, the stub-valgrind end-to-end test for profile.sh, the workflow pin checks and the matrix-suite.sh mutation test are the right guards. The one gap is suggestion 1.

ajitpratap0 and others added 2 commits November 2, 2026 00:32
…0-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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 21s —— View job


Review of PR #921: perf-gate methodology (W0-15)

I read the cg_annotate block in profile.sh and runnerCPU in gate.go. I ran nothing. Three earlier reviews cover the rest of the diff, so this one only adds what I confirmed in the code. I found no blocking issues. The shadow gate passes on head 44249405.

Confirmed in the code (all non-blocking)

  1. cg-top-*.txt can hold only an error message (scripts/perf/profile.sh:168-169). Both cg_annotate calls redirect 2>&1 into the same file, and the chain ends in || true. If both fail, the artifact looks like a listing but contains the error text. The second call also truncates whatever the first wrote. Suggested fix: write to a temp file, and if both calls fail, replace it with a one-line cg_annotate unavailable note.
  2. runnerCPU ordering is load-bearing (scripts/perf/gate/gate.go:529). The loop tries cpu model: first and then Model name:, and that order is what makes the lscpu dump a fallback. A comment saying so would protect it from a reorder.
  3. No time cap or logging for cachegrind. Each query runs valgrind twice, and the second run does 2200 iterations. A timeout around cachegrind_run, or a logged duration per run, would help when diagnosing slow profile jobs.
  4. Matrix mode with a fixed 64MiB limit. Large-input benchmarks (the 30KB LSP documents, ParseRW/*/large) may keep a live heap near the limit, so the GC runs often. The effect is the same for base and head, so ratios stay valid. The matrix A/A showed 52 of 907 benchmarks with varying allocs/op. A line in the matrix report saying that only the interleaved ratio is meaningful there, and that "allocs/op increased" is not a regression signal, would prevent misreading.
  5. TestSetupTestServerDiscardsOutput compares the writer field with io.Discard. It fails on a harmless rename or if the writer gets wrapped. A behavioural check would be sturdier.
  6. matrix-suite.sh roots. pkg and wasm/internal are hardcoded. Walking the whole repo in the independent-walk test would catch a benchmark package in a new top-level directory.
  7. CHANGELOG. Confirm the entry says GOGC=off timings from before and after this change are not comparable. The PR body already says so.

Test coverage

Coverage is strong for tooling that otherwise only fails in CI. That includes the env(1) round trip, the stub-valgrind test for profile.sh, the workflow pin and lint tests, and the mutation checks. The one gap is item 1, where a stub-valgrind case with a failing cg_annotate would cover it.

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 18s —— View job


Review of PR #921: perf-gate methodology (W0-15)

I confirmed one point in the code, scripts/perf/profile.sh:168-169, and did not run anything. Five earlier reviews cover the rest of the diff and I did not re-read it. I found no blocking issues. The shadow gate passes at head a9a2d60d/ec096578: geomean -0.04%, A/A 0 of 32, allocs/op identical for all 32.

Still open (all non-blocking, repeated from earlier reviews)

  1. cg-top-*.txt can hold only an error message (profile.sh:168-169). Both cg_annotate attempts redirect 2>&1 into the same file and the chain ends in || true. The second call truncates what the first wrote. If both fail, the artifact looks like a listing but contains error text. Fix: write to a temp file, move it into place on success, and otherwise write a one-line "cg_annotate unavailable" note. A stub-valgrind test case with a failing cg_annotate would cover it.
  2. No time cap or timing for cachegrind. Each query gets two valgrind runs, and the second does 2200 iterations. A timeout or a logged duration per run would help diagnose slow profile jobs.
  3. Matrix mode and the fixed 64MiB limit. Some large-input benchmarks, such as the 30KB LSP documents and ParseRW/*/large, may keep a live heap near the limit. The runtime then collects continuously. Base and head are affected equally, so ratios stay valid but get noisier. Your matrix A/A showed allocs/op varying in 52 of 907 benchmarks. Consider a line in the matrix report saying that only the interleaved ratio is meaningful there and that "allocs/op increased" is not a regression signal.
  4. runnerCPU ordering (gate.go). The cpu model: prefix has to be tried before Model name:, and that order is what makes the lscpu dump a fallback. A one-line comment would protect it.
  5. TestSetupTestServerDiscardsOutput compares the writer field with io.Discard. It fails on a harmless rename or if the writer gets wrapped. A behavioural check would be sturdier.
  6. matrix-suite.sh roots. pkg and wasm/internal are hardcoded, and the independent-walk test only covers those two roots. A benchmark package in a new top-level directory would be missed. Walking the whole repo in the test would catch it.

The CHANGELOG diff adds 41 lines. I did not open it, so I can't say whether it states that GOGC=off timings from before and after this change are not comparable. The PR body does state it.

Test coverage

Coverage is strong for tooling that otherwise only fails in CI. That includes the env(1) round trip, the stub-valgrind test for profile.sh, the workflow pin and GOMEMLIMIT lint tests, and the mutation checks. The one gap is item 1.

This branch was successfully deployed

1 active deployment
Preview — 58a3ccde Deployed Nov 2, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-cd documentation Improvements or additions to documentation lsp performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant