1
0
mirror of https://github.com/golang/go synced 2024-11-23 15:30:05 -07:00

net/http: avoid deferred unlock in ServeMux.shouldRedirect

CL 96575 introduced concurrency protection for
ServeMux.shouldRedirect with a read lock and deferred unlock.
However, the change produced a noticeable regression.
Instead add the suffix "RLocked" to the function name to
declare that we should hold the read lock as a pre-requisite
before calling it, hence avoiding the defer altogether.

Benchmarks:
name                  old time/op    new time/op    delta
ServeMux-8              63.3µs ± 0%    54.6µs ± 0%  -13.74%  (p=0.000 n=9+9)
ServeMux_SkipServe-8    41.4µs ± 2%    32.7µs ± 1%  -21.05%  (p=0.000 n=10+10)

name                  old alloc/op   new alloc/op   delta
ServeMux-8              17.3kB ± 0%    17.3kB ± 0%     ~     (all equal)
ServeMux_SkipServe-8     0.00B          0.00B          ~     (all equal)

name                  old allocs/op  new allocs/op  delta
ServeMux-8                 360 ± 0%       360 ± 0%     ~     (all equal)
ServeMux_SkipServe-8      0.00           0.00          ~     (all equal)

Updates #25383
Updates #25482

Change-Id: I2ffa4eafe165faa961ce23bd29b5653a89facbc2
Reviewed-on: https://go-review.googlesource.com/113996
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit is contained in:
Emmanuel T Odeke 2018-05-21 12:57:53 -07:00 committed by Brad Fitzpatrick
parent 85e38cccb4
commit 1988b3ed0e

View File

@ -2236,7 +2236,10 @@ func (mux *ServeMux) match(path string) (h Handler, pattern string) {
// not for path itself. If the path needs appending to, it creates a new
// URL, setting the path to u.Path + "/" and returning true to indicate so.
func (mux *ServeMux) redirectToPathSlash(host, path string, u *url.URL) (*url.URL, bool) {
if !mux.shouldRedirect(host, path) {
mux.mu.RLock()
shouldRedirect := mux.shouldRedirectRLocked(host, path)
mux.mu.RUnlock()
if !shouldRedirect {
return u, false
}
path = path + "/"
@ -2244,13 +2247,10 @@ func (mux *ServeMux) redirectToPathSlash(host, path string, u *url.URL) (*url.UR
return u, true
}
// shouldRedirect reports whether the given path and host should be redirected to
// shouldRedirectRLocked reports whether the given path and host should be redirected to
// path+"/". This should happen if a handler is registered for path+"/" but
// not path -- see comments at ServeMux.
func (mux *ServeMux) shouldRedirect(host, path string) bool {
mux.mu.RLock()
defer mux.mu.RUnlock()
func (mux *ServeMux) shouldRedirectRLocked(host, path string) bool {
p := []string{path, host + path}
for _, c := range p {