eskip: improve lexer performance

[AlexanderYastrebov]

• do not use fmt.Sprintf in ParseFilters and ParsePredicates
• avoid allocations for fixed tokens
• optimize scanWhile loop
• add scanEscaped fast path
• optimize scanRegexp and scanEscaped using strings.Builder
• optimize scanWhitespace
• optimize selectScanner using switch

goos: linux
goarch: amd64
pkg: github.com/zalando/skipper/eskip
cpu: Intel(R) Core(TM) i5-8350U CPU @ 1.70GHz
                  │   HEAD~1    │                HEAD                 │
                  │   sec/op    │   sec/op     vs base                │
ParsePredicates-8   29.88µ ± 3%   11.33µ ± 2%  -62.09% (p=0.000 n=10)

                  │    HEAD~1    │                 HEAD                 │
                  │     B/op     │     B/op      vs base                │
ParsePredicates-8   4.863Ki ± 0%   2.008Ki ± 0%  -58.71% (p=0.000 n=10)

                  │   HEAD~1    │                HEAD                │
                  │  allocs/op  │ allocs/op   vs base                │
ParsePredicates-8   198.00 ± 0%   33.00 ± 0%  -83.33% (p=0.000 n=10)

[szuecs]

return scalars instead of token to reduce allocations

how much does this speedup?
I think the code is already ugly if you have more than 2 return values and now we have increased from 3 to 4.

[szuecs]

👍

[MustafaSaber]

👍

[AlexanderYastrebov]

return scalars instead of token to reduce allocations

how much does this speedup?
I think the code is already ugly if you have more than 2 return values and now we have increased from 3 to 4.

Reverted to return token (benchmark results did not change). Looks like compiler can figure out that it does not need to allocate the token if its not stored anywhere, previously there was:

skipper/eskip/lexer.go

Line 360 in 0cdb93a

l.lastToken = &t

)

[szuecs]

👍

[szuecs]

Thanks for checking this @AlexanderYastrebov !

[sepehrdaddev]

@AlexanderYastrebov seems like the fuzzer found an index out of bound in the code, pls check the workflow.
specifically the in the ParsePredicates() function.

[sepehrdaddev]

ParsePredicates() panics when given ///

here is how reproduce it:

diff --git a/eskip/eskip_test.go b/eskip/eskip_test.go
index 249dced0..f9a75270 100644
--- a/eskip/eskip_test.go
+++ b/eskip/eskip_test.go
@@ -388,6 +388,9 @@ func TestPredicateParsing(t *testing.T) {
        }, {
                title: "star notation",
                input: `*`,
+       }, {
+               title: "panics",
+               input: `///`,
        }} {
                t.Run(test.title, func(t *testing.T) {
                        p, err := ParsePredicates(test.input)
   

this line is causing the panic: https://github.com/zalando/skipper/pull/2755/files#diff-c7c35e59a50fd3d4f06f22410b38b31f543ff4a988b500468920cf3e07a943f4R698

[AlexanderYastrebov]

@sepehrdaddev Nice one! Did you get the /// payload from the logs

==72==ERROR: AddressSanitizer: ABRT on unknown address 0x000000000048 (pc 0x0000005e4481 bp 0x10c000633bd0 sp 0x10c000633bb8 T0)
SCARINESS: 10 (signal)
    #0 0x5e4481 in runtime.raise.abi0 runtime/sys_linux_amd64.s:154

DEDUP_TOKEN: runtime.raise.abi0
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT runtime/sys_linux_amd64.s:154 in runtime.raise.abi0
==72==ABORTING
MS: 1 CopyPart-; base unit: 994af86bef6520042698904387522e903ba5149f
0x2f,0x2f,0x2f,
///

?

[sepehrdaddev]

yes, I got it from the logs (the same log you posted).

[AlexanderYastrebov]

master does not fail properly on this payload, I've created #2849

[sepehrdaddev]

for reference, the fuzzer found multiple payloads that crash (multiple iterations of /), here is another payload that causes it to crash that I found a bit interesting since it was different than others:
"\x2f\x2f\x00\x00\x00\xe6\xfe\x00\x00\x2f\x00\x00\x00\x00\x00\x00\x00\xe6\xfe\x00\x00\x2f\x00\x00\x00\x00"

[AlexanderYastrebov]

I've fixed the code and will followup in #2866

[sepehrdaddev]

I ran the fuzzer for 30 mins and it didn't find anything so I think its safe to say that its fixed.

[sepehrdaddev]

👍

[szuecs]

👍