Skip to content

Avoid quadratic suffix allocations in runes.Buffer construction - #1602

Merged
dmitriplotnikov merged 1 commit into
cel-expr:masterfrom
dmitriplotnikov:runes-buffer-suffix-alloc
Nov 4, 2026
Merged

dmitriplotnikov merged 1 commit into
cel-expr:masterfrom
dmitriplotnikov:runes-buffer-suffix-alloc

Conversation

@dmitriplotnikov

@dmitriplotnikov dmitriplotnikov commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator
  • Replace utf8.DecodeRuneInString(data[idx:]) loops in countRemainingCodePoints and newBufferWithLimit with for range string iteration. Under -gcflags=all=-d=libfuzzer (OSS-Fuzz), the inlined empty-string check in DecodeRuneInString invokes runtime.libfuzzerHookStrCmp(data[idx:], ""), allocating a heap copy of the remaining input suffix on every rune (~5 GB on 100 KB inputs, causing severe GC thrashing and timeouts).
  • Bound preallocated slice capacities in newBufferWithLimit to the validated code-point count when limit >= 0 && len(data) > limit.
  • Add unit tests and BenchmarkNewBuffer.

Benchmarks (go test -bench=BenchmarkNewBuffer -benchmem)

goos: linux · goarch: amd64 · pkg: cel.dev/cel-go/common/runes · cpu: Intel(R) Xeon(R) CPU @ 2.20GHz

Benchmark before.txt (sec/op) after.txt (sec/op) vs base
NewBuffer/ASCII_100KB-8 890.8µ ± 3% 875.2µ ± 8% ~ (p=0.485 n=6)
NewBuffer/BMP_100KRunes-8 1.549m ± 11% 1.247m ± 21% -19.54% (p=0.004 n=6)
NewBuffer/Supplemental_100KRunes-8 2.216m ± 42% 1.639m ± 25% -26.01% (p=0.015 n=6)
NewBuffer/Exceeded_500KB-8 633.8µ ± 15% 450.6µ ± 7% -28.90% (p=0.002 n=6)
geomean 1.180m 947.5µ -19.70%
Benchmark before.txt (B/op) after.txt (B/op) vs base
NewBuffer/ASCII_100KB-8 977.9Ki ± 0% 977.9Ki ± 0% ~ (p=0.665 n=6)
NewBuffer/BMP_100KRunes-8 1.291Mi ± 0% 1.150Mi ± 0% -10.89% (p=0.002 n=6)
NewBuffer/Supplemental_100KRunes-8 2.057Mi ± 0% 1.338Mi ± 0% -34.95% (p=0.002 n=6)
NewBuffer/Exceeded_500KB-8 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=6) 1
geomean 80.76Ki 70.47Ki -12.75%

1 all samples are equal

Benchmark before.txt (allocs/op) after.txt (allocs/op) vs base
NewBuffer/ASCII_100KB-8 25.00 ± 0% 25.00 ± 0% ~ (p=1.000 n=6) 1
NewBuffer/BMP_100KRunes-8 26.00 ± 0% 26.00 ± 0% ~ (p=1.000 n=6) 1
NewBuffer/Supplemental_100KRunes-8 26.00 ± 0% 26.00 ± 0% ~ (p=1.000 n=6) 1
NewBuffer/Exceeded_500KB-8 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=6) 1
geomean 11.40 11.40 +0.00%

1 all samples are equal

Under -d=libfuzzer (asan-fuzzer FuzzParser target):

  • 102,823 B over-limit input: 2,591 ms (1,285 GCs) -> 9 ms (1 GC)
  • 500,000 B over-limit input: >61,000 ms (timeout) -> 42 ms (2 GCs)

- Replace `utf8.DecodeRuneInString(data[idx:])` loops in `countRemainingCodePoints` and `newBufferWithLimit` with `for range` string iteration. Under `-gcflags=all=-d=libfuzzer` (OSS-Fuzz), the inlined empty-string check in `DecodeRuneInString` invokes `runtime.libfuzzerHookStrCmp(data[idx:], "")`, allocating a heap copy of the remaining input suffix on every rune (~5 GB on 100 KB inputs, causing severe GC thrashing and timeouts).
- Bound preallocated slice capacities in `newBufferWithLimit` to the validated code-point count when `limit >= 0 && len(data) > limit`.
- Add unit tests and `BenchmarkNewBuffer`.

Benchmarks (`go test -bench=BenchmarkNewBuffer -benchmem`):

goos: linux
goarch: amd64
pkg: cel.dev/cel-go/common/runes
cpu: Intel(R) Xeon(R) CPU @ 2.20GHz
                                   │  before.txt  │              after.txt              │
                                   │    sec/op    │    sec/op     vs base               │
NewBuffer/ASCII_100KB-8              890.8µ ±  3%   875.2µ ±  8%        ~ (p=0.485 n=6)
NewBuffer/BMP_100KRunes-8            1.549m ± 11%   1.247m ± 21%  -19.54% (p=0.004 n=6)
NewBuffer/Supplemental_100KRunes-8   2.216m ± 42%   1.639m ± 25%  -26.01% (p=0.015 n=6)
NewBuffer/Exceeded_500KB-8           633.8µ ± 15%   450.6µ ±  7%  -28.90% (p=0.002 n=6)
geomean                              1.180m         947.5µ        -19.70%

                                   │  before.txt  │               after.txt               │
                                   │     B/op     │     B/op      vs base                 │
NewBuffer/ASCII_100KB-8              977.9Ki ± 0%   977.9Ki ± 0%        ~ (p=0.665 n=6)
NewBuffer/BMP_100KRunes-8            1.291Mi ± 0%   1.150Mi ± 0%  -10.89% (p=0.002 n=6)
NewBuffer/Supplemental_100KRunes-8   2.057Mi ± 0%   1.338Mi ± 0%  -34.95% (p=0.002 n=6)
NewBuffer/Exceeded_500KB-8             16.00 ± 0%     16.00 ± 0%        ~ (p=1.000 n=6) ¹
geomean                              80.76Ki        70.47Ki       -12.75%
¹ all samples are equal

                                   │ before.txt │             after.txt              │
                                   │ allocs/op  │ allocs/op   vs base                │
NewBuffer/ASCII_100KB-8              25.00 ± 0%   25.00 ± 0%       ~ (p=1.000 n=6) ¹
NewBuffer/BMP_100KRunes-8            26.00 ± 0%   26.00 ± 0%       ~ (p=1.000 n=6) ¹
NewBuffer/Supplemental_100KRunes-8   26.00 ± 0%   26.00 ± 0%       ~ (p=1.000 n=6) ¹
NewBuffer/Exceeded_500KB-8           1.000 ± 0%   1.000 ± 0%       ~ (p=1.000 n=6) ¹
geomean                              11.40        11.40       +0.00%
¹ all samples are equal

Under `-d=libfuzzer` (`asan-fuzzer` `FuzzParser` target):
- 102,823 B over-limit input: 2,591 ms (1,285 GCs) -> 9 ms (1 GC)
- 500,000 B over-limit input: >61,000 ms (timeout) -> 42 ms (2 GCs)
Comment thread common/runes/buffer.go Resolved
@dmitriplotnikov
dmitriplotnikov merged commit 37a02e5 into cel-expr:master Nov 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants