From 1988b3ed0ed72995f566630558e5bb0531aeac60 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 21 May 2018 12:57:53 -0700 Subject: [PATCH] net/http: avoid deferred unlock in ServeMux.shouldRedirect MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/net/http/server.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/net/http/server.go b/src/net/http/server.go index e8903c5346..c244b372fc 100644 --- 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 { -- 2.50.0