Skip to content

Commit de7178d

Browse files
titpetricTit Petric
and
Tit Petric
authored
Optimize/new route regexp allocations (#732)
## What type of PR is this? (check all applicable) - [x] Refactor - [ ] Feature - [ ] Bug Fix - [x] Optimization - [ ] Documentation Update ## Description This optimizes NewRouter creation. The change introduces strings.Builder for more efficient strings concatenation, and replaces SplitN with Index, avoiding the allocations for the slice. ## Related Tickets & Documents | Test Group | Condition | Average Time (ns/op) | Average Memory (B/op) | Average Allocations | |-----------------------------|-----------|----------------------|-----------------------|---------------------| | **BenchmarkNewRouter** | Before | 37041 | 26553 | 376 | | | After | 32046 | 25800 | 347 | | **BenchmarkNewRouterRegexpFunc** | Before | 4713 | 2385 | 75 | | | After | 2792 | 1640 | 46 | BenchmarkNewRouterRegexpFunc: Time Improvement: Significant improvement in execution time from 4,713 ns/op to 2,792 ns/op. Memory Usage: Memory usage per operation dropped from 2,385 bytes to 1,640 bytes. Allocations: Allocations per operation decreased substantially from 75 to 46. ## Added/updated tests? - [x] Yes ## Run verifications and test - [ ] `make verify` is passing - [x] `make test` is passing --------- Co-authored-by: Tit Petric <[email protected]>
1 parent cfec64d commit de7178d

File tree

2 files changed

+67
-26
lines changed

2 files changed

+67
-26
lines changed

regexp.go

+41-26
Original file line numberDiff line numberDiff line change
@@ -65,37 +65,50 @@ func newRouteRegexp(tpl string, typ regexpType, options routeRegexpOptions) (*ro
6565
}
6666
varsN := make([]string, len(idxs)/2)
6767
varsR := make([]*regexp.Regexp, len(idxs)/2)
68-
pattern := bytes.NewBufferString("")
68+
69+
var pattern, reverse strings.Builder
6970
pattern.WriteByte('^')
70-
reverse := bytes.NewBufferString("")
71-
var end int
71+
72+
var end, colonIdx, groupIdx int
7273
var err error
74+
var patt, param, name string
7375
for i := 0; i < len(idxs); i += 2 {
7476
// Set all values we are interested in.
77+
groupIdx = i / 2
78+
7579
raw := tpl[end:idxs[i]]
7680
end = idxs[i+1]
77-
parts := strings.SplitN(tpl[idxs[i]+1:end-1], ":", 2)
78-
name := parts[0]
79-
patt := defaultPattern
80-
if len(parts) == 2 {
81-
patt = parts[1]
81+
tag := tpl[idxs[i]:end]
82+
83+
// trim braces from tag
84+
param = tag[1 : len(tag)-1]
85+
86+
colonIdx = strings.Index(param, ":")
87+
if colonIdx == -1 {
88+
name = param
89+
patt = defaultPattern
90+
} else {
91+
name = param[0:colonIdx]
92+
patt = param[colonIdx+1:]
8293
}
94+
8395
// Name or pattern can't be empty.
8496
if name == "" || patt == "" {
85-
return nil, fmt.Errorf("mux: missing name or pattern in %q",
86-
tpl[idxs[i]:end])
97+
return nil, fmt.Errorf("mux: missing name or pattern in %q", tag)
8798
}
8899
// Build the regexp pattern.
89-
fmt.Fprintf(pattern, "%s(?P<%s>%s)", regexp.QuoteMeta(raw), varGroupName(i/2), patt)
100+
groupName := varGroupName(groupIdx)
101+
102+
pattern.WriteString(regexp.QuoteMeta(raw) + "(?P<" + groupName + ">" + patt + ")")
90103

91104
// Build the reverse template.
92-
fmt.Fprintf(reverse, "%s%%s", raw)
105+
reverse.WriteString(raw + "%s")
93106

94107
// Append variable name and compiled pattern.
95-
varsN[i/2] = name
96-
varsR[i/2], err = RegexpCompileFunc(fmt.Sprintf("^%s$", patt))
108+
varsN[groupIdx] = name
109+
varsR[groupIdx], err = RegexpCompileFunc("^" + patt + "$")
97110
if err != nil {
98-
return nil, err
111+
return nil, fmt.Errorf("mux: error compiling regex for %q: %w", tag, err)
99112
}
100113
}
101114
// Add the remaining.
@@ -114,18 +127,9 @@ func newRouteRegexp(tpl string, typ regexpType, options routeRegexpOptions) (*ro
114127
pattern.WriteByte('$')
115128
}
116129

117-
var wildcardHostPort bool
118-
if typ == regexpTypeHost {
119-
if !strings.Contains(pattern.String(), ":") {
120-
wildcardHostPort = true
121-
}
122-
}
123-
reverse.WriteString(raw)
124-
if endSlash {
125-
reverse.WriteByte('/')
126-
}
127130
// Compile full regexp.
128-
reg, errCompile := RegexpCompileFunc(pattern.String())
131+
patternStr := pattern.String()
132+
reg, errCompile := RegexpCompileFunc(patternStr)
129133
if errCompile != nil {
130134
return nil, errCompile
131135
}
@@ -136,6 +140,17 @@ func newRouteRegexp(tpl string, typ regexpType, options routeRegexpOptions) (*ro
136140
"Only non-capturing groups are accepted: e.g. (?:pattern) instead of (pattern)")
137141
}
138142

143+
var wildcardHostPort bool
144+
if typ == regexpTypeHost {
145+
if !strings.Contains(patternStr, ":") {
146+
wildcardHostPort = true
147+
}
148+
}
149+
reverse.WriteString(raw)
150+
if endSlash {
151+
reverse.WriteByte('/')
152+
}
153+
139154
// Done!
140155
return &routeRegexp{
141156
template: template,

regexp_test.go

+26
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,35 @@ import (
44
"net/url"
55
"reflect"
66
"strconv"
7+
"strings"
78
"testing"
89
)
910

11+
func Test_newRouteRegexp_Errors(t *testing.T) {
12+
tests := []struct {
13+
in, out string
14+
}{
15+
{"/{}", `mux: missing name or pattern in "{}"`},
16+
{"/{:.}", `mux: missing name or pattern in "{:.}"`},
17+
{"/{a:}", `mux: missing name or pattern in "{a:}"`},
18+
{"/{id:abc(}", `mux: error compiling regex for "{id:abc(}":`},
19+
}
20+
21+
for _, tc := range tests {
22+
t.Run("Test case for "+tc.in, func(t *testing.T) {
23+
_, err := newRouteRegexp(tc.in, 0, routeRegexpOptions{})
24+
if err != nil {
25+
if strings.HasPrefix(err.Error(), tc.out) {
26+
return
27+
}
28+
t.Errorf("Resulting error does not contain %q as expected, error: %s", tc.out, err)
29+
} else {
30+
t.Error("Expected error, got nil")
31+
}
32+
})
33+
}
34+
}
35+
1036
func Test_findFirstQueryKey(t *testing.T) {
1137
tests := []string{
1238
"a=1&b=2",

0 commit comments

Comments
 (0)