From 25044d60c259e7590ad677dec268f4b49bcedc67 Mon Sep 17 00:00:00 2001 From: Carl Jackson Date: Sat, 23 Aug 2014 14:41:10 -0700 Subject: [PATCH] Eliminate a lock in the request hot path If you're manipulating your middleware stack concurrently with active requests you're probably doing something wrong, and it's not worth either the complexity or runtime cost to support you hitting yourself. We can probably take this principle a bit further and disallow mutating the middleware stack after any requests have been made (which will eliminate even more complexity) but that can be a project for another day. --- web/middleware.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/web/middleware.go b/web/middleware.go index 882cdd4..6ebf379 100644 --- a/web/middleware.go +++ b/web/middleware.go @@ -87,9 +87,6 @@ func (m *mStack) invalidate() { } func (m *mStack) newStack() *cStack { - m.lock.Lock() - defer m.lock.Unlock() - cs := cStack{} router := m.router @@ -104,10 +101,6 @@ func (m *mStack) newStack() *cStack { } func (m *mStack) alloc() *cStack { - // This is a little sloppy: this is only safe if this pointer - // dereference is atomic. Maybe someday I'll replace it with - // sync/atomic, but for now I happen to know that on all the - // architectures I care about it happens to be atomic. p := m.pool cs := p.alloc() if cs == nil { @@ -130,7 +123,8 @@ func (m *mStack) release(cs *cStack) { // Append the given middleware to the middleware stack. See the documentation // for type Mux for a list of valid middleware types. // -// No attempt is made to enforce the uniqueness of middlewares. +// No attempt is made to enforce the uniqueness of middlewares. It is illegal to +// call this function concurrently with active requests. func (m *mStack) Use(middleware interface{}) { m.lock.Lock() defer m.lock.Unlock() @@ -143,7 +137,8 @@ func (m *mStack) Use(middleware interface{}) { // types. Returns an error if no middleware has the name given by "before." // // No attempt is made to enforce the uniqueness of middlewares. If the insertion -// point is ambiguous, the first (outermost) one is chosen. +// point is ambiguous, the first (outermost) one is chosen. It is illegal to +// call this function concurrently with active requests. func (m *mStack) Insert(middleware, before interface{}) error { m.lock.Lock() defer m.lock.Unlock() @@ -165,7 +160,8 @@ func (m *mStack) Insert(middleware, before interface{}) error { // no such middleware can be found. // // If the name of the middleware to delete is ambiguous, the first (outermost) -// one is chosen. +// one is chosen. It is illegal to call this function concurrently with active +// requests. func (m *mStack) Abandon(middleware interface{}) error { m.lock.Lock() defer m.lock.Unlock()