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>

diff --git a/src/net/http/server.go b/src/net/http/server.go
index e8903c5346005ecaa4c210e4634a84183f4f1082..c244b372fc224e04f3cbe5d87fd5fc869d1a5af7 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -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 {