pkg/parser: cross-commit benchmark shows sec/op and B/op regressions after hand parser rewrite
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
### 1. Minimal reproduce step (Required)
Context for why this benchmark exists:
- Commit `d87ca30fdc700a6c44d7ccc7e398205e20bbc8e0` removed the old goyacc parser path and old parser benchmarks, and replaced them with `BenchmarkHandParser`.
- So the merged tree no longer has an in-tree old-vs-new benchmark path.
- I could not find concrete final `benchstat` numbers in PR discussion for sec/op and B/op deltas.
- Therefore I ran a cross-commit same-harness benchmark to get reproducible before/after data.
Compared commits:
- before: `cea5e203ea01817f07c155eea70ad71b6558edb3`
- after: `d87ca30fdc700a6c44d7ccc7e398205e20bbc8e0`
Environment:
- Host: `darwin/arm64` (Apple M4 Max)
- Go: `go1.25.6`
Benchmark harness used (same file on both commits):
```go
package parser_test
import (
"testing"
"github.com/pingcap/tidb/pkg/parser"
_ "github.com/pingcap/tidb/pkg/parser/test_driver"
)
type parserABCase struct {
name string
sql string
}
var parserABCases = []parserABCase{
{name: "SimpleSelect", sql: "SELECT a FROM t WHERE a = 1"},
{name: "SimpleInsert", sql: "INSERT INTO t VALUES (1), (2), (3)"},
{name: "SimpleUpdate", sql: "UPDATE t SET a = 1 WHERE b = 2"},
{name: "SimpleDelete", sql: "DELETE FROM t WHERE a = 1"},
{name: "SelectComplex", sql: "SELECT a, b, c FROM t1 JOIN t2 ON t1.id = t2.id WHERE t1.a > 1 AND t2.b < 10 ORDER BY t1.a LIMIT 100"},
{name: "SelectWhere3", sql: "SELECT a FROM t WHERE a = 1 AND b = 2 AND c = 3"},
{name: "CreateTable", sql: "CREATE TABLE t_bench (id INT PRIMARY KEY, b VARCHAR(20), KEY idx_b(b))"},
{name: "WithCTE", sql: "WITH cte AS (SELECT a FROM t WHERE a > 1) SELECT * FROM cte"},
}
func BenchmarkParserAB(b *testing.B) {
for _, tc := range parserABCases {
tc := tc
b.Run(tc.name, func(b *testing.B) {
p := parser.New()
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
stmts, _, err := p.Parse(tc.sql, "", "")
if err != nil {
b.Fatalf("parse failed for %s: %v", tc.name, err)
}
if len(stmts) == 0 {
b.Fatalf("no statement parsed for %s", tc.name)
}
}
})
}
}
```
Repro commands:
```bash
git worktree add /tmp/tidb-parser-before cea5e203ea01817f07c155eea70ad71b6558edb3
git worktree add /tmp/tidb-parser-after d87ca30fdc700a6c44d7ccc7e398205e20bbc8e0
# put identical bench_ab_test.go into both worktrees under pkg/parser/
pushd /tmp/tidb-parser-before/pkg/parser
GOTOOLCHAIN=go1.25.6 go test -run '^$' -bench '^BenchmarkParserAB$' -benchmem -count=20 -benchtime=2s -cpu=1 > /tmp/parser-before.txt
popd
pushd /tmp/tidb-parser-after/pkg/parser
GOTOOLCHAIN=go1.25.6 go test -run '^$' -bench '^BenchmarkParserAB$' -benchmem -count=20 -benchtime=2s -cpu=1 > /tmp/parser-after.txt
popd
benchstat /tmp/parser-before.txt /tmp/parser-after.txt
```
### 2. What did you expect to see? (Required)
No major regression in `sec/op` and `B/op` on the same parse API path (`Parser.Parse`), while ideally reducing `allocs/op`.
### 3. What did you see instead (Required)
`benchstat` geomean:
| Metric | Before | After | Delta |
|---|---:|---:|---:|
| sec/op | 2.655µ | 4.336µ | +63.33% |
| B/op | 2.927Ki | 32.29Ki | +1003.00% |
| allocs/op | 26.11 | 18.51 | -29.09% |
So allocation count improved, but runtime and allocated bytes regressed significantly.
Per-case trend in this harness:
- `sec/op`: all 8 cases regressed.
- `B/op`: all 8 cases regressed.
- `allocs/op`: all 8 cases improved.
Additional signal:
- Built-in `BenchmarkHandParser` in `after` also shows high `B/op` (e.g. `SimpleSelect` around `42440 B/op`).
### 4. What is your TiDB version? (Required)
Source-level parser benchmark between commits:
- before: `cea5e203ea01817f07c155eea70ad71b6558edb3`
- after: `d87ca30fdc700a6c44d7ccc7e398205e20bbc8e0`
---
### Extra diagnostic context
A temporary experiment in a throwaway worktree suggests regression is strongly correlated with slab reset behavior in `pkg/parser/arena.go` (`slab.reset()` dropping backing storage every parse).
That experiment greatly improved benchmark numbers, but naive reuse caused parser correctness failures in full tests (`go test ./...` in `pkg/parser`), so it is not a ready fix.
Still, it suggests allocator lifecycle/reset behavior is a key area to investigate.
Contributor guide
Assessment
This issue has not been assessed yet.