Skip to content

Commit

Permalink
perf: clone request URL struct directly for path manipulation
Browse files Browse the repository at this point in the history
Save 4 allocations worth 200B per request cycle with redirect.
The rewrite operation takes before O(600ns) vs after O(200ns).

```
$ go test -benchmem -benchtime 5000000x -bench BenchmarkStrictSlash
goos: linux
goarch: amd64
pkg: github.com/gorilla/mux
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkStrictSlashClone-8      5000000               183.9 ns/op            64 B/op          4 allocs/op
BenchmarkStrictSlashParse-8      5000000               559.8 ns/op           264 B/op          8 allocs/op
PASS
ok      github.com/gorilla/mux  3.740s

$ go test -benchmem -benchtime 5000000x -bench BenchmarkStrictSlash
goos: linux
goarch: amd64
pkg: github.com/gorilla/mux
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkStrictSlashClone-8      5000000               180.4 ns/op            64 B/op          4 allocs/op
BenchmarkStrictSlashParse-8      5000000               573.5 ns/op           264 B/op          8 allocs/op
PASS
ok      github.com/gorilla/mux  3.788s

$ go test -benchmem -benchtime 5000000x -bench BenchmarkStrictSlash
goos: linux
goarch: amd64
pkg: github.com/gorilla/mux
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkStrictSlashClone-8      5000000               175.8 ns/op            64 B/op          4 allocs/op
BenchmarkStrictSlashParse-8      5000000               569.4 ns/op           264 B/op          8 allocs/op
PASS
ok      github.com/gorilla/mux  3.744s
```

```
func BenchmarkStrictSlashClone(b *testing.B) {
	req := newRequest(http.MethodGet, "http://localhost/x")
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		_ = replaceURLPath(req.URL, req.URL.Path+"/")
	}
}

func BenchmarkStrictSlashParse(b *testing.B) {
	req := newRequest(http.MethodGet, "http://localhost/x")
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		u, _ := url.Parse(req.URL.String())
		u.Path += "/"
		_ = u.String()
	}
}
```

Signed-off-by: Jakob Ackermann <[email protected]>
  • Loading branch information
das7pad committed Sep 3, 2023
1 parent 5616b91 commit f3aa4cb
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
19 changes: 10 additions & 9 deletions mux.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"net/http"
"net/url"
"path"
"regexp"
)
Expand Down Expand Up @@ -180,15 +181,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
}
// Clean path to canonical form and redirect.
if p := cleanPath(path); p != path {

// Added 3 lines (Philip Schlump) - It was dropping the query string and #whatever from query.
// This matches with fix in go 1.2 r.c. 4 for same problem. Go Issue:
// http://code.google.com/p/go/issues/detail?id=5252
url := *req.URL
url.Path = p
p = url.String()

w.Header().Set("Location", p)
w.Header().Set("Location", replaceURLPath(req.URL, p))
w.WriteHeader(http.StatusMovedPermanently)
return
}
Expand Down Expand Up @@ -488,6 +481,14 @@ func cleanPath(p string) string {
return np
}

// replaceURLPath prints an url.URL with a different path.
func replaceURLPath(u *url.URL, p string) string {
// Operate on a copy of the request url.
u2 := *u
u2.Path = p
return u2.String()
}

// uniqueVars returns an error if two slices contain duplicated strings.
func uniqueVars(s1, s2 []string) error {
for _, v1 := range s1 {
Expand Down
9 changes: 5 additions & 4 deletions regexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,14 @@ func (v routeRegexpGroup) setMatch(req *http.Request, m *RouteMatch, r *Route) {
p1 := strings.HasSuffix(path, "/")
p2 := strings.HasSuffix(v.path.template, "/")
if p1 != p2 {
u, _ := url.Parse(req.URL.String())
p := req.URL.Path
if p1 {
u.Path = u.Path[:len(u.Path)-1]
p = p[:len(p)-1]
} else {
u.Path += "/"
p += "/"
}
m.Handler = http.RedirectHandler(u.String(), http.StatusMovedPermanently)
u := replaceURLPath(req.URL, p)
m.Handler = http.RedirectHandler(u, http.StatusMovedPermanently)
}
}
}
Expand Down

0 comments on commit f3aa4cb

Please sign in to comment.