Skip to content
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

eskip: improve lexer performance 2 #2870

Merged
merged 2 commits into from
Jan 17, 2024
Merged

Conversation

AlexanderYastrebov
Copy link
Member

  • use plain ascii instead of unicode package
  • use loop for scanSymbol
  • call scan functions directly instead of selectScanner to aid inlining
goos: linux
goarch: amd64
pkg: github.com/zalando/skipper/eskip
                  │    HEAD~1    │                HEAD                 │
                  │    sec/op    │   sec/op     vs base                │
ParsePredicates-8   9.637µ ± 11%   8.894µ ± 4%   -7.71% (p=0.001 n=10)
Parse-8             329.1m ±  4%   272.7m ± 2%  -17.15% (p=0.000 n=10)
geomean             1.781m         1.557m       -12.56%

                  │    HEAD~1    │                HEAD                 │
                  │     B/op     │     B/op      vs base               │
ParsePredicates-8   2.008Ki ± 0%   2.008Ki ± 0%       ~ (p=1.000 n=10)
Parse-8             49.94Mi ± 0%   49.94Mi ± 0%       ~ (p=0.926 n=10)
geomean             320.4Ki        320.4Ki       -0.00%

                  │   HEAD~1    │                 HEAD                 │
                  │  allocs/op  │  allocs/op   vs base                 │
ParsePredicates-8    33.00 ± 0%    33.00 ± 0%       ~ (p=1.000 n=10) ¹
Parse-8             1.100M ± 0%   1.100M ± 0%       ~ (p=0.367 n=10)
geomean             6.025k        6.025k       +0.00%
¹ all samples are equal

See previous #2755

Add a benchmark for parsing 10000 routes.

Signed-off-by: Alexander Yastrebov <[email protected]>
@AlexanderYastrebov AlexanderYastrebov added the major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs label Jan 16, 2024
Comment on lines 269 to 279
func scanSymbol(code string) (t token, rest string, err error) {
t.id = symbol
t.val, rest = scanWhile(code, isSymbolChar)
for i := 0; i < len(code); i++ {
if !isSymbolChar(code[i]) {
t.val, rest = code[0:i], code[i:]
return
}
}
t.val, rest = code, ""
return
}
Copy link
Member Author

@AlexanderYastrebov AlexanderYastrebov Jan 16, 2024

Choose a reason for hiding this comment

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

Before scanSymbol had awful performance - slower than scanning escaped string:

Screenshot from 2024-01-16 22-37-12

After
Screenshot from 2024-01-16 22-38-19

@AlexanderYastrebov AlexanderYastrebov changed the title eskip: optimize lexer (2) eskip: improve lexer performance 2 Jan 16, 2024
@AlexanderYastrebov AlexanderYastrebov marked this pull request as draft January 16, 2024 21:42
* use plain ascii instead of unicode package
* use loop for scanSymbol
* call scan functions directly instead of selectScanner to aid inlining

```
goos: linux
goarch: amd64
pkg: github.com/zalando/skipper/eskip
                  │    HEAD~1    │                HEAD                 │
                  │    sec/op    │   sec/op     vs base                │
ParsePredicates-8   9.637µ ± 11%   8.894µ ± 4%   -7.71% (p=0.001 n=10)
Parse-8             329.1m ±  4%   272.7m ± 2%  -17.15% (p=0.000 n=10)
geomean             1.781m         1.557m       -12.56%

                  │    HEAD~1    │                HEAD                 │
                  │     B/op     │     B/op      vs base               │
ParsePredicates-8   2.008Ki ± 0%   2.008Ki ± 0%       ~ (p=1.000 n=10)
Parse-8             49.94Mi ± 0%   49.94Mi ± 0%       ~ (p=0.926 n=10)
geomean             320.4Ki        320.4Ki       -0.00%

                  │   HEAD~1    │                 HEAD                 │
                  │  allocs/op  │  allocs/op   vs base                 │
ParsePredicates-8    33.00 ± 0%    33.00 ± 0%       ~ (p=1.000 n=10) ¹
Parse-8             1.100M ± 0%   1.100M ± 0%       ~ (p=0.367 n=10)
geomean             6.025k        6.025k       +0.00%
¹ all samples are equal
```

See previous #2755

Signed-off-by: Alexander Yastrebov <[email protected]>
@AlexanderYastrebov AlexanderYastrebov marked this pull request as ready for review January 16, 2024 22:03
Comment on lines -81 to -82
func isAlpha(c byte) bool { return unicode.IsLetter(rune(c)) }
func isDigit(c byte) bool { return unicode.IsDigit(rune(c)) }
Copy link
Member Author

Choose a reason for hiding this comment

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

This is equivalent but inlines better since c is byte and unicode.* does internal check for ascii

func isSymbolChar(c byte) bool { return isUnderscore(c) || isAlpha(c) || isDigit(c) }
func isAlpha(c byte) bool { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') }
func isDigit(c byte) bool { return c >= '0' && c <= '9' }
func isSymbolChar(c byte) bool { return isAlpha(c) || isDigit(c) || isUnderscore(c) }
Copy link
Member Author

Choose a reason for hiding this comment

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

Check more frequent class first

@szuecs
Copy link
Member

szuecs commented Jan 17, 2024

👍

1 similar comment
@AlexanderYastrebov
Copy link
Member Author

👍

@AlexanderYastrebov AlexanderYastrebov merged commit 200e590 into master Jan 17, 2024
14 checks passed
@AlexanderYastrebov AlexanderYastrebov deleted the eskip/optimize-lexer-5 branch January 17, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants