Skip to content

perf(linter): shared cached preorder node list for AST rules [W6-05a] - #906

Merged
ajitpratap0 merged 6 commits into
mainfrom
perf/w6-05a-lint-context-nodes
Nov 2, 2026
Merged

perf(linter): shared cached preorder node list for AST rules [W6-05a]#906
ajitpratap0 merged 6 commits into
mainfrom
perf/w6-05a-lint-context-nodes

Conversation

@ajitpratap0

@ajitpratap0 ajitpratap0 commented Oct 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

PLAN W6-05(a). Thirteen linter rules each ran their own ast.Inspect walk over every statement. linter.Context now has an additive Nodes() method that builds the preorder node list once per Context (lazily, cached) and all 13 rules iterate it instead. The list is in ast.Inspect preorder with nil nodes filtered, and every original callback returned true on every path, so rules visit the same nodes in the same order and violations are identical (TestGoldenLint from W0-02 passes with no golden diffs).

The cache is keyed on the AST pointer (nodesAST), so a direct ctx.AST = other or a by-value Context copy with a new AST rebuilds the list instead of returning stale nodes; WithAST also resets it.

Why the #566 rejection no longer holds: #566 rejected this as "Linter per-rule AST walks ... plausible but unmeasured; would change public Rule/Context API". The gain is now measured (below: about -26% time, -39% allocs/op, -9% B/op on amd64 CI), and the API change is additive only (a new method; no existing signature changes).

Test Plan

  • task check, task test:cbinding, go test -count=1 ./pkg/compatibility/..., go run honnef.co/go/tools/cmd/staticcheck@latest ./pkg/linter/...: all pass after merging origin/main (69a5a72, W2-01) and the pointer-keyed cache.
  • New tests: TestContextNodesInspectOrder (preorder identity vs ast.Inspect on 3,574 parsed inputs, all 8 dialects), TestContextNodesNilAndReplacement, TestContextNodesDirectASTAssignment (direct assignment and by-value copy), concurrent-Contexts test under -race. Mutation checks (pruning children, dropping the reset, top-level-only iteration) were caught by the tests.
  • All checks on the PR head b1bbae6 are green, including Test (ubuntu/macos/windows), Race Detector, CGO cbinding, Lint, CodeQL, Fuzz.

Local A/B (M4, shared machine, indicative only): task bench:ab BASE=origin/main BENCH='./pkg/linter=^BenchmarkLintRW$' (10 rounds, 200ms)

LintRW time/stmt B/op allocs/op
postgresql -32.56% -9.64% -40.06%
mysql -29.16% -8.23% -39.56%
mariadb -29.52% -8.28% -39.58%
sqlserver -26.66% -10.00% -38.86%
oracle -25.74% -8.92% -38.72%
sqlite -28.96% -9.57% -39.64%
snowflake -28.51% -8.95% -39.92%
clickhouse -27.97% -9.43% -39.51%
geomean -28.66% -9.13% -39.48%

All p=0.000, n=10.

CI perf gate, PR run (shadow, amd64): passed, 0 of 26 benchmarks moved, geomean +0.13%, allocs/op and B/op unchanged. Stated plainly: the gate suite (scripts/perf/gate-suite.txt) has no lint benchmark, so this run does NOT measure the target.

CI perf gate with the target included (workflow_dispatch run 41332537014, amd64, base origin/main vs a throwaway branch = this PR + one extra suite line ./pkg/linter ^BenchmarkLintRW$; the branch is deleted and the suite line is not part of this PR):

LintRW time p B/op allocs/op
postgresql -25.28% <0.001 -9.64% 60867 -> 36481
mysql -25.33% <0.001 -8.24% 73146 -> 44201
mariadb -27.68% <0.001 -8.28% 67443 -> 40743
sqlserver -26.74% <0.001 -9.99% 60525 -> 36994
oracle -25.03% <0.001 -8.90% 73952 -> 45297
sqlite -28.55% <0.001 -9.54% 67284 -> 40584
snowflake -27.66% <0.001 -8.95% 66361 -> 39865
clickhouse -27.30% <0.001 -9.43% 74386 -> 44997
geomean (time) -26.71%

No other gated benchmark regressed in that run (time geomean +0.07% over the full suite). Meets the PLAN W6-05(a) target of <= -20%. Adding BenchmarkLintRW to the permanent gate suite is left for a separate PR.

Breaking Changes

None. Context.Nodes() is additive. Context gains three unexported fields (nodes, nodesAST, nodesBuilt); the exported fields and the Rule interface are unchanged. Violations are identical for every corpus input.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

@ajitpratap0 @claude
perf(linter): shared cached preorder node list for AST rules [W6-05a]
eac81e1 
Thirteen AST-based lint rules each walked the whole tree with ast.Inspect,
so every lint ran the same traversal 13 times. linter.Context gains
Nodes(): every node of every statement in ast.Inspect preorder, built on
first use, cached per Context and reset by WithAST. The 13 rules iterate
that list. None of their Inspect callbacks returned false, so iterating the
flat list visits exactly the nodes in the same order and violations are
identical. Part (b) (aliasing and the 4 Visitor rules) and the L022/L029
scoped walks are unchanged.

Context.Nodes is public-additive and documents its lifetime: valid for the
Context's AST, read-only, not shared across Contexts.

Tests: Nodes() order equals ast.Inspect order over the real-world corpora;
nil AST/statement handling and cache reset on WithAST; concurrent use of
separate Contexts under -race. Before submission, ordered violations from
LintWithDialect were verified byte-identical to origin/main 319b55b on
3,740 inputs (all 8 dialects, statements and whole files).

M4, GOGC=off -cpu 1, 10 interleaved rounds, LintWithDialect per dialect:
-25.5% to -32.6% (geomean -27.9%), allocs -37.3% to -40.1%, B/op -8% to
-10%.

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

vercel Bot commented Oct 31, 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 2, 2026 9:00am UTC

@github-actions

github-actions Bot commented Oct 31, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Review of PR #906: shared cached preorder node list

Verdict: looks good. I found no blocking issues.

What I checked

  • Context.Nodes() in pkg/linter/context.go collects nodes with the same ast.Inspect callback (nil nodes skipped, always returns true), so the order matches what the rules saw before. I read the converted drop_without_condition.go diff and the new tests. I did not read the other 12 converted rules, and I did not run the test suite or benchmarks.
  • WithAST resets both nodes and nodesBuilt, so replacing the AST (including with nil) invalidates the cache. TestContextNodesNilAndReplacement covers this.
  • The cache has no locking. pkg/linter/linter.go has no goroutines or sync use, so rules run sequentially on one Context. That is safe today. The doc comment says a Context must not be used concurrently, which is the right contract.
  • Converting each rule from the ast.Inspect closure to a for range ctx.Nodes() loop is straightforward. drop_without_condition.go shows the pattern: same switch bodies, one less nesting level. The equivalence argument (no callback returned false) is sound.

Minor suggestions (non-blocking)

  1. Stale cache on direct field assignment. Context.AST is an exported field. Code that assigns ctx.AST = x directly, instead of calling WithAST, keeps a stale Nodes() list. Either note this in the Nodes doc comment or key the cache on the AST pointer (nodesFor *ast.AST). Keying on the pointer is cheap and removes the footgun.
  2. Capacity hint. make([]ast.Node, 0, 64) is a reasonable guess. A larger hint for multi-statement files, such as max(64, 16*len(Statements)), could avoid a few regrowths. This is optional, and the benchmarks already show a large win.
  3. Concurrency test. TestContextNodesSeparateContextsConcurrent only builds trivial ASTs. That is fine for the race-detector purpose, but it does not exercise concurrent Nodes() on parsed trees. It is low value to expand.
  4. Remaining rules. The remaining for _, stmt := range ctx.AST.Statements loops (union_all_preferred, subquery_can_be_join, leading_wildcard, and others) are out of scope per the description. Part (b) can pick them up if they fit the same shape.

Process note: the equivalence golden over 3,740 inputs was a one-off run against the base commit and is not in the repo. Since it is not committed, TestContextNodesInspectOrder plus the W0-02 goldens are the only guard from here on. That is acceptable given your merge-ordering plan.
· branch perf/w6-05a-lint-context-nodes

@ajitpratap0
ajitpratap0 marked this pull request as ready for review November 1, 2026 17:10
@github-actions

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

Copy link
Copy Markdown

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

Review of PR #906: shared cached preorder node list

Verdict: looks good, no blocking issues.

What I checked

  • I read the git diff for context.go, nodes_order_test.go, table_alias_required.go and missing_where.go. I did not read the other 14 changed files.
  • I did not run the tests or benchmarks.

Correctness

  • Context.Nodes() uses the same callback as the old rule code: it skips nil nodes and always returns true. Because no rule pruned a subtree, a flat preorder list visits the same nodes in the same order.
  • WithAST resets both nodes and nodesBuilt.
  • Nodes() marks the cache built before checking for a nil AST, so a nil AST is also cached. That is fine, because WithAST resets it.
  • The two converted rules I read keep the same logic. Each return true became continue, and the diff is otherwise the same code one nesting level shallower.

Tests

  • TestContextNodesInspectOrder compares node identity (got[i] != want[i]) over the corpora, and it also checks that the slice is cached.
  • TestContextNodesNilAndReplacement covers nil statements, replacing the AST, and clearing it.
  • The concurrency test only builds trivial trees. That is enough for its purpose of checking that Contexts are independent.

Non-blocking suggestions

  1. Stale cache on direct assignment. Context.AST is exported. Code that sets ctx.AST = x directly, instead of calling WithAST, keeps a stale Nodes() list. Either say so in the doc comment, or store the AST pointer the cache was built from (nodesFor *ast.AST) and compare it in Nodes().
  2. Capacity hint. The fixed make([]ast.Node, 0, 64) could be scaled with the statement count, for example max(64, 16*len(Statements)). This is minor, given the measured gains.
  3. Memory held. The cached slice keeps references to AST nodes for the life of the Context. If callers release the AST while the Context is still alive, the nodes stay reachable. If a Context could outlive its AST, you may want a doc note or a Release path. In linter.go, Context appears to be short-lived, so this is likely fine.
  4. Golden tests. The 3,740-input equivalence run is not committed, so the order test plus the W0-02 goldens are the guard from here on. That matches your merge-ordering plan.
    · branch perf/w6-05a-lint-context-nodes

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Nov 1, 2026
@github-actions

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

Copy link
Copy Markdown

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

Base 24879bd9862e, head 521ee73f6654. 10 interleaved rounds, benchtime 200ms, -cpu 1, pinned to CPU 3 (SMT siblings 2-3) with taskset.

  • 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.38% 0.031 -0.32% +0.00% 5 → 5 ok
ParseE2E/select_where -0.42% 0.140 -0.09% +0.00% 18 → 18 ok
ParseE2E/join +0.03% 0.850 -0.30% +0.00% 17 → 17 ok
ParseE2E/aggregate -0.27% 0.472 -0.34% +0.00% 18 → 18 ok
ParseE2E/subquery -0.09% 0.910 -0.46% +0.00% 14 → 14 ok
ParseE2E/cte -0.19% 0.762 -0.13% +0.00% 21 → 21 ok
ParseE2E/window -0.22% 0.970 -0.12% +0.00% 11 → 11 ok
ParseE2E/complex +0.44% 0.104 +0.36% +0.00% 50 → 50 ok
ParseE2E/insert +0.11% 0.762 +0.13% +0.00% 17 → 17 ok
ParseE2E/update +0.66% 0.064 +0.13% +0.00% 13 → 13 ok
ParseE2E/delete +1.00% 0.003 +0.55% +0.00% 5 → 5 ok
ParseRW/postgresql/medium -0.71% 0.089 +0.35% +0.00% 646 → 646 ok
ParseRW/mysql/medium -0.05% 1.000 -0.05% +0.00% 365 → 365 ok
ParseRW/mariadb/medium -0.86% 0.001 +0.05% +0.00% 523 → 523 ok
ParseRW/sqlserver/medium -0.63% 0.043 +0.08% -0.00% 936 → 936 ok
ParseRW/oracle/medium -0.11% 0.739 +0.31% -0.00% 1093 → 1093 ok
ParseRW/sqlite/medium -0.97% 0.089 -0.22% +0.00% 346 → 346 ok
ParseRW/snowflake/medium -0.57% 0.089 +0.28% +0.00% 777 → 777 ok
ParseRW/clickhouse/medium -0.79% 0.063 -0.21% +0.00% 384 → 384 ok
ParseFresh/simple_select +0.79% 0.143 -0.29% +0.00% 6 → 6 ok
ParseFresh/join +0.59% 0.226 +0.07% +0.00% 18 → 18 ok
ParseFresh/complex +0.36% 0.496 +0.61% +0.00% 51 → 51 ok
PoolGetPut/serial +2.41% <0.001 -0.17% +0.00% 0 → 0 ok
Tokenize/scratch/join -0.11% 0.139 +0.06% +0.00% 0 → 0 ok
Tokenize/scratch/complex +0.09% 0.820 +0.23% +0.00% 0 → 0 ok
Tokenize/scratch/insert -0.08% 0.469 +0.20% +0.00% 0 → 0 ok
Normalize/simple_select +0.45% 0.289 +0.22% +0.00% 10 → 10 ok
Normalize/join +0.94% 0.010 -0.27% +0.00% 40 → 40 ok
Normalize/complex +1.22% 0.043 +0.77% +0.00% 110 → 110 ok
TransformParse/simple_select +0.44% 0.150 +0.08% +0.00% 5 → 5 ok
TransformParse/join +0.72% 0.019 +0.17% +0.00% 17 → 17 ok
TransformParse/complex +1.03% 0.002 +0.46% +0.00% 50 → 50 ok
geomean +0.17% 0.280 +0.07%

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

Instructions per operation (cmd/perfdriver)

case instructions/op status
e2e/aggregate +0.01% ok
e2e/complex +0.05% ok
e2e/cte +0.01% ok
e2e/delete +0.08% ok
e2e/insert -0.00% ok
e2e/join +0.03% ok
e2e/select_where -0.02% ok
e2e/simple_select -0.02% ok
e2e/subquery -0.01% ok
e2e/update +0.09% ok
e2e/window +0.04% ok
rw/postgresql/medium -0.03% ok
Secondary metric: default GC, GOMAXPROCS=1 (informational)
benchmark time p
ParseE2E/simple_select -0.41% 0.739
ParseE2E/select_where -1.20% 0.112
ParseE2E/join -0.22% 0.481
ParseE2E/aggregate -0.52% 0.579
ParseE2E/subquery -1.35% 0.190
ParseE2E/cte -0.74% 0.218
ParseE2E/window -0.79% 0.064
ParseE2E/complex -1.34% 0.218
ParseE2E/insert -3.04% 0.011
ParseE2E/update -1.50% 0.290
ParseE2E/delete +1.16% 0.089
ParseRW/postgresql/medium -1.70% 0.043
ParseRW/mysql/medium -2.77% <0.001
ParseRW/mariadb/medium -1.26% 0.063
ParseRW/sqlserver/medium -0.42% 0.105
ParseRW/oracle/medium -1.15% 0.043
ParseRW/sqlite/medium -2.57% 0.011
ParseRW/snowflake/medium -1.24% 0.019
ParseRW/clickhouse/medium -2.38% 0.089
ParseFresh/simple_select +0.88% 0.034
ParseFresh/join +0.66% 0.089
ParseFresh/complex -0.15% 0.912
PoolGetPut/serial +1.59% 0.003
Tokenize/scratch/join -0.15% 0.287
Tokenize/scratch/complex -0.22% 0.075
Tokenize/scratch/insert -0.12% 0.939
Normalize/simple_select +0.55% 0.733
Normalize/join +0.43% 0.364
Normalize/complex -1.12% 0.199
TransformParse/simple_select -0.40% 0.280
TransformParse/join -0.09% 0.684
TransformParse/complex -0.11% 0.796
geomean -0.68%
Environment 
date:        2026-11-02T09:02:11Z
base:        24879bd9862e92ebfb51d8652fea5ca33343d3e1
head:        working tree of 521ee73f66543995ba3765888fc759174afa12d8
go:          go version go1.26.1 linux/amd64
kernel:      Linux 7.0.0-1017-azure x86_64
pinning:     pinned to CPU 3 (SMT siblings 2-3) with taskset
clocksource: tsc
load:        1.32, 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.916µ ± 1%   2.929µ ± 1%       ~ (p=0.288 n=10)
Normalize/join            10.82µ ± 1%   10.92µ ± 2%  +0.94% (p=0.007 n=10)
Normalize/complex         27.17µ ± 1%   27.50µ ± 1%       ~ (p=0.043 n=10)
geomean                   9.500µ        9.582µ       +0.87%

                        │     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.858µ ± 1%   1.865µ ± 1%       ~ (p=0.027 n=10)
