Skip to content

perf(security): pooled struct visitor for Scanner.Scan [W6-02] - #938

Open
ajitpratap0 wants to merge 5 commits into
mainfrom
perf/w6-02-pooled-scan-visitor
Open

perf(security): pooled struct visitor for Scanner.Scan [W6-02]#938
ajitpratap0 wants to merge 5 commits into
mainfrom
perf/w6-02-pooled-scan-visitor

Conversation

@ajitpratap0

@ajitpratap0 ajitpratap0 commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

W6-02 (SEC-2): Scanner.Scan walks the AST with a pooled struct ast.Visitor (scanWalker) instead of a per-call closure and heap-escaping ancestors slice. Popped slots are nil-ed and walkers deeper than 1024 are not pooled (fixes a bug in the audit prototype). No API or behaviour change; findings are byte-identical.

Test Plan

  • go test -count=1 ./pkg/sql/security/... and -race: pass (golden security.sha256 unchanged).
  • Frozen-reference differential test over the corpus, real-world statements with 6 injection suffixes, and targeted cases; plus release-invariant, oversized-drop, reset, alloc-over-walk and concurrency tests. 8 mutations each fail a test.
  • task test:cbinding, go test ./pkg/compatibility/..., staticcheck on pkg/sql/security: pass. task check: only pkg/transpiler TestTranspileDeepChainLinear failed (timing-ratio test, load average ~97 on the shared M4; the package does not import pkg/sql/security).
  • Local M4 task bench:ab (indicative, load 7-12+): BenchmarkScanASTRW B/op -53.3% to -58.3% per dialect, allocs/op -29% to -33%; ComplexQuery/LargeQuery/Parallel B/op -14% to -36%, allocs -19% to -43%. Time not significant (noise).
  • CI perf-gate (layout-randomized, amd64): PASS, no gated metric moved. NOTE: the gain is B/op and allocs/op only, measured locally on M4; the amd64 gate does not cover the target (no Scan benchmark in the gated suite) and time was not significant. Adding BenchmarkScanASTRW to the gate is a separate follow-up.
  • Race-shard fix: TestScanConcurrentPooledWalker is scaled under -race (4 workers x 8 iterations, 10 corpus statements, 100-term chain instead of 5000); full-size run kept without -race. go test -race ./pkg/sql/security now 18.7s locally (was 25-32s). Timeout not changed. Branch merged with origin/main (ci(perf): randomize code layout in the A/B perf gate so layout noise cannot read as a regression #934 gate now in).

Breaking Changes

None.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

@ajitpratap0 @claude
perf(security): pooled struct visitor for Scanner.Scan [W6-02]
2db7a70 
Replace the per-call ast.Inspect closure and heap-escaping ancestors slice with a pooled
scanWalker. Findings are byte-identical (frozen-reference differential test). BenchmarkScanASTRW
B/op -53% to -58%, allocs/op -29% to -33% on M4 (indicative).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@vercel

vercel Bot commented Nov 3, 2026 •
edited
Loading

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 3, 2026 10:32pm UTC

@github-actions

github-actions Bot commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Review of PR #938: pooled struct visitor for Scanner.Scan

I read scan_walker.go and the scanner.go diff, and checked them against ast.Walk in visitor.go. I did not run the tests, and I only skimmed the two new test files (scan_walker_test.go, scan_walker_equiv_test.go). The tests are covered by your description and CI.

Verdict: looks good. I found no blocking issues.

Correctness

  • The Visit(nil) pairing is sound. Walk calls Visit(nil) only after a non-nil visitor was returned. visit appends to ancestors only on the path that returns true. The early returns (the JoinClause.Left skip and the text-only statements) don't push and aren't popped, so push and pop stay balanced.
  • The switch body is moved over unchanged, so findings should be identical. The differential test is the right way to prove that.
  • Pooling is safe for concurrency. Each Scan call gets its own walker, and release clears s and result, so a pooled walker doesn't keep the Scanner or the results alive.
  • Nil-ing popped slots on the node == nil path stops pooled walkers from keeping AST nodes alive. Dropping walkers whose capacity exceeds 1024 stops one deep AST from bloating the pool permanently.

Minor suggestions (non-blocking)

  1. release() (scan_walker.go:37) relies on the invariant that ancestors is empty with all slots nil at the end of the walk. If that invariant ever broke (for example, a future Visit error return aborts Walk mid-tree), [:0] would leave stale node pointers in the backing array. A cheap clear(w.ancestors) before the reslice would make the "all slots nil" comment true by construction. It costs almost nothing, because the slice is normally empty at that point.
  2. scanner.go calls release() without defer. If a scan* helper panics, the walker is simply dropped and collected by the GC. That is safe, and it is arguably better than returning a half-used walker to the pool. A short comment saying so would stop someone later "fixing" it with a defer, which CLAUDE.md's "always defer" rule might encourage.
  3. In acquireScanWalker, the !ok branch of the type assertion can't be reached because New always returns a *scanWalker. It's harmless defensive code, and it matches the repo's safe-assertion convention.
  4. The performance numbers are indicative only: the M4 was under load, and time differences were within noise. The B/op and allocs/op reductions are deterministic, so they are the credible part. Please confirm the CI perf-gate verdict before merging, since it is still pending.

Security: There is no behaviour change in what the scanner detects. The extra pooled state is cleared on release, so nothing leaks between scans.

Test coverage: Coverage looks thorough. It includes a frozen-reference differential test, release-invariant and oversized-drop tests, concurrency tests, and mutation checks.

The task check failure in pkg/transpiler (TestTranspileDeepChainLinear) is a timing-ratio test, and that package doesn't import pkg/sql/security, so it is plausibly unrelated.
· branch perf/w6-02-pooled-scan-visitor

@github-actions github-actions Bot added documentation Improvements or additions to documentation security labels Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Base b1d862502441, head b454053a069e. 20 interleaved rounds, 20 randomized code layouts per arm (-randlayout seeds 417315510102..417315510121; A/A' 417315510122..417315510141), benchtime 100ms, -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: each arm runs 20 randomized code layouts (-ldflags=-randlayout); time regresses at >= +3% when significant at a false discovery rate of 0.01 over the suite (Benjamini-Hochberg on exact Mann-Whitney U p-values of the per-layout medians, so a regression must hold across layouts, not within one binary), or a suite geomean >= +1.5% (per-layout suite 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%. The A/A' control runs the base revision under a disjoint set of layout seeds; when more than 10% of benchmarks move over 3% in it, the run is noisy: re-run once, then inconclusive (neutral).

Gated suite (GOGC=off, GOMAXPROCS=1)

benchmark time p A/A' spread B/op allocs/op status
ParseE2E/simple_select -0.57% 0.534 +0.09% 1.55% +0.00% 5 → 5 ok
ParseE2E/select_where +0.16% 0.925 +0.15% 3.53% +0.00% 18 → 18 ok
ParseE2E/join -0.18% 0.756 -0.34% 1.43% +0.00% 17 → 17 ok
ParseE2E/aggregate -0.55% 0.499 -0.35% 2.61% +0.00% 18 → 18 ok
ParseE2E/subquery -1.01% 0.250 -0.57% 2.14% +0.00% 14 → 14 ok
ParseE2E/cte -0.03% 1.000 +0.11% 1.96% +0.00% 21 → 21 ok
ParseE2E/window -0.74% 0.273 -0.43% 1.73% +0.00% 11 → 11 ok
ParseE2E/complex -0.59% 0.525 +0.06% 3.95% +0.00% 49 → 49 ok
ParseE2E/insert -0.09% 0.839 -0.27% 0.88% +0.00% 17 → 17 ok
ParseE2E/update -0.70% 0.330 -0.96% 2.37% +0.00% 13 → 13 ok
ParseE2E/delete +0.08% 0.892 -0.24% 1.65% +0.00% 5 → 5 ok
ParseRW/postgresql/medium -0.55% 0.265 -0.02% 1.26% -0.00% 646 → 646 ok
ParseRW/mysql/medium -0.56% 0.327 +0.13% 1.74% +0.00% 368 → 368 ok
ParseRW/mariadb/medium -0.63% 0.211 +0.17% 1.43% -0.00% 528 → 528 ok
ParseRW/sqlserver/medium -0.81% 0.174 -0.21% 2.06% -0.00% 927 → 927 ok
ParseRW/oracle/medium -1.05% 0.042 -0.07% 1.17% -0.00% 1090 → 1090 ok
ParseRW/sqlite/medium -1.87% 0.070 +0.30% 3.73% +0.00% 347 → 347 ok
ParseRW/snowflake/medium -0.54% 0.149 +0.07% 1.36% +0.00% 776 → 776 ok
ParseRW/clickhouse/medium +0.33% 0.678 -0.50% 2.56% +0.00% 383 → 383 ok
ParseFresh/simple_select -0.75% 0.213 -0.53% 2.37% +0.00% 6 → 6 ok
ParseFresh/join -0.76% 0.156 +0.02% 2.18% +0.00% 18 → 18 ok
ParseFresh/complex -0.38% 0.620 +0.01% 1.64% +0.00% 50 → 50 ok
PoolGetPut/serial +0.38% 0.383 +1.10% 0.93% +0.00% 0 → 0 ok
Tokenize/scratch/join +0.56% 0.394 +1.45% 2.70% +0.00% 0 → 0 ok
Tokenize/scratch/complex +0.11% 0.914 +1.27% 2.87% +0.00% 0 → 0 ok
Tokenize/scratch/insert +0.28% 0.725 +1.10% 2.00% +0.00% 0 → 0 ok
Normalize/simple_select -0.25% 0.570 -0.73% 1.22% +0.00% 7 → 7 ok
Normalize/join -0.19% 0.808 -0.13% 1.31% +0.00% 23 → 23 ok
Normalize/complex -0.53% 0.330 +0.33% 1.83% +0.00% 68 → 68 ok
TransformParse/simple_select -0.03% 0.978 +0.20% 4.02% +0.00% 5 → 5 ok
TransformParse/join +0.09% 0.935 +0.16% 1.67% +0.00% 17 → 17 ok
TransformParse/complex +0.17% 0.818 -0.24% 1.59% +0.00% 49 → 49 ok
LintParallel/rules +0.33% 0.698 +0.53% 2.33% -0.00% 158312 → 158312 ok
LintParallel/lint -0.33% 0.583 +0.66% 2.30% +0.00% 236892 → 236893 ok
LintRW/postgresql +0.11% 0.820 +1.02% 1.75% -0.00% 26500 → 26500 ok
LintRW/mysql -0.36% 0.718 +0.37% 2.96% -0.00% 32413 → 32413 ok
LintRW/mariadb -0.03% 0.947 +0.40% 1.42% -0.00% 29610 → 29610 ok
LintRW/sqlserver -0.08% 0.883 +0.26% 2.24% +0.00% 25622 → 25622 ok
LintRW/oracle +0.10% 0.758 +0.16% 1.72% -0.00% 31614 → 31614 ok
LintRW/sqlite -0.48% 0.698 +0.15% 2.78% -0.00% 29427 → 29427 ok
LintRW/snowflake -0.24% 0.620 +0.23% 0.75% +0.00% 28806 → 28806 ok
LintRW/clickhouse -0.30% 0.659 -0.56% 1.31% -0.00% 32891 → 32891 ok
LintRule/L007 -1.00% 0.265 -0.40% 2.14% +0.00% 1194 → 1194 ok
LintRule/L010 -0.19% 0.698 -0.28% 3.08% +0.00% 0 → 0 ok
ParseError/syntax_mid/parse +0.37% 0.365 -0.08% 1.54% +0.00% 12 → 12 ok
ParseError/syntax_mid/validate -0.13% 0.829 -0.65% 1.43% +0.00% 12 → 12 ok
ParseError/syntax_end/parse -1.32% 0.190 -0.79% 3.78% +0.00% 16 → 16 ok
ParseError/syntax_end/validate -0.78% 0.285 +0.19% 2.60% +0.00% 16 → 16 ok
ParseError/tokenize/parse +0.80% 0.478 -0.32% 3.05% +0.00% 4 → 4 ok
ParseError/tokenize/validate -0.76% 0.372 -0.87% 2.93% +0.00% 21 → 21 ok
ParseError/long_prefix/parse +0.74% 0.429 +0.27% 1.90% -0.01% 219 → 219 ok
ParseError/long_prefix/validate +0.33% 0.820 +0.15% 3.84% +0.00% 219 → 219 ok
geomean -0.28% 0.327 +0.03%

time and A/A' are Hodges-Lehmann shifts over the per-layout medians. spread: the base arm's robust standard deviation across its layouts (1.4826 x MAD, relative), i.e. how much this benchmark moves between builds that differ only in function placement, plus run-to-run noise; a change much smaller than it cannot be resolved in one run.

A/A' control (base revision, disjoint layout seeds): 0 of 52 benchmarks moved more than 3%; 0 would have been flagged by the time rule; suite geomean +0.03%, p 0.820.

Low power

6 benchmark(s) have under 80% estimated power to be flagged on their own for a +5% slowdown (their spread across layouts is too wide for 20 layouts at the Benjamini-Hochberg level alpha/m = 0.000192). A slowdown below the last column usually goes unflagged there; instructions/op, allocs/op and the suite geomean still guard them.

benchmark spread power at +5% slowdown caught 80% of the time
TransformParse/simple_select 4.02% 51% +6.1%
ParseE2E/complex 3.95% 53% +6.0%
ParseError/long_prefix/validate 3.84% 58% +5.8%
ParseError/syntax_end/parse 3.78% 60% +5.7%
ParseRW/sqlite/medium 3.73% 62% +5.7%
ParseE2E/select_where 3.53% 70% +5.4%

Power is estimated with the normal approximation for the U test (Noether): power = Phi(ln(1+shift)/s * sqrt(K/2) * sqrt(3/pi) - z(1 - alpha/(2m))), with s the spread and K the layouts per arm.

Instructions per operation (cmd/perfdriver)

case instructions/op status
e2e/aggregate +0.02% ok
e2e/complex +0.00% ok
e2e/cte -0.08% ok
e2e/delete +0.06% ok
e2e/insert -0.04% ok
e2e/join -0.01% ok
e2e/select_where +0.01% ok
e2e/simple_select +0.07% ok
e2e/subquery +0.04% ok
e2e/update +0.03% ok
e2e/window +0.06% ok
rw/postgresql/medium +0.01% ok
Secondary metric: default GC, GOMAXPROCS=1 (informational)
benchmark time p
ParseE2E/simple_select +0.42% 0.912
ParseE2E/select_where +0.20% 0.850
ParseE2E/join -0.24% 0.684
ParseE2E/aggregate -0.30% 0.684
ParseE2E/subquery -0.18% 0.853
ParseE2E/cte -0.16% 0.850
ParseE2E/window +0.42% 0.684
ParseE2E/complex -0.90% 0.247
ParseE2E/insert +0.11% 0.853
ParseE2E/update -0.34% 0.739
ParseE2E/delete -0.26% 0.820
ParseRW/postgresql/medium -0.76% 0.436
ParseRW/mysql/medium -0.71% 0.436
ParseRW/mariadb/medium +0.17% 0.971
ParseRW/sqlserver/medium -0.55% 0.579
ParseRW/oracle/medium -0.44% 0.353
ParseRW/sqlite/medium -0.90% 0.123
ParseRW/snowflake/medium -0.22% 0.796
ParseRW/clickhouse/medium -1.58% 0.063
ParseFresh/simple_select +0.69% 0.481
ParseFresh/join +0.41% 0.481
ParseFresh/complex -0.43% 0.631
PoolGetPut/serial +0.34% 0.226
Tokenize/scratch/join -0.67% 0.623
Tokenize/scratch/complex +0.24% 0.853
Tokenize/scratch/insert +0.25% 0.910
Normalize/simple_select -0.10% 0.912
Normalize/join -0.92% 0.123
Normalize/complex -1.47% 0.165
TransformParse/simple_select -0.25% 0.796
TransformParse/join -0.87% 0.190
TransformParse/complex -0.30% 0.364
LintParallel/rules -0.48% 0.684
LintParallel/lint -0.49% 0.436
LintRW/postgresql -0.23% 0.739
LintRW/mysql +0.47% 0.436
LintRW/mariadb +0.54% 0.631
LintRW/sqlserver +0.20% 0.853
LintRW/oracle -0.25% 0.912
LintRW/sqlite +0.76% 0.393
LintRW/snowflake +0.16% 0.796
LintRW/clickhouse -0.43% 0.796
LintRule/L007 +1.02% 0.315
LintRule/L010 +0.23% 0.280
ParseError/syntax_mid/parse +0.43% 0.853
ParseError/syntax_mid/validate -0.19% 0.971
ParseError/syntax_end/parse -0.45% 0.473
ParseError/syntax_end/validate -0.54% 0.496
ParseError/tokenize/parse -0.63% 0.791
ParseError/tokenize/validate +0.12% 0.912
ParseError/long_prefix/parse +0.43% 0.739
ParseError/long_prefix/validate +0.77% 0.579
geomean -0.15%
Environment 
date:        2026-11-03T22:34:53Z
base:        b1d862502441c292e007a93e75925272bfa0a5f7
head:        working tree of b454053a069ea732de3e69f64ca5bd1e269afa1e
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
layouts:     20 per arm, -ldflags=-randlayout seeds 417315510102..417315510121 (base, head), 417315510122..417315510141 (A/A')
pinning:     pinned to CPU 3 (SMT siblings 2-3) with taskset
gcoff env:   GOGC=off GOMEMLIMIT=64MiB
clocksource: tsc
load:        8.16, 2.97, 1.08
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, samples pooled across layouts: informational, the verdict uses the per-layout test) 
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.371µ ± 1%   2.360µ ± 1%       ~ (p=0.569 n=20)
Normalize/join            7.391µ ± 1%   7.379µ ± 1%       ~ (p=0.805 n=20)
Normalize/complex         20.30µ ± 2%   20.34µ ± 1%       ~ (p=0.331 n=20)
geomean                   7.085µ        7.075µ       -0.15%

                        │     base     │                 head                  │
                        │     B/op     │     B/op      vs base                 │
Normalize/simple_select     896.0 ± 0%     896.0 ± 0%       ~ (p=1.000 n=20) ¹
Normalize/join            2.211Ki ± 0%   2.211Ki ± 0%       ~ (p=1.000 n=20) ¹
Normalize/complex         6.117Ki ± 0%   6.117Ki ± 0%       ~ (p=1.000 n=20) ¹
geomean                   2.279Ki        2.279Ki       +0.00%
¹ all samples are equal

                        │    base    │                head                 │
                        │ allocs/op  │ allocs/op   vs base                 │
Normalize/simple_select   7.000 ± 0%   7.000 ± 0%       ~ (p=1.000 n=20) ¹
Normalize/join            23.00 ± 0%   23.00 ± 0%       ~ (p=1.000 n=20) ¹
Normalize/complex         68.00 ± 0%   68.00 ± 0%       ~ (p=1.000 n=20) ¹
geomean                   22.20        22.20       +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.677µ ± 1%   1.664µ ± 2%       ~ (p=0.533 n=20)
ParseE2E/select_where             3.608µ ± 2%   3.606µ ± 3%       ~ (p=0.920 n=20)
ParseE2E/join                     4.782µ ± 1%   4.772µ ± 1%       ~ (p=0.753 n=20)
ParseE2E/aggregate                3.575µ ± 2%   3.562µ ± 2%       ~ (p=0.499 n=20)
ParseE2E/subquery                 3.259µ ± 1%   3.217µ ± 1%       ~ (p=0.251 n=20)
ParseE2E/cte                      4.574µ ± 1%   4.564µ ± 4%       ~ (p=1.000 n=20)
ParseE2E/window                   2.992µ ± 1%   2.963µ ± 1%       ~ (p=0.274 n=20)
ParseE2E/complex                  12.74µ ± 4%   12.63µ ± 5%       ~ (p=0.525 n=20)
ParseE2E/insert                   2.192µ ± 1%   2.194µ ± 1%       ~ (p=0.836 n=20)
ParseE2E/update                   1.975µ ± 2%   1.958µ ± 2%       ~ (p=0.330 n=20)
ParseE2E/delete                   1.260µ ± 1%   1.262µ ± 2%       ~ (p=0.888 n=20)
ParseRW/postgresql/medium         175.5µ ± 1%   174.5µ ± 1%       ~ (p=0.265 n=20)
ParseRW/mysql/medium              98.91µ ± 1%   98.05µ ± 1%       ~ (p=0.327 n=20)
ParseRW/mariadb/medium            141.8µ ± 1%   141.0µ ± 1%       ~ (p=0.211 n=20)
ParseRW/sqlserver/medium          287.7µ ± 2%   284.6µ ± 1%       ~ (p=0.174 n=20)
ParseRW/oracle/medium             338.7µ ± 1%   334.4µ ± 1%       ~ (p=0.041 n=20)
ParseRW/sqlite/medium             79.64µ ± 4%   76.78µ ± 3%       ~ (p=0.069 n=20)
ParseRW/snowflake/medium          193.8µ ± 1%   192.2µ ± 1%       ~ (p=0.149 n=20)
ParseRW/clickhouse/medium         88.80µ ± 1%   89.35µ ± 2%       ~ (p=0.678 n=20)
ParseFresh/simple_select          7.259µ ± 2%   7.196µ ± 2%       ~ (p=0.214 n=20)
ParseFresh/join                   11.77µ ± 1%   11.70µ ± 2%       ~ (p=0.155 n=20)
ParseFresh/complex                21.29µ ± 1%   21.24µ ± 2%       ~ (p=0.620 n=20)
PoolGetPut/serial                 67.27n ± 1%   67.60n ± 2%       ~ (p=0.383 n=20)
ParseError/syntax_mid/parse       2.452µ ± 1%   2.463µ ± 1%       ~ (p=0.365 n=20)
ParseError/syntax_mid/validate    2.229µ ± 2%   2.225µ ± 2%       ~ (p=0.825 n=20)
ParseError/syntax_end/parse       2.885µ ± 3%   2.853µ ± 2%       ~ (p=0.189 n=20)
ParseError/syntax_end/validate    2.707µ ± 3%   2.694µ ± 1%       ~ (p=0.286 n=20)
ParseError/tokenize/parse         924.0n ± 3%   934.9n ± 2%       ~ (p=0.478 n=20)
ParseError/tokenize/validate      2.959µ ± 2%   2.940µ ± 2%       ~ (p=0.372 n=20)
ParseError/long_prefix/parse      62.92µ ± 2%   62.67µ ± 3%       ~ (p=0.429 n=20)
ParseError/long_prefix/validate   60.89µ ± 3%   61.37µ ± 2%       ~ (p=0.820 n=20)
geomean                           9.824µ        9.775µ       -0.50%

                                │     base     │                head                 │
                                │     B/s      │     B/s       vs base               │
ParseE2E/simple_select            14.79Mi ± 1%   14.90Mi ± 2%       ~ (p=0.533 n=20)
ParseE2E/select_where             27.23Mi ± 2%   27.25Mi ± 3%       ~ (p=0.920 n=20)
ParseE2E/join                     25.13Mi ± 1%   25.18Mi ± 1%       ~ (p=0.753 n=20)
ParseE2E/aggregate                22.41Mi ± 2%   22.49Mi ± 2%       ~ (p=0.499 n=20)
ParseE2E/subquery                 24.00Mi ± 1%   24.31Mi ± 1%       ~ (p=0.256 n=20)
ParseE2E/cte                      25.86Mi ± 1%   25.91Mi ± 4%       ~ (p=0.984 n=20)
ParseE2E/window                   29.96Mi ± 1%   30.25Mi ± 1%       ~ (p=0.268 n=20)
ParseE2E/complex                  28.30Mi ± 4%   28.54Mi ± 5%       ~ (p=0.525 n=20)
ParseE2E/insert                   33.07Mi ± 1%   33.04Mi ± 1%       ~ (p=0.878 n=20)
ParseE2E/update                   26.57Mi ± 2%   26.79Mi ± 2%       ~ (p=0.341 n=20)
ParseE2E/delete                   39.36Mi ± 1%   39.29Mi ± 2%       ~ (p=0.888 n=20)
ParseRW/postgresql/medium         20.44Mi ± 1%   20.57Mi ± 1%       ~ (p=0.250 n=20)
ParseRW/mysql/medium              27.37Mi ± 1%   27.60Mi ± 1%       ~ (p=0.317 n=20)
ParseRW/mariadb/medium            25.00Mi ± 1%   25.15Mi ± 1%       ~ (p=0.208 n=20)
ParseRW/sqlserver/medium          23.13Mi ± 2%   23.37Mi ± 1%       ~ (p=0.167 n=20)
ParseRW/oracle/medium             24.75Mi ± 1%   25.07Mi ± 1%       ~ (p=0.041 n=20)
ParseRW/sqlite/medium             25.94Mi ± 4%   26.90Mi ± 3%       ~ (p=0.071 n=20)
ParseRW/snowflake/medium          26.23Mi ± 1%   26.45Mi ± 1%       ~ (p=0.155 n=20)
ParseRW/clickhouse/medium         25.38Mi ± 1%   25.22Mi ± 2%       ~ (p=0.683 n=20)
ParseError/syntax_mid/parse       18.67Mi ± 1%   18.59Mi ± 1%       ~ (p=0.344 n=20)
ParseError/syntax_mid/validate    20.54Mi ± 2%   20.58Mi ± 2%       ~ (p=0.825 n=20)
ParseError/syntax_end/parse       12.89Mi ± 3%   13.04Mi ± 2%       ~ (p=0.189 n=20)
ParseError/syntax_end/validate    13.74Mi ± 3%   13.81Mi ± 1%       ~ (p=0.279 n=20)
ParseError/tokenize/parse         47.48Mi ± 2%   46.93Mi ± 2%       ~ (p=0.478 n=20)
ParseError/tokenize/validate      14.83Mi ± 2%   14.92Mi ± 2%       ~ (p=0.394 n=20)
ParseError/long_prefix/parse      36.71Mi ± 2%   36.85Mi ± 3%       ~ (p=0.429 n=20)
ParseError/long_prefix/validate   37.94Mi ± 3%   37.64Mi ± 2%       ~ (p=0.825 n=20)
geomean                           24.65Mi        24.78Mi       +0.52%

                                │      base      │                 head                  │
                                │      B/op      │     B/op      vs base                 │
ParseE2E/simple_select              816.0 ± 0%       816.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/select_where             1.523Ki ± 0%     1.523Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/join                     1.836Ki ± 0%     1.836Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/aggregate                1.812Ki ± 0%     1.812Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/subquery                 1.828Ki ± 0%     1.828Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/cte                      2.336Ki ± 0%     2.336Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/window                   1.281Ki ± 0%     1.281Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/complex                  5.025Ki ± 0%     5.025Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/insert                     824.0 ± 0%       824.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/update                     736.0 ± 0%       736.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/delete                     400.0 ± 0%       400.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/postgresql/medium         65.91Ki ± 0%     65.91Ki ± 0%       ~ (p=0.240 n=20)
ParseRW/mysql/medium              42.37Ki ± 0%     42.37Ki ± 0%       ~ (p=0.356 n=20)
ParseRW/mariadb/medium            60.61Ki ± 0%     60.61Ki ± 0%       ~ (p=0.210 n=20)
ParseRW/sqlserver/medium          83.68Ki ± 0%     83.68Ki ± 0%       ~ (p=0.185 n=20)
ParseRW/oracle/medium             104.8Ki ± 0%     104.8Ki ± 0%       ~ (p=0.168 n=20)
ParseRW/sqlite/medium             38.12Ki ± 0%     38.12Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/snowflake/medium          77.63Ki ± 0%     77.63Ki ± 0%       ~ (p=1.000 n=20)
ParseRW/clickhouse/medium         34.82Ki ± 0%     34.82Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseFresh/simple_select            896.0 ± 0%       896.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseFresh/join                   1.914Ki ± 0%     1.914Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseFresh/complex                5.102Ki ± 0%     5.102Ki ± 0%       ~ (p=1.000 n=20) ¹
PoolGetPut/serial                   0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_mid/parse         680.0 ± 0%       680.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_mid/validate      680.0 ± 0%       680.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_end/parse       1.430Ki ± 0%     1.430Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_end/validate    1.430Ki ± 0%     1.430Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseError/tokenize/parse           272.0 ± 0%       272.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/tokenize/validate      1.758Ki ± 0%     1.758Ki ± 0%       ~ (p=1.000 n=20) ¹
ParseError/long_prefix/parse      22.76Ki ± 0%     22.76Ki ± 0%       ~ (p=0.742 n=20)
ParseError/long_prefix/validate   22.68Ki ± 0%     22.68Ki ± 0%       ~ (p=1.000 n=20) ¹
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=20) ¹
ParseE2E/select_where              18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/join                      17.00 ± 0%      17.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/aggregate                 18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/subquery                  14.00 ± 0%      14.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/cte                       21.00 ± 0%      21.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/window                    11.00 ± 0%      11.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/complex                   49.00 ± 0%      49.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/insert                    17.00 ± 0%      17.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/update                    13.00 ± 0%      13.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseE2E/delete                    5.000 ± 0%      5.000 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/postgresql/medium          646.0 ± 0%      646.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/mysql/medium               368.0 ± 0%      368.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/mariadb/medium             528.0 ± 0%      528.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/sqlserver/medium           927.0 ± 0%      927.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/oracle/medium             1.090k ± 0%     1.090k ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/sqlite/medium              347.0 ± 0%      347.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/snowflake/medium           776.0 ± 0%      776.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseRW/clickhouse/medium          383.0 ± 0%      383.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseFresh/simple_select           6.000 ± 0%      6.000 ± 0%       ~ (p=1.000 n=20) ¹
ParseFresh/join                    18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseFresh/complex                 50.00 ± 0%      50.00 ± 0%       ~ (p=1.000 n=20) ¹
PoolGetPut/serial                  0.000 ± 0%      0.000 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_mid/parse        12.00 ± 0%      12.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_mid/validate     12.00 ± 0%      12.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_end/parse        16.00 ± 0%      16.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/syntax_end/validate     16.00 ± 0%      16.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/tokenize/parse          4.000 ± 0%      4.000 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/tokenize/validate       21.00 ± 0%      21.00 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/long_prefix/parse       219.0 ± 0%      219.0 ± 0%       ~ (p=1.000 n=20) ¹
ParseError/long_prefix/validate    219.0 ± 0%      219.0 ± 0%       ~ (p=1.000 n=20) ¹
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   14.63µ ± 1%   14.54µ ± 1%       ~ (p=0.256 n=20)
ParseRW/mysql/medium        12.36µ ± 1%   12.26µ ± 1%       ~ (p=0.327 n=20)
ParseRW/mariadb/medium      15.76µ ± 1%   15.67µ ± 1%       ~ (p=0.211 n=20)
ParseRW/sqlserver/medium    13.70µ ± 2%   13.55µ ± 1%       ~ (p=0.174 n=20)
ParseRW/oracle/medium       14.11µ ± 1%   13.94µ ± 1%       ~ (p=0.041 n=20)
ParseRW/sqlite/medium       13.27µ ± 4%   12.80µ ± 3%       ~ (p=0.069 n=20)
ParseRW/snowflake/medium    14.91µ ± 1%   14.78µ ± 1%       ~ (p=0.149 n=20)
ParseRW/clickhouse/medium   12.69µ ± 1%   12.76µ ± 2%       ~ (p=0.678 n=20)
geomean                     13.89µ        13.74µ       -1.03%

pkg: github.com/ajitpratap0/GoSQLX/pkg/linter
                   │     base     │                head                 │
                   │    sec/op    │    sec/op     vs base               │
LintParallel/rules   30.52m ±  2%   30.60m ±  1%       ~ (p=0.698 n=20)
LintParallel/lint    53.36m ±  2%   52.95m ±  1%       ~ (p=0.583 n=20)
LintRW/postgresql    6.717m ±  2%   6.733m ±  1%       ~ (p=0.820 n=20)
LintRW/mysql         7.076m ±  2%   7.029m ±  2%       ~ (p=0.718 n=20)
LintRW/mariadb       6.478m ±  1%   6.489m ±  2%       ~ (p=0.947 n=20)
LintRW/sqlserver     5.995m ±  2%   5.987m ±  1%       ~ (p=0.883 n=20)
LintRW/oracle        7.222m ±  1%   7.232m ±  1%       ~ (p=0.758 n=20)
LintRW/sqlite        6.181m ±  2%   6.145m ±  2%       ~ (p=0.698 n=20)
LintRW/snowflake     6.311m ±  0%   6.278m ±  2%       ~ (p=0.620 n=20)
LintRW/clickhouse    7.007m ±  1%   7.005m ±  1%       ~ (p=0.659 n=20)
LintRule/L007        2.789m ±  2%   2.751m ±  2%       ~ (p=0.265 n=20)
LintRule/L010        896.6µ ± 20%   893.0µ ± 19%       ~ (p=0.698 n=20)
geomean              7.041m         7.019m        -0.31%

                  │     base     │                head                 │
                  │     B/s      │     B/s       vs base               │
LintRW/postgresql   4.215Mi ± 2%   4.206Mi ± 1%       ~ (p=0.846 n=20)
LintRW/mysql        4.625Mi ± 2%   4.659Mi ± 2%       ~ (p=0.703 n=20)
LintRW/mariadb      4.706Mi ± 1%   4.702Mi ± 2%       ~ (p=0.909 n=20)
LintRW/sqlserver    5.169Mi ± 1%   5.178Mi ± 1%       ~ (p=0.931 n=20)
LintRW/oracle       5.498Mi ± 1%   5.493Mi ± 2%       ~ (p=0.804 n=20)
LintRW/sqlite       4.644Mi ± 2%   4.673Mi ± 2%       ~ (p=0.713 n=20)
LintRW/snowflake    4.940Mi ± 1%   4.969Mi ± 2%       ~ (p=0.732 n=20)
LintRW/clickhouse   4.635Mi ± 1%   4.630Mi ± 1%       ~ (p=0.693 n=20)
geomean             4.790Mi        4.800Mi       +0.20%

                   │      base      │                 head                  │
                   │      B/op      │     B/op      vs base                 │
LintParallel/rules   8.111Mi ± 0%     8.111Mi ± 0%       ~ (p=0.109 n=20)
LintParallel/lint    28.08Mi ± 0%     28.08Mi ± 0%       ~ (p=0.597 n=20)
LintRW/postgresql    3.089Mi ± 0%     3.089Mi ± 0%       ~ (p=0.482 n=20)
LintRW/mysql         3.880Mi ± 0%     3.880Mi ± 0%       ~ (p=0.234 n=20)
LintRW/mariadb       3.570Mi ± 0%     3.570Mi ± 0%       ~ (p=0.899 n=20)
LintRW/sqlserver     3.124Mi ± 0%     3.124Mi ± 0%       ~ (p=0.311 n=20)
LintRW/oracle        3.818Mi ± 0%     3.818Mi ± 0%       ~ (p=0.122 n=20)
LintRW/sqlite        3.385Mi ± 0%     3.385Mi ± 0%       ~ (p=0.941 n=20)
LintRW/snowflake     3.384Mi ± 0%     3.384Mi ± 0%       ~ (p=0.129 n=20)
LintRW/clickhouse    3.832Mi ± 0%     3.832Mi ± 0%       ~ (p=0.560 n=20)
LintRule/L007        92.98Ki ± 0%     92.98Ki ± 0%       ~ (p=1.000 n=20) ¹
LintRule/L010          0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=20) ¹
geomean                           ²                 -0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                   │     base      │                 head                 │
                   │   allocs/op   │  allocs/op   vs base                 │
LintParallel/rules   158.3k ± 0%     158.3k ± 0%       ~ (p=0.380 n=20)
LintParallel/lint    236.9k ± 0%     236.9k ± 0%       ~ (p=0.387 n=20)
LintRW/postgresql    26.50k ± 0%     26.50k ± 0%       ~ (p=0.693 n=20)
LintRW/mysql         32.41k ± 0%     32.41k ± 0%       ~ (p=1.000 n=20)
LintRW/mariadb       29.61k ± 0%     29.61k ± 0%       ~ (p=1.000 n=20)
LintRW/sqlserver     25.62k ± 0%     25.62k ± 0%       ~ (p=1.000 n=20) ¹
LintRW/oracle        31.61k ± 0%     31.61k ± 0%       ~ (p=0.695 n=20)
LintRW/sqlite        29.43k ± 0%     29.43k ± 0%       ~ (p=1.000 n=20) ¹
LintRW/snowflake     28.81k ± 0%     28.81k ± 0%       ~ (p=0.176 n=20)
LintRW/clickhouse    32.89k ± 0%     32.89k ± 0%       ~ (p=0.513 n=20)
LintRule/L007        1.194k ± 0%     1.194k ± 0%       ~ (p=1.000 n=20) ¹
LintRule/L010         0.000 ± 0%      0.000 ± 0%       ~ (p=1.000 n=20) ¹
geomean                          ²                +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                  │     base     │                head                 │
                  │   sec/stmt   │   sec/stmt    vs base               │
LintRW/postgresql   22.02µ ±  2%   22.08µ ±  1%       ~ (p=0.820 n=20)
LintRW/mysql        17.09µ ±  2%   16.98µ ±  2%       ~ (p=0.718 n=20)
LintRW/mariadb      17.05µ ±  1%   17.08µ ±  2%       ~ (p=0.947 n=20)
LintRW/sqlserver    21.64µ ±  1%   21.61µ ±  1%       ~ (p=0.883 n=20)
LintRW/oracle       19.63µ ±  1%   19.65µ ±  1%       ~ (p=0.764 n=20)
LintRW/sqlite       17.76µ ±  2%   17.66µ ±  2%       ~ (p=0.703 n=20)
LintRW/snowflake    19.30µ ±  0%   19.20µ ±  2%       ~ (p=0.620 n=20)
LintRW/clickhouse   17.79µ ±  1%   17.78µ ±  1%       ~ (p=0.659 n=20)
LintRule/L007       991.5n ±  2%   977.8n ±  2%       ~ (p=0.268 n=20)
LintRule/L010       318.8n ± 20%   317.4n ± 19%       ~ (p=0.673 n=20)
geomean             9.376µ         9.346µ        -0.32%

pkg: github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer
                         │    base     │                head                │
                         │   sec/op    │   sec/op     vs base               │
Tokenize/scratch/join      1.538µ ± 2%   1.545µ ± 2%       ~ (p=0.394 n=20)
Tokenize/scratch/complex   4.029µ ± 2%   4.006µ ± 3%       ~ (p=0.909 n=20)
Tokenize/scratch/insert    742.1n ± 2%   745.4n ± 1%       ~ (p=0.723 n=20)
geomean                    1.663µ        1.665µ       +0.11%

                         │     base     │                head                 │
                         │     B/s      │     B/s       vs base               │
Tokenize/scratch/join      78.13Mi ± 2%   77.79Mi ± 2%       ~ (p=0.369 n=20)
Tokenize/scratch/complex   89.48Mi ± 2%   89.99Mi ± 3%       ~ (p=0.904 n=20)
Tokenize/scratch/insert    97.67Mi ± 1%   97.24Mi ± 1%       ~ (p=0.733 n=20)
geomean                    88.06Mi        87.97Mi       -0.11%

                         │     base     │                head                 │
                         │     B/op     │    B/op     vs base                 │
Tokenize/scratch/join      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
Tokenize/scratch/complex   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
Tokenize/scratch/insert    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
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=20) ¹
Tokenize/scratch/complex   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
Tokenize/scratch/insert    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
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.588µ ± 3%   1.608µ ± 4%       ~ (p=0.973 n=20)
TransformParse/join            5.159µ ± 2%   5.178µ ± 2%       ~ (p=0.931 n=20)
TransformParse/complex         13.53µ ± 1%   13.57µ ± 2%       ~ (p=0.815 n=20)
geomean                        4.804µ        4.834µ       +0.64%

                             │     base     │                 head                  │
                             │     B/op     │     B/op      vs base                 │
TransformParse/simple_select     816.0 ± 0%     816.0 ± 0%       ~ (p=1.000 n=20) ¹
TransformParse/join            1.836Ki ± 0%   1.836Ki ± 0%       ~ (p=1.000 n=20) ¹
TransformParse/complex         5.023Ki ± 0%   5.023Ki ± 0%       ~ (p=1.000 n=20) ¹
geomean                        1.944Ki        1.944Ki       +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=20) ¹
TransformParse/join            17.00 ± 0%   17.00 ± 0%       ~ (p=1.000 n=20) ¹
TransformParse/complex         49.00 ± 0%   49.00 ± 0%       ~ (p=1.000 n=20) ¹
geomean                        16.09        16.09       +0.00%
¹ all samples are equal

Run, raw data and benchstat

Test and others added 2 commits November 3, 2026 22:56
Test
Merge remote-tracking branch 'origin/main' into perf/w6-02-pooled-sca…
79d5093 
…n-visitor
Test @claude
test(security): scale concurrent pooled-walker test down under -race …
4d38c7e 
…[W6-02]

The race shard exceeded its 2m budget; use fewer workers/iterations and a
100-term chain under -race (full 5000-deep run stays in the non-race run).

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 3, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Review of PR #938: pooled struct visitor for Scanner.Scan

Verdict: looks good. I found no blocking issues. I read scan_walker.go and the scanner.go diff. I did not run the tests, and I did not read the two new test files closely. I'm relying on your description and CI for those.

Correctness

  • The switch body in visit is a line-for-line move of the old closure body, with s and result now read from the walker. Findings should be unchanged.
  • The Visit(nil) pop is balanced with the push. A node is appended to ancestors only on the path that returns true, and Walk calls Visit(nil) only for nodes that returned a non-nil visitor.
  • The early-return paths don't push, so they aren't popped. These are the JoinClause.Left skip and the text-only statements.
  • ancestors[len-1] = nil before the reslice means a pooled walker holds no references to AST nodes.
  • release() clears s and result, so the pool doesn't retain the Scanner or the results.
  • Walkers with cap > 1024 are dropped, so one deep AST can't bloat the pool permanently.
  • release() is deliberately not deferred, and the comment in scanner.go explains why: a panic during the walk drops the walker instead of pooling it with dirty slots. That is a sensible choice and doesn't conflict with the CLAUDE.md "always defer" rule.

Minor, non-blocking

  1. release() assumes ancestors is empty with all slots nil, as the type comment says. The only guard is that Walk never aborts mid-tree. Calling clear(w.ancestors) before the reslice would make the invariant hold by construction. It costs almost nothing, because the slice is normally empty at that point.
  2. The !ok branch in acquireScanWalker can't be reached, since New always returns a *scanWalker. It is harmless and follows the repo's safe-assertion convention.
  3. _ = ast.Walk(w, tree) drops the error. Visit never returns an error, so this is fine. A one-line comment would say so.

