From bf2f0737850f48980428c52ea4ceeb221d656b0f Mon Sep 17 00:00:00 2001 From: Carl Jackson Date: Sun, 4 May 2014 10:40:22 -0700 Subject: [PATCH] Pass a *C between middleware and router Previously, the middleware stack passed the router a C, but this was both odd semantically (a pattern which mutated the environment might see a *different* environment) and bad for perf: it cost us an allocation. Now we only pass around *C's internally. Importantly ("importantly"), this gets us down to 0 allocations for the static routing case, and one allocation (the URLParams map) for the normal routing case. --- web/middleware.go | 8 ++++++-- web/middleware_test.go | 14 ++++++++++---- web/mux.go | 2 +- web/router.go | 8 ++++---- web/router_test.go | 14 +++++++------- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/web/middleware.go b/web/middleware.go index d39a428..a8477c5 100644 --- a/web/middleware.go +++ b/web/middleware.go @@ -19,7 +19,11 @@ type mStack struct { lock sync.Mutex stack []mLayer pool chan *cStack - router Handler + router internalRouter +} + +type internalRouter interface { + route(*C, http.ResponseWriter, *http.Request) } /* @@ -93,7 +97,7 @@ func (m *mStack) newStack() *cStack { router := m.router cs.m = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - router.ServeHTTPC(cs.C, w, r) + router.route(&cs.C, w, r) }) for i := len(m.stack) - 1; i >= 0; i-- { cs.m = m.stack[i].fn(&cs.C, cs.m) diff --git a/web/middleware_test.go b/web/middleware_test.go index e783427..e52027d 100644 --- a/web/middleware_test.go +++ b/web/middleware_test.go @@ -7,14 +7,20 @@ import ( "time" ) +type iRouter func(*C, http.ResponseWriter, *http.Request) + +func (i iRouter) route(c *C, w http.ResponseWriter, r *http.Request) { + i(c, w, r) +} + func makeStack(ch chan string) *mStack { - router := func(c C, w http.ResponseWriter, r *http.Request) { + router := func(c *C, w http.ResponseWriter, r *http.Request) { ch <- "router" } return &mStack{ stack: make([]mLayer, 0), pool: make(chan *cStack, mPoolSize), - router: HandlerFunc(router), + router: iRouter(router), } } @@ -202,7 +208,7 @@ func TestInvalidation(t *testing.T) { } func TestContext(t *testing.T) { - router := func(c C, w http.ResponseWriter, r *http.Request) { + router := func(c *C, w http.ResponseWriter, r *http.Request) { if c.Env["reqID"].(int) != 2 { t.Error("Request id was not 2 :(") } @@ -210,7 +216,7 @@ func TestContext(t *testing.T) { st := mStack{ stack: make([]mLayer, 0), pool: make(chan *cStack, mPoolSize), - router: HandlerFunc(router), + router: iRouter(router), } st.Use(func(c *C, h http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { diff --git a/web/mux.go b/web/mux.go index e40eaf1..a8e9bc6 100644 --- a/web/mux.go +++ b/web/mux.go @@ -68,7 +68,7 @@ func New() *Mux { notFound: parseHandler(http.NotFound), }, } - mux.mStack.router = HandlerFunc(mux.router.route) + mux.mStack.router = &mux.router return &mux } diff --git a/web/router.go b/web/router.go index 2d94d8f..e5ed9dd 100644 --- a/web/router.go +++ b/web/router.go @@ -219,18 +219,18 @@ func (rt *router) Compile() { rt.setMachine(&sm) } -func (rt *router) route(c C, w http.ResponseWriter, r *http.Request) { +func (rt *router) route(c *C, w http.ResponseWriter, r *http.Request) { if rt.machine == nil { rt.Compile() } - methods, ok := rt.getMachine().route(&c, w, r) + methods, ok := rt.getMachine().route(c, w, r) if ok { return } if methods == 0 { - rt.notFound.ServeHTTPC(c, w, r) + rt.notFound.ServeHTTPC(*c, w, r) return } @@ -249,7 +249,7 @@ func (rt *router) route(c C, w http.ResponseWriter, r *http.Request) { } else { c.Env[ValidMethodsKey] = methodsList } - rt.notFound.ServeHTTPC(c, w, r) + rt.notFound.ServeHTTPC(*c, w, r) } func (rt *router) handleUntyped(p interface{}, m method, h interface{}) { diff --git a/web/router_test.go b/web/router_test.go index 163ea09..442903e 100644 --- a/web/router_test.go +++ b/web/router_test.go @@ -46,7 +46,7 @@ func TestMethods(t *testing.T) { for _, method := range methods { r, _ := http.NewRequest(method, "/", nil) w := httptest.NewRecorder() - rt.route(C{}, w, r) + rt.route(&C{}, w, r) select { case val := <-ch: if val != method { @@ -119,7 +119,7 @@ func TestHandlerTypes(t *testing.T) { for route, response := range testHandlerTable { r, _ := http.NewRequest("GET", route, nil) w := httptest.NewRecorder() - rt.route(C{}, w, r) + rt.route(&C{}, w, r) select { case resp := <-ch: if resp != response { @@ -209,7 +209,7 @@ func TestRouteSelection(t *testing.T) { for counter, n = range test.results { r, _ := http.NewRequest("GET", test.key, nil) w := httptest.NewRecorder() - rt.route(C{}, w, r) + rt.route(&C{}, w, r) actual := <-ichan if n != actual { t.Errorf("Expected %q @ %d to be %d, got %d", @@ -225,7 +225,7 @@ func TestNotFound(t *testing.T) { r, _ := http.NewRequest("post", "/", nil) w := httptest.NewRecorder() - rt.route(C{}, w, r) + rt.route(&C{}, w, r) if w.Code != 404 { t.Errorf("Expected 404, got %d", w.Code) } @@ -236,7 +236,7 @@ func TestNotFound(t *testing.T) { r, _ = http.NewRequest("POST", "/", nil) w = httptest.NewRecorder() - rt.route(C{}, w, r) + rt.route(&C{}, w, r) if w.Code != http.StatusTeapot { t.Errorf("Expected a teapot, got %d", w.Code) } @@ -253,7 +253,7 @@ func TestPrefix(t *testing.T) { r, _ := http.NewRequest("GET", "/hello/world", nil) w := httptest.NewRecorder() - rt.route(C{}, w, r) + rt.route(&C{}, w, r) select { case val := <-ch: if val != "/hello/world" { @@ -302,7 +302,7 @@ func TestValidMethods(t *testing.T) { for path, eMethods := range validMethodsTable { r, _ := http.NewRequest("BOGUS", path, nil) - rt.route(C{}, httptest.NewRecorder(), r) + rt.route(&C{}, httptest.NewRecorder(), r) aMethods := <-ch if !reflect.DeepEqual(eMethods, aMethods) { t.Errorf("For %q, expected %v, got %v", path, eMethods,