ParseE2E/select_where       3.917µ ± 1%   3.900µ ± 1%       ~ (p=0.137 n=10)
ParseE2E/join               5.882µ ± 1%   5.884µ ± 1%       ~ (p=0.838 n=10)
ParseE2E/aggregate          4.287µ ± 1%   4.276µ ± 1%       ~ (p=0.481 n=10)
ParseE2E/subquery           3.706µ ± 1%   3.702µ ± 1%       ~ (p=0.896 n=10)
ParseE2E/cte                5.141µ ± 1%   5.131µ ± 2%       ~ (p=0.754 n=10)
ParseE2E/window             3.360µ ± 1%   3.352µ ± 1%       ~ (p=0.954 n=10)
ParseE2E/complex            15.80µ ± 1%   15.87µ ± 0%       ~ (p=0.101 n=10)
ParseE2E/insert             2.610µ ± 1%   2.613µ ± 1%       ~ (p=0.753 n=10)
ParseE2E/update             2.359µ ± 0%   2.374µ ± 1%       ~ (p=0.060 n=10)
ParseE2E/delete             1.443µ ± 0%   1.458µ ± 0%  +1.00% (p=0.001 n=10)
ParseRW/postgresql/medium   202.6µ ± 1%   201.2µ ± 1%       ~ (p=0.089 n=10)
ParseRW/mysql/medium        111.4µ ± 1%   111.4µ ± 1%       ~ (p=1.000 n=10)
ParseRW/mariadb/medium      164.4µ ± 1%   163.0µ ± 1%  -0.86% (p=0.001 n=10)
ParseRW/sqlserver/medium    343.9µ ± 1%   341.8µ ± 1%       ~ (p=0.043 n=10)
ParseRW/oracle/medium       401.3µ ± 1%   400.8µ ± 0%       ~ (p=0.739 n=10)
ParseRW/sqlite/medium       95.15µ ± 2%   94.23µ ± 1%       ~ (p=0.089 n=10)
ParseRW/snowflake/medium    235.0µ ± 1%   233.7µ ± 2%       ~ (p=0.089 n=10)
ParseRW/clickhouse/medium   108.1µ ± 1%   107.2µ ± 1%       ~ (p=0.063 n=10)
ParseFresh/simple_select    10.14µ ± 2%   10.22µ ± 1%       ~ (p=0.143 n=10)
ParseFresh/join             23.59µ ± 2%   23.73µ ± 1%       ~ (p=0.225 n=10)
ParseFresh/complex          36.39µ ± 1%   36.52µ ± 1%       ~ (p=0.494 n=10)
PoolGetPut/serial           65.93n ± 0%   67.52n ± 0%  +2.41% (p=0.000 n=10)
geomean                     15.03µ        15.03µ       +0.04%

                          │     base     │                head                 │
                          │     B/s      │     B/s       vs base               │
ParseE2E/simple_select      13.35Mi ± 1%   13.30Mi ± 1%       ~ (p=0.027 n=10)
ParseE2E/select_where       25.08Mi ± 1%   25.19Mi ± 1%       ~ (p=0.128 n=10)
ParseE2E/join               20.43Mi ± 1%   20.42Mi ± 1%       ~ (p=0.810 n=10)
ParseE2E/aggregate          18.69Mi ± 1%   18.73Mi ± 1%       ~ (p=0.492 n=10)
ParseE2E/subquery           21.10Mi ± 1%   21.12Mi ± 1%       ~ (p=0.896 n=10)
ParseE2E/cte                23.00Mi ± 1%   23.05Mi ± 2%       ~ (p=0.753 n=10)
ParseE2E/window             26.68Mi ± 1%   26.75Mi ± 1%       ~ (p=0.986 n=10)
ParseE2E/complex            22.82Mi ± 1%   22.72Mi ± 0%       ~ (p=0.109 n=10)
ParseE2E/insert             27.78Mi ± 1%   27.74Mi ± 1%       ~ (p=0.754 n=10)
ParseE2E/update             22.24Mi ± 0%   22.09Mi ± 1%       ~ (p=0.066 n=10)
ParseE2E/delete             34.37Mi ± 0%   34.02Mi ± 0%  -1.03% (p=0.001 n=10)
ParseRW/postgresql/medium   17.70Mi ± 1%   17.83Mi ± 1%       ~ (p=0.072 n=10)
ParseRW/mysql/medium        24.29Mi ± 1%   24.30Mi ± 1%       ~ (p=1.000 n=10)
ParseRW/mariadb/medium      21.56Mi ± 1%   21.75Mi ± 1%  +0.88% (p=0.001 n=10)
ParseRW/sqlserver/medium    19.35Mi ± 1%   19.47Mi ± 1%       ~ (p=0.045 n=10)
ParseRW/oracle/medium       20.89Mi ± 1%   20.91Mi ± 0%       ~ (p=0.669 n=10)
ParseRW/sqlite/medium       21.71Mi ± 2%   21.92Mi ± 1%       ~ (p=0.085 n=10)
ParseRW/snowflake/medium    21.63Mi ± 1%   21.76Mi ± 2%       ~ (p=0.092 n=10)
ParseRW/clickhouse/medium   20.85Mi ± 1%   21.01Mi ± 1%       ~ (p=0.060 n=10)
geomean                     21.90Mi        21.94Mi       +0.17%

                          │      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.52Ki ± 0%     70.52Ki ± 0%       ~ (p=0.942 n=10)
ParseRW/mysql/medium        43.73Ki ± 0%     43.73Ki ± 0%       ~ (p=1.000 n=10)
ParseRW/mariadb/medium      64.85Ki ± 0%     64.85Ki ± 0%       ~ (p=0.700 n=10)
ParseRW/sqlserver/medium    90.61Ki ± 0%     90.61Ki ± 0%       ~ (p=0.337 n=10)
ParseRW/oracle/medium       113.8Ki ± 0%     113.8Ki ± 0%       ~ (p=0.350 n=10)
ParseRW/sqlite/medium       39.89Ki ± 0%     39.89Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/snowflake/medium    83.38Ki ± 0%     83.38Ki ± 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    646.0 ± 0%      646.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/mysql/medium         365.0 ± 0%      365.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/mariadb/medium       523.0 ± 0%      523.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/sqlserver/medium     936.0 ± 0%      936.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/oracle/medium       1.093k ± 0%     1.093k ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/sqlite/medium        346.0 ± 0%      346.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/snowflake/medium     777.0 ± 0%      777.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   16.89µ ± 1%   16.77µ ± 1%       ~ (p=0.089 n=10)
ParseRW/mysql/medium        13.93µ ± 1%   13.92µ ± 1%       ~ (p=1.000 n=10)
ParseRW/mariadb/medium      18.27µ ± 1%   18.12µ ± 1%  -0.86% (p=0.001 n=10)
ParseRW/sqlserver/medium    16.38µ ± 1%   16.27µ ± 1%       ~ (p=0.043 n=10)
ParseRW/oracle/medium       16.72µ ± 1%   16.70µ ± 0%       ~ (p=0.700 n=10)
ParseRW/sqlite/medium       15.86µ ± 2%   15.71µ ± 1%       ~ (p=0.089 n=10)
ParseRW/snowflake/medium    18.08µ ± 1%   17.97µ ± 2%       ~ (p=0.089 n=10)
ParseRW/clickhouse/medium   15.44µ ± 1%   15.32µ ± 1%       ~ (p=0.055 n=10)
geomean                     16.39µ        16.29µ       -0.59%

pkg: github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer
                         │    base     │                head                │
                         │   sec/op    │   sec/op     vs base               │
Tokenize/scratch/join      2.636µ ± 0%   2.633µ ± 0%       ~ (p=0.138 n=10)
Tokenize/scratch/complex   7.119µ ± 0%   7.125µ ± 1%       ~ (p=0.810 n=10)
Tokenize/scratch/insert    1.265µ ± 0%   1.264µ ± 0%       ~ (p=0.469 n=10)
geomean                    2.873µ        2.872µ       -0.03%

                         │     base     │                head                 │
                         │     B/s      │     B/s       vs base               │
Tokenize/scratch/join      45.59Mi ± 0%   45.65Mi ± 0%       ~ (p=0.118 n=10)
Tokenize/scratch/complex   50.64Mi ± 0%   50.59Mi ± 1%       ~ (p=0.753 n=10)
Tokenize/scratch/insert    57.31Mi ± 0%   57.36Mi ± 0%       ~ (p=0.447 n=10)
geomean                    50.95Mi        50.98Mi       +0.04%

                         │     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.801µ ± 1%   1.809µ ± 0%       ~ (p=0.148 n=10)
TransformParse/join            6.263µ ± 1%   6.308µ ± 0%       ~ (p=0.015 n=10)
TransformParse/complex         16.73µ ± 0%   16.90µ ± 3%  +1.03% (p=0.001 n=10)
geomean                        5.735µ        5.777µ       +0.73%

                             │     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 23:05
@ajitpratap0
Merge origin/main into perf/w6-05a-lint-context-nodes
58f5f58
@ajitpratap0 @claude
perf(linter): key Context.Nodes cache on the AST pointer [W6-05a]
b1bbae6 
Direct ctx.AST assignment or a by-value Context copy with a new AST can no
longer return a stale node list.

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 •
edited
Loading

Copy link
Copy Markdown

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

Review of PR #906: shared cached preorder node list

Verdict: looks good, no blocking issues. The latest commit (b1bbae6) keys the cache on the AST pointer, which resolves the stale-cache concern from the earlier reviews.

What I checked

  • I read the diff for context.go, nodes_order_test.go, nodes_corpus_test.go, bench_matrix_test.go and the CHANGELOG.
  • I did not re-read the 13 converted rules in this pass. I did not run the tests or benchmarks.
  • The perf gate comment reports PASS, with no change in allocs/op or B/op.

Correctness

  • Nodes() checks nodesBuilt && nodesAST == AST. Direct ctx.AST = x assignment now invalidates the cache, and a nil AST is cached correctly.
  • Rebuilding sets c.nodes = nil before appending, so a by-value copy (cp := *ctx) never appends into a slice shared with the original. TestContextNodesDirectASTAssignment covers this.
  • The callback matches the old rule walks (skips nil nodes, always returns true), so preorder is unchanged.
  • TestLintContextHasNoCachedState is updated for the three new fields, so the field guard still works.

Non-blocking suggestions

  1. Pointer reuse with pooled ASTs. ast.NewAST() and ast.ReleaseAST() use a sync.Pool. If a caller keeps one Context, releases its AST, and gets a recycled *ast.AST at the same address, nodesAST == AST stays true. The cache then returns nodes from the old tree. The doc already says in-place mutation does not invalidate. I'd add one sentence saying a Context must not outlive a release of its AST, or that callers should call WithAST after reuse. linter.go creates short-lived Contexts, so this is only a latent issue.
  2. Doc comment formatting. In the Nodes comment, the line "…mutating the tree in place does not. Separate Contexts have independent caches;" is much longer than the surrounding lines. It looks like a reflow leftover. gofmt won't complain, but a re-wrap would read better.
  3. Stale comment in the benchmark test. The TestLintContextHasNoCachedState doc still says "When a field such as a cached node list is added (PLAN W6-05) this fails". That has now happened, so the wording could be updated.
  4. Capacity hint. make([]ast.Node, 0, 64) could scale with the statement count. This is optional, since the benchmarks already show a win.
  5. Equivalence golden. The 3,740-input run is not committed. TestContextNodesInspectOrder compares node identity across all 8 dialect corpora, so that is a reasonable guard.

· · branch perf/w6-05a-lint-context-nodes

@ajitpratap0 @claude
Merge origin/main into perf/w6-05a-lint-context-nodes
71bc2c5 
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 2, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Review of PR #906: shared cached preorder node list

Verdict: looks good, no blocking issues.

What I checked

  • I read the diff for context.go, bench_matrix_test.go and nodes_order_test.go.
  • I did not re-read the 13 converted rules in this pass. I did not run the tests or benchmarks.

Correctness

  • Nodes() checks nodesBuilt && nodesAST == AST, so direct ctx.AST = x assignment and by-value copies with a new AST rebuild the list. WithAST also resets it.
  • Rebuilding sets c.nodes = nil before appending, so a by-value copy never appends into the original's backing array.
  • The callback matches the old rule walks: it skips nil nodes and always returns true, so the order is the same as ast.Inspect.
  • TestContextNodesInspectOrder checks node identity against ast.Inspect on the corpus, and TestLintContextHasNoCachedState is updated for the three new fields.

Non-blocking suggestions

  1. Pooled AST pointer reuse. If a Context outlives ast.ReleaseAST and ast.NewAST() returns the same address, nodesAST == AST stays true and the cache returns nodes from the old tree. linter.go uses short-lived Contexts, so this is only latent. One doc sentence would cover it: a Context must not outlive a release of its AST, or callers should call WithAST after reuse.
  2. Doc comment wrapping. In Nodes() (context.go, ~line 165), the line "…does not. Separate Contexts have independent caches;" is much longer than the lines around it. It looks like a reflow leftover.
  3. Stale test comment. The TestLintContextHasNoCachedState doc comment in bench_matrix_test.go still says "When a field such as a cached node list is added (PLAN W6-05) this fails". That has now happened, so the wording could be updated.
  4. Capacity hint. make([]ast.Node, 0, 64) could scale with the statement count. This is optional, given the measured gains.
  5. Concurrency test. TestContextNodesSeparateContextsConcurrent only uses trivial ASTs. That is enough to show that Contexts are independent.
    · branch perf/w6-05a-lint-context-nodes

@ajitpratap0
ajitpratap0 merged commit 68a04b4 into main Nov 2, 2026
48 checks passed
@ajitpratap0
ajitpratap0 deleted the perf/w6-05a-lint-context-nodes branch November 2, 2026 09:12
This was referenced Nov 2, 2026
Merged
Open
ajitpratap0 added a commit that referenced this pull request Nov 3, 2026
@ajitpratap0 @claude
ci(perf): gate LintRW, LintRule L007/L010 and ParseError in the shado…
01ee824 
…w suite (#927)

* ci(perf): add BenchmarkLintRW to the shadow gate suite

PR #906's gate run moved 0 of 26 benchmarks because no linter benchmark was
gated. Add it so W6-05b, W6-06 and W6-07 are measured by the PR-run gate.

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

* ci(perf): gate LintRule L007/L010 and ParseError in the shadow suite

#896 (L007), #902 (L010) and #905 (lazy error wrap) target benchmarks the
PR-run gate never measured, so none of them could be accepted or closed
on an amd64 verdict. long_prefix is included so #905's expected miss is
still checked for regressions.

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>

This branch was successfully deployed

1 active deployment
Preview — 71bc2c52 Deployed Nov 2, 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 linter performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ajitpratap0