Performance

  • CI perf-gate: PASS. No gated metric moved. Geomean is +0.06% with p=0.62, and B/op and allocs/op are unchanged across the gated suite.
  • The one flagged-looking row is ParseRW/clickhouse/medium at +2.70% (p=0.043). It is below the 3% threshold and doesn't survive multiple-comparison correction. Its A/A' control shows +0.93%, and that benchmark doesn't touch the security package.
  • As you noted, the gated suite has no Scanner.Scan benchmark. The -53% to -58% B/op and -29% to -33% allocs/op gains therefore rest on your local M4 measurements. These are deterministic but not independently gated. I'd add a Scan benchmark to the gate, either here or as a follow-up, so a future regression is caught.

Security

  • No change in what the scanner detects. The only new shared state is the pool, and it is cleared on release. Each Scan call owns its walker, so concurrent scans share no walk state.

Tests

  • Coverage as described is thorough: a frozen-reference differential test, release-invariant, oversized-drop and reset tests, an alloc check, concurrency, and 8 mutations that each fail a test.
  • The pkg/transpiler TestTranspileDeepChainLinear failure you reported is a timing-ratio test at load average ~97. That package doesn't import pkg/sql/security, so it is plausibly unrelated.
    · branch perf/w6-02-pooled-scan-visitor

Test @claude
merge: origin/main into perf/w6-02-pooled-scan-visitor
823e25c 
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 3, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Review of PR #938: pooled struct visitor for Scanner.Scan

I read scan_walker.go and the scanner.go and CHANGELOG diffs. I did not run the tests or read the two test files closely, so I'm relying on your description and CI for those.

Verdict: looks good. I found no blocking issues.

Correctness

  • The visit switch is a line-for-line move of the old closure body, with s and result now read from the walker. Findings should be identical.
  • Push and pop are balanced. A node is appended to ancestors only on the path that returns true, and Walk calls Visit(nil) only for nodes whose visit returned a non-nil visitor. The JoinClause.Left skip and the text-only statements return before the append, so they are never popped.
  • Popped slots are nil-ed, and release() clears s and result. A pooled walker therefore keeps no AST nodes, Scanner or results alive.
  • Walkers with cap > 1024 are dropped, so one deep AST can't bloat the pool.
  • Not using defer for release() is deliberate, and the comment in scanner.go explains it: a panic during the walk drops the walker instead of pooling it with dirty slots. That is a reasonable exception to CLAUDE.md's "always defer" rule.

Minor, non-blocking

  1. release() relies on ancestors being empty with every slot nil, which is only true if the walk finishes. Adding clear(w.ancestors[:cap(w.ancestors)]) before the reslice would make the invariant in the type comment hold by construction. It costs almost nothing, because the slice is normally empty at that point.
  2. The !ok branch in acquireScanWalker can't be reached, since New always returns a *scanWalker. It is harmless, and it follows the repo's safe-assertion convention.
  3. _ = ast.Walk(w, tree) discards the error. Visit never returns one, so this is fine, but a short comment saying so would help the next reader.

Performance

  • The CI perf-gate passes. No gated metric moved. B/op and allocs/op are unchanged across the gated suite, and the geomean is +0.04% with p=0.738. The A/A' control is clean.
  • The gated suite has no Scanner.Scan benchmark. The -53% to -58% B/op and -29% to -33% allocs/op gains therefore rest on your local M4 runs. They are deterministic, but nothing gates them. The follow-up to add BenchmarkScanASTRW to the gate is worth doing.

Security

  • What the scanner detects is unchanged. The new shared state is the pool, and it is cleared on release. Each Scan call owns its walker, so concurrent scans share no walk state.

Tests

  • Coverage as described is thorough: a frozen-reference differential test, release-invariant, oversized-drop, reset, alloc and concurrency tests, and 8 mutations that each fail a test.
  • The pkg/transpiler TestTranspileDeepChainLinear failure you reported is a timing-ratio test. That package doesn't import pkg/sql/security, so it is plausibly unrelated.
    · branch perf/w6-02-pooled-scan-visitor

@github-actions

github-actions Bot commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Review of PR #938: pooled struct visitor for Scanner.Scan

I read scan_walker.go and the scanner.go diff. I did not run the tests or read the two new test files closely, so I'm relying on your description and CI for those.

Verdict: looks good. I found no blocking issues.

Correctness

  • The visit switch is a line-for-line move of the old closure body, with s and result now read from the walker. Findings should be identical.
  • Push and pop are balanced. A node is appended to ancestors only on the path that returns true. Walk calls Visit(nil) only for nodes whose visit returned a non-nil visitor. The JoinClause.Left skip and the text-only statements return before the append, so they are never popped.
  • Popped slots are nil-ed, and release() clears s and result. A pooled walker therefore keeps no AST nodes, Scanner or results alive.
  • Walkers with cap > 1024 are dropped, so one deep AST can't bloat the pool.
  • release() is deliberately not deferred, and the comment in scanner.go explains why: a panic during the walk drops the walker instead of pooling it with dirty slots. That is a reasonable exception to CLAUDE.md's "always defer" rule.

Minor, non-blocking

  1. release() (scan_walker.go:37) relies on ancestors being empty with every slot nil, which holds only if the walk finishes. clear(w.ancestors[:cap(w.ancestors)]) before the reslice would make the type comment true by construction. It costs almost nothing, because the slice is normally empty at that point.
  2. The !ok branch in acquireScanWalker can't be reached, since New always returns a *scanWalker. It is harmless and follows the repo's safe-assertion convention.
  3. _ = ast.Walk(w, tree) discards the error. Visit never returns one, so a short comment saying so would help the next reader.

Performance

  • The CI perf-gate passes. No gated metric moved, and B/op and allocs/op are unchanged across the gated suite. The A/A' control is clean.
  • The gated suite has no Scanner.Scan benchmark. The -53% to -58% B/op and -29% to -33% allocs/op gains rest on your local M4 runs. They are deterministic but nothing gates them. Adding BenchmarkScanASTRW to the gate, as you planned, is worth doing.

Security

  • What the scanner detects is unchanged. The new shared state is the pool, and it is cleared on release. Each Scan call owns its walker, so concurrent scans share no walk state.

Tests

  • Coverage as described is thorough: a frozen-reference differential test, release-invariant, oversized-drop, reset, alloc and concurrency tests, and 8 mutations that each fail a test.
  • The pkg/transpiler TestTranspileDeepChainLinear failure you reported is a timing-ratio test. That package doesn't import pkg/sql/security, so it is plausibly unrelated.
    · branch perf/w6-02-pooled-scan-visitor

This branch was successfully deployed

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

Reviewers

No reviews

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation security

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ajitpratap0