Skip to content

Reduce lexer memory allocs #811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

diegommm
Copy link

Fixes #810

Benchmark results (20% faster, 40% less memory copying, 75% less allocations):

goos: darwin
goarch: arm64
pkg: github.com/expr-lang/expr/parser/lexer
cpu: Apple M4 Pro
          │   old.txt   │               new.txt               │
          │   sec/op    │   sec/op     vs base                │
Parser-12   1.549µ ± 1%   1.254µ ± 1%  -19.05% (p=0.000 n=20)

          │   old.txt    │               new.txt                │
          │     B/op     │     B/op      vs base                │
Parser-12   2.961Ki ± 0%   1.789Ki ± 0%  -39.58% (p=0.000 n=20)

          │   old.txt   │              new.txt               │
          │  allocs/op  │ allocs/op   vs base                │
Parser-12   27.000 ± 0%   7.000 ± 0%  -74.07% (p=0.000 n=20)

Benchmarks were performed in the parser/lexer directory executing the command:

go test -run=zzz-no-tests -bench=. -count=20 > file

The file was renamed to either old.txt or new.txt, and the comparison was made with:

benchstat old.txt new.txt
Raw results from old.txt

goos: darwin
goarch: arm64
pkg: github.com/expr-lang/expr/parser/lexer
cpu: Apple M4 Pro
BenchmarkParser-12        790826              1559 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        727635              1547 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        657169              1522 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        670780              1572 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        739662              1558 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        763692              1560 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        749037              1550 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        761952              1526 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        791458              1524 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        757533              1533 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        762931              1606 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        752461              1593 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        727683              1545 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        775984              1534 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        771271              1534 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        783654              1582 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        774580              1547 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        754653              1539 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        758157              1568 ns/op            3032 B/op         27 allocs/op
BenchmarkParser-12        754888              1551 ns/op            3032 B/op         27 allocs/op
PASS
ok      github.com/expr-lang/expr/parser/lexer  24.998s

Raw results from new.txt

goos: darwin
goarch: arm64
pkg: github.com/expr-lang/expr/parser/lexer
cpu: Apple M4 Pro
BenchmarkParser-12        883687              1250 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        926926              1229 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        965379              1227 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        961345              1251 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        902880              1332 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        880496              1368 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        958262              1236 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        933210              1241 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        941917              1271 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        886873              1263 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        927643              1251 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        960315              1257 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        944788              1263 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        904806              1267 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        938611              1243 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        947118              1256 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        946309              1251 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        948300              1293 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        909182              1260 ns/op            1832 B/op          7 allocs/op
BenchmarkParser-12        933751              1251 ns/op            1832 B/op          7 allocs/op
PASS
ok      github.com/expr-lang/expr/parser/lexer  25.899s

}

func (s Source) String() string {
return string(s)
return s.raw
}

func (s Source) Snippet(line int) (string, bool) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is no longer used but I'm keeping it in case someone was using it.


"github.com/expr-lang/expr/file"
)

const minTokens = 10
Copy link
Author

@diegommm diegommm Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows for optimistically allocating tokens. Discussing the "right" value for this parameter can be a loss of time (everyone has an opinion on what should be the "right" value).
The only thing I will hold is that using zero (the default) is the worst of all values because you will likely always need some nodes. It would be very rare that you don't. And the first few times your add a new node with append then the runtime will just have to copy over and over the same underlying array while it grows the slice.

@@ -335,6 +335,7 @@ literal not terminated (1:10)
früh ♥︎
unrecognized character: U+2665 '♥' (1:6)
| früh ♥︎
| .....^
Copy link
Author

@diegommm diegommm Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appeared to be a small bug. In the original code, if it found a rune longer than 1 byte it would skip the indicator line entirely. I imagine that wouldn't be good if we have something like:

let myStr = "Hello, 世界"; someError

In that case it wouldn't put the indicator line because it will find the '世' rune.

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.

Reduce allocations during parsing
1 participant