eskip: use Predicate instead of matcher (#2871)

They are equivalent and using predicate reduces unnecessary copying. This is also consistent with parsing filters straight into `[]*Filter`. ``` goos: linux goarch: amd64 pkg: github.com/zalando/skipper/eskip │ HEAD~1 │ HEAD │ │ sec/op │ sec/op vs base │ ParsePredicates-8 9.192µ ± 2% 8.788µ ± 3% -4.40% (p=0.007 n=10) Parse-8 273.1m ± 2% 260.6m ± 2% -4.58% (p=0.000 n=10) geomean 1.585m 1.513m -4.49% │ HEAD~1 │ HEAD │ │ B/op │ B/op vs base │ ParsePredicates-8 2.008Ki ± 0% 1.961Ki ± 0% -2.33% (p=0.000 n=10) Parse-8 49.94Mi ± 0% 49.02Mi ± 0% -1.83% (p=0.000 n=10) geomean 320.4Ki 313.7Ki -2.08% │ HEAD~1 │ HEAD │ │ allocs/op │ allocs/op vs base │ ParsePredicates-8 33.00 ± 0% 32.00 ± 0% -3.03% (p=0.000 n=10) Parse-8 1.100M ± 0% 1.080M ± 0% -1.82% (p=0.000 n=10) geomean 6.025k 5.879k -2.43% ``` Signed-off-by: Alexander Yastrebov <[email protected]>
zalando · Jan 17, 2024 · ce65352 · ce65352
1 parent 3bb19db
commit ce65352
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 65 deletions.
diff --git a/eskip/eskip.go b/eskip/eskip.go
@@ -202,15 +202,6 @@ func (df *DefaultFilters) Do(routes []*Route) []*Route {
 	return nextRoutes
 }
 
-// Represents a matcher condition for incoming requests.
-type matcher struct {
-	// The name of the matcher, e.g. Path or Header
-	name string
-
-	// The args of the matcher, e.g. the path to be matched.
-	args []interface{}
-}
-
 // BackendType indicates whether a route is a network backend, a shunt or a loopback.
 type BackendType int
 
@@ -228,7 +219,7 @@ var errMixedProtocols = errors.New("loadbalancer endpoints cannot have mixed pro
 // document.
 type parsedRoute struct {
 	id          string
-	matchers    []*matcher
+	predicates  []*Predicate
 	filters     []*Filter
 	shunt       bool
 	loopback    bool
@@ -483,48 +474,48 @@ func applyPredicates(route *Route, proute *parsedRoute) error {
 		methodSet bool
 	)
 
-	for _, m := range proute.matchers {
+	for _, p := range proute.predicates {
 		if err != nil {
 			return err
 		}
 
-		switch m.name {
+		switch p.Name {
 		case "Path":
 			if pathSet {
 				return duplicatePathTreePredicateError
 			}
 
-			if args, err = getStringArgs(1, m.args); err == nil {
+			if args, err = getStringArgs(1, p.Args); err == nil {
 				route.Path = args[0]
 				pathSet = true
 			}
 		case "Host":
-			if args, err = getStringArgs(1, m.args); err == nil {
+			if args, err = getStringArgs(1, p.Args); err == nil {
 				route.HostRegexps = append(route.HostRegexps, args[0])
 			}
 		case "PathRegexp":
-			if args, err = getStringArgs(1, m.args); err == nil {
+			if args, err = getStringArgs(1, p.Args); err == nil {
 				route.PathRegexps = append(route.PathRegexps, args[0])
 			}
 		case "Method":
 			if methodSet {
 				return duplicateMethodPredicateError
 			}
 
-			if args, err = getStringArgs(1, m.args); err == nil {
+			if args, err = getStringArgs(1, p.Args); err == nil {
 				route.Method = args[0]
 				methodSet = true
 			}
 		case "HeaderRegexp":
-			if args, err = getStringArgs(2, m.args); err == nil {
+			if args, err = getStringArgs(2, p.Args); err == nil {
 				if route.HeaderRegexps == nil {
 					route.HeaderRegexps = make(map[string][]string)
 				}
 
 				route.HeaderRegexps[args[0]] = append(route.HeaderRegexps[args[0]], args[1])
 			}
 		case "Header":
-			if args, err = getStringArgs(2, m.args); err == nil {
+			if args, err = getStringArgs(2, p.Args); err == nil {
 				if route.Headers == nil {
 					route.Headers = make(map[string]string)
 				}
@@ -538,9 +529,7 @@ func applyPredicates(route *Route, proute *parsedRoute) error {
 		case "*", "Any":
 			// void
 		default:
-			route.Predicates = append(
-				route.Predicates,
-				&Predicate{m.name, m.args})
+			route.Predicates = append(route.Predicates, p)
 		}
 	}
 
@@ -698,15 +687,16 @@ func ParsePredicates(p string) ([]*Predicate, error) {
 		return nil, nil
 	}
 
-	var ps []*Predicate
-	for _, matcher := range rs[0].matchers {
-		if matcher.name != "*" {
-			ps = append(ps, &Predicate{
-				Name: matcher.name,
-				Args: matcher.args,
-			})
+	r := rs[0]
+	ps := make([]*Predicate, 0, len(r.predicates))
+	for _, p := range r.predicates {
+		if p.Name != "*" {
+			ps = append(ps, p)
 		}
 	}
+	if len(ps) == 0 {
+		ps = nil
+	}
 
 	return ps, nil
 }

diff --git a/eskip/parser.go b/eskip/parser.go
diff --git a/eskip/parser.y b/eskip/parser.y
@@ -1,11 +1,11 @@
 // Copyright 2015 Zalando SE
-// 
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 // http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -32,8 +32,8 @@ func convertNumber(s string) float64 {
 	token string
 	route *parsedRoute
 	routes []*parsedRoute
-	matchers []*matcher
-	matcher *matcher
+	predicates []*Predicate
+	predicate *Predicate
 	filter *Filter
 	filters []*Filter
 	args []interface{}
@@ -110,9 +110,9 @@ routeid:
 	}
 
 route:
-	frontend arrow backend {
+	predicates arrow backend {
 		$$.route = &parsedRoute{
-			matchers: $1.matchers,
+			predicates: $1.predicates,
 			backend: $3.backend,
 			shunt: $3.shunt,
 			loopback: $3.loopback,
@@ -121,13 +121,13 @@ route:
 			lbAlgorithm: $3.lbAlgorithm,
 			lbEndpoints: $3.lbEndpoints,
 		}
-		$1.matchers = nil
+		$1.predicates = nil
 		$3.lbEndpoints = nil
 	}
 	|
-	frontend arrow filters arrow backend {
+	predicates arrow filters arrow backend {
 		$$.route = &parsedRoute{
-			matchers: $1.matchers,
+			predicates: $1.predicates,
 			filters: $3.filters,
 			backend: $5.backend,
 			shunt: $5.shunt,
@@ -137,28 +137,28 @@ route:
 			lbAlgorithm: $5.lbAlgorithm,
 			lbEndpoints: $5.lbEndpoints,
 		}
-		$1.matchers = nil
+		$1.predicates = nil
 		$3.filters = nil
 		$5.lbEndpoints = nil
 	}
 
-frontend:
-	matcher {
-		$$.matchers = []*matcher{$1.matcher}
+predicates:
+	predicate {
+		$$.predicates = []*Predicate{$1.predicate}
 	}
 	|
-	frontend and matcher {
-		$$.matchers = $1.matchers
-		$$.matchers = append($$.matchers, $3.matcher)
+	predicates and predicate {
+		$$.predicates = $1.predicates
+		$$.predicates = append($$.predicates, $3.predicate)
 	}
 
-matcher:
+predicate:
 	any {
-		$$.matcher = &matcher{"*", nil}
+		$$.predicate = &Predicate{"*", nil}
 	}
 	|
 	symbol openparen args closeparen {
-		$$.matcher = &matcher{$1.token, $3.args}
+		$$.predicate = &Predicate{$1.token, $3.args}
 		$3.args = nil
 	}
 

diff --git a/eskip/parser_test.go b/eskip/parser_test.go
@@ -45,11 +45,11 @@ const (
 )
 
 func checkSingleRouteExample(r *parsedRoute, t *testing.T) {
-	if len(r.matchers) != 2 ||
-		r.matchers[0].name != "PathRegexp" || len(r.matchers[0].args) != 1 ||
-		r.matchers[0].args[0] != "\\.html$" ||
-		r.matchers[1].name != "Header" || len(r.matchers[1].args) != 2 ||
-		r.matchers[1].args[0] != "Accept" || r.matchers[1].args[1] != "text/html" {
+	if len(r.predicates) != 2 ||
+		r.predicates[0].Name != "PathRegexp" || len(r.predicates[0].Args) != 1 ||
+		r.predicates[0].Args[0] != "\\.html$" ||
+		r.predicates[1].Name != "Header" || len(r.predicates[1].Args) != 2 ||
+		r.predicates[1].Args[0] != "Accept" || r.predicates[1].Args[1] != "text/html" {
 		t.Error("failed to parse match expression")
 	}
 
@@ -227,7 +227,7 @@ func testRegExpOnce(t *testing.T, regexpStr string, expectedRegExp string) {
 		return
 	}
 
-	if expectedRegExp != routes[0].matchers[0].args[0] {
+	if expectedRegExp != routes[0].predicates[0].Args[0] {
 		t.Error("failed to parse PathRegexp:"+regexpStr+", expected regexp to be "+expectedRegExp, err)
 	}
 }