Replace quadratic string concatenation in qualified struct type names by dmitriplotnikov · Pull Request #1607 · cel-expr/cel-go

dmitriplotnikov · 2026-11-02T23:50:44Z

Builds on #1605.

Build qualified message type names with a pre-sized strings.Builder in antlrVisitor.VisitCreateMessage instead of repeated += string concatenation.
Assemble qualified struct type names in a single strings.Builder pass in prattParserWorker.extractStructName instead of name = "." + sel.FieldName() + name, and delete intermediate Select/Ident IDs inline.
Add TestParseDeepQualifiedStructName and extend BenchmarkParseLargePathological.

Benchmarks (`go test -bench='^BenchmarkParseLargePathological/.*/deep_struct_name$' -benchmem`)

goos: linux · goarch: amd64 · pkg: cel.dev/cel-go/parser · cpu: AMD EPYC 7B13

Benchmark	`before.txt` (`sec/op`)	`after.txt` (`sec/op`)	vs base
`ParseLargePathological/antlr/deep_struct_name-64`	739.35m ± 25%	45.11m ± 9%	-93.90% (p=0.002 n=6)
`ParseLargePathological/pratt/deep_struct_name-64`	316.84m ± 6%	26.88m ± 2%	-91.52% (p=0.002 n=6)
geomean	484.0m	34.82m	-92.81%

Benchmark	`before.txt` (`B/op`)	`after.txt` (`B/op`)	vs base
`ParseLargePathological/antlr/deep_struct_name-64`	5100.91Mi ± 0%	37.72Mi ± 0%	-99.26% (p=0.002 n=6)
`ParseLargePathological/pratt/deep_struct_name-64`	2539.39Mi ± 0%	11.94Mi ± 0%	-99.53% (p=0.002 n=6)
geomean	3.515Gi	21.22Mi	-99.41%

Benchmark	`before.txt` (`allocs/op`)	`after.txt` (`allocs/op`)	vs base
`ParseLargePathological/antlr/deep_struct_name-64`	300.2k ± 0%	200.2k ± 0%	-33.31% (p=0.002 n=6)
`ParseLargePathological/pratt/deep_struct_name-64`	300.0k ± 0%	250.1k ± 0%	-16.66% (p=0.002 n=6)
geomean	300.1k	223.7k	-25.45%

- Check `sourceInfo.ComputeOffset(1, 0) != 0` once in `newParserHelper` to detect offset-shifted `RelativeSource` inputs with zero extra allocations, making `computeOffset` an `O(1)` no-op on standard sources while preserving embedded policy source offset translation. - Add `newOffsetRangeID(start, stop int32)` and update `exprHelper.nextMacroID` and `exprHelper.Copy` to record `OffsetRange` directly without round-tripping through `common.Location` or boxing `ast.OffsetRange` into `any`. - Add `TestParseMultilineMacrosAndMaps` and `BenchmarkParseLargePathological`. Benchmarks (`go test -bench='^Benchmark(ByCategory|ParseLargePathological)$' -benchmem`): goos: linux goarch: amd64 pkg: cel.dev/cel-go/parser cpu: Intel(R) Xeon(R) CPU @ 2.20GHz │ before.txt │ after.txt │ │ sec/op │ sec/op vs base │ ByCategory/antlr/Simple-8 571.0µ ± 17% 359.0µ ± 14% -37.13% (p=0.002 n=6) ByCategory/antlr/Complex-8 2.522m ± 5% 1.802m ± 4% -28.56% (p=0.002 n=6) ByCategory/antlr/Macros-8 576.2µ ± 16% 442.8µ ± 4% -23.15% (p=0.002 n=6) ByCategory/antlr/Errors-8 1.922m ± 11% 1.843m ± 6% ~ (p=0.093 n=6) ByCategory/pratt/Simple-8 36.08µ ± 65% 29.33µ ± 5% -18.69% (p=0.002 n=6) ByCategory/pratt/Complex-8 190.9µ ± 9% 137.7µ ± 7% -27.84% (p=0.002 n=6) ByCategory/pratt/Macros-8 76.53µ ± 9% 53.65µ ± 4% -29.90% (p=0.002 n=6) ByCategory/pratt/Errors-8 108.23µ ± 5% 93.53µ ± 3% -13.58% (p=0.002 n=6) ParseLargePathological/antlr/multiline_macros-8 117.7m ± 33% 112.1m ± 3% -4.73% (p=0.002 n=6) ParseLargePathological/antlr/multiline_map-8 255.4m ± 9% 244.4m ± 15% ~ (p=0.699 n=6) ParseLargePathological/pratt/multiline_macros-8 27.64m ± 27% 19.00m ± 6% -31.26% (p=0.002 n=6) ParseLargePathological/pratt/multiline_map-8 24.57m ± 5% 18.05m ± 5% -26.54% (p=0.002 n=6) geomean 1.872m 1.469m -21.57% │ before.txt │ after.txt │ │ B/op │ B/op vs base │ ByCategory/antlr/Simple-8 106.6Ki ± 4% 106.8Ki ± 4% ~ (p=0.058 n=6) ByCategory/antlr/Complex-8 506.7Ki ± 0% 506.8Ki ± 0% ~ (p=0.065 n=6) ByCategory/antlr/Macros-8 134.0Ki ± 0% 133.2Ki ± 0% -0.64% (p=0.002 n=6) ByCategory/antlr/Errors-8 581.3Ki ± 0% 581.4Ki ± 0% +0.03% (p=0.041 n=6) ByCategory/pratt/Simple-8 12.77Ki ± 0% 11.47Ki ± 0% -10.16% (p=0.002 n=6) ByCategory/pratt/Complex-8 58.16Ki ± 0% 48.29Ki ± 0% -16.98% (p=0.002 n=6) ByCategory/pratt/Macros-8 23.03Ki ± 0% 20.50Ki ± 0% -11.01% (p=0.002 n=6) ByCategory/pratt/Errors-8 36.94Ki ± 0% 31.31Ki ± 0% -15.24% (p=0.002 n=6) ParseLargePathological/antlr/multiline_macros-8 30.64Mi ± 0% 30.17Mi ± 0% -1.53% (p=0.002 n=6) ParseLargePathological/antlr/multiline_map-8 75.02Mi ± 0% 75.02Mi ± 0% ~ (p=0.818 n=6) ParseLargePathological/pratt/multiline_macros-8 6.970Mi ± 0% 6.164Mi ± 0% -11.56% (p=0.002 n=6) ParseLargePathological/pratt/multiline_map-8 7.211Mi ± 0% 6.066Mi ± 0% -15.87% (p=0.002 n=6) geomean 513.7Ki 477.0Ki -7.16% │ before.txt │ after.txt │ │ allocs/op │ allocs/op vs base │ ByCategory/antlr/Simple-8 1.631k ± 4% 1.631k ± 3% ~ (p=1.000 n=6) ByCategory/antlr/Complex-8 6.744k ± 0% 6.744k ± 0% ~ (p=1.000 n=6) ByCategory/antlr/Macros-8 2.049k ± 0% 1.986k ± 0% -3.07% (p=0.002 n=6) ByCategory/antlr/Errors-8 7.935k ± 0% 7.935k ± 0% ~ (p=1.000 n=6) ByCategory/pratt/Simple-8 389.0 ± 0% 297.0 ± 0% -23.65% (p=0.002 n=6) ByCategory/pratt/Complex-8 1612.0 ± 0% 972.0 ± 0% -39.70% (p=0.002 n=6) ByCategory/pratt/Macros-8 641.0 ± 0% 470.0 ± 0% -26.68% (p=0.002 n=6) ByCategory/pratt/Errors-8 1163.0 ± 0% 793.0 ± 0% -31.81% (p=0.002 n=6) ParseLargePathological/antlr/multiline_macros-8 394.8k ± 0% 364.3k ± 0% -7.74% (p=0.002 n=6) ParseLargePathological/antlr/multiline_map-8 750.5k ± 0% 750.5k ± 0% ~ (p=0.504 n=6) ParseLargePathological/pratt/multiline_macros-8 161.4k ± 0% 108.6k ± 0% -32.70% (p=0.002 n=6) ParseLargePathological/pratt/multiline_map-8 175.2k ± 0% 100.2k ± 0% -42.82% (p=0.002 n=6) geomean 9.707k 7.858k -19.05%

- Build qualified message type names with a pre-sized `strings.Builder` in `antlrVisitor.VisitCreateMessage` instead of repeated `+=` string concatenation. - Assemble qualified struct type names in a single `strings.Builder` pass in `prattParserWorker.extractStructName` instead of `name = "." + sel.FieldName() + name`, and delete intermediate `Select`/`Ident` IDs outermost-to-innermost only after confirming an `ast.IdentKind` root. - Add `TestParseDeepQualifiedStructName` and extend `BenchmarkParseLargePathological`. Benchmarks (`go test -bench='^BenchmarkParseLargePathological/.*/deep_struct_name$' -benchmem`): goos: linux goarch: amd64 pkg: cel.dev/cel-go/parser cpu: Intel(R) Xeon(R) CPU @ 2.20GHz │ before.txt │ after.txt │ │ sec/op │ sec/op vs base │ ParseLargePathological/antlr/deep_struct_name-8 1508.25m ± 4% 79.91m ± 4% -94.70% (p=0.002 n=6) ParseLargePathological/pratt/deep_struct_name-8 654.89m ± 111% 39.74m ± 24% -93.93% (p=0.002 n=6) geomean 993.9m 56.35m -94.33% │ before.txt │ after.txt │ │ B/op │ B/op vs base │ ParseLargePathological/antlr/deep_struct_name-8 5100.91Mi ± 0% 37.72Mi ± 0% -99.26% (p=0.002 n=6) ParseLargePathological/pratt/deep_struct_name-8 2537.86Mi ± 0% 12.28Mi ± 0% -99.52% (p=0.002 n=6) geomean 3.514Gi 21.52Mi -99.40% │ before.txt │ after.txt │ │ allocs/op │ allocs/op vs base │ ParseLargePathological/antlr/deep_struct_name-8 300.2k ± 2% 200.2k ± 0% -33.32% (p=0.002 n=6) ParseLargePathological/pratt/deep_struct_name-8 200.0k ± 0% 150.1k ± 0% -24.97% (p=0.002 n=6) geomean 245.1k 173.3k -29.27%

Collect qualified struct name segments in a pre-allocated slice while deleting intermediate `Select`/`Ident` IDs inline, then reverse and join with `strings.Join` instead of tracking a separate `ids` slice and pre-computing length for `strings.Builder`.

TristonianJones · 2026-11-04T16:43:46Z

-		sourceInfo:  ast.NewSourceInfo(source),
+		sourceInfo:  sourceInfo,
 		nextID:      1,
+		isRelative:  sourceInfo.ComputeOffset(1, 0) != 0,


I'm still a bit wary of this particular line and handling relative source differently. Any reason why we need it in this change or the other one?

Removing this part of the change from this PR. It belongs in PR #1605 anyway. Will address your concer in that PR.

Remove the isRelative field and ComputeOffset(1, 0) != 0 check from newParserHelper and restore the unconditional OffsetLocation translation in computeOffset.

dmitriplotnikov added 3 commits November 2, 2026 15:23

TristonianJones previously approved these changes Nov 3, 2026

View reviewed changes

TristonianJones reviewed Nov 3, 2026

View reviewed changes

Comment thread parser/pratt_parser.go Resolved

Use strings.Builder with Grow in extractStructName

87856bd

dmitriplotnikov dismissed TristonianJones’s stale review via 87856bd November 3, 2026 23:15

TristonianJones reviewed Nov 4, 2026

View reviewed changes

Remove isRelative fast-path from parserHelper

b2cf5e0

Remove the isRelative field and ComputeOffset(1, 0) != 0 check from newParserHelper and restore the unconditional OffsetLocation translation in computeOffset.

Navigation Menu

Repository navigation

Replace quadratic string concatenation in qualified struct type names - #1607

Replace quadratic string concatenation in qualified struct type names#1607
dmitriplotnikov wants to merge 5 commits into
cel-expr:masterfrom
dmitriplotnikov:parser-struct-name-builder

dmitriplotnikov commented Nov 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

TristonianJones Nov 4, 2026

Uh oh!

dmitriplotnikov Nov 5, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dmitriplotnikov commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks (go test -bench='^BenchmarkParseLargePathological/.*/deep_struct_name$' -benchmem)

Uh oh!

Uh oh!

TristonianJones Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

dmitriplotnikov Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dmitriplotnikov commented Nov 2, 2026 •

edited

Loading

Benchmarks (`go test -bench='^BenchmarkParseLargePathological/.*/deep_struct_name$' -benchmem`)