Skip to content
This repository has been archived by the owner on Dec 23, 2023. It is now read-only.

Commit

Permalink
Remove Router.paramsPool
Browse files Browse the repository at this point in the history
  • Loading branch information
razonyang committed May 5, 2020
1 parent 7fa7866 commit c2ecabe
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 84 deletions.
13 changes: 8 additions & 5 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,15 @@ var contextPool = sync.Pool{
},
}

func getContext() *Context {
func getContext(router *Router, w http.ResponseWriter, r *http.Request) *Context {
ctx := contextPool.Get().(*Context)
ctx.reset()
ctx.router = router
ctx.Response = w
ctx.Request = r
if cap(ctx.Params) < int(router.maxParams) {
ctx.Params = make(Params, 0, router.maxParams)
}
return ctx
}

Expand All @@ -76,11 +82,8 @@ func newContext(w http.ResponseWriter, r *http.Request) *Context {
}

func (ctx *Context) reset() {
ctx.router = nil
ctx.Params = nil
ctx.Params = ctx.Params[0:0]
ctx.Route = nil
ctx.Response = nil
ctx.Request = nil
ctx.query = nil
}

Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ module github.com/clevergo/clevergo

go 1.7

require github.com/stretchr/testify v1.5.1
require (
github.com/docker/docker v1.13.1
github.com/stretchr/testify v1.5.1
)
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/docker/docker v1.13.1 h1:IkZjBSIc8hBjLpqeAbeE5mca5mNgeatLHBy3GO78BWo=
github.com/docker/docker v1.13.1/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
Expand Down
41 changes: 7 additions & 34 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"net/http"
"net/url"
"strings"
"sync"
)

// errors
Expand Down Expand Up @@ -71,8 +70,7 @@ type Router struct {
// Named routes.
routes map[string]*Route

paramsPool sync.Pool
maxParams uint16
maxParams uint16

// Enables automatic redirection if the current route can't be matched but a
// handler for the path with (without) the trailing slash exists.
Expand Down Expand Up @@ -152,16 +150,6 @@ func NewRouter() *Router {
}
}

func (r *Router) getParams() *Params {
ps := r.paramsPool.Get().(*Params)
*ps = (*ps)[0:0] // reset slice
return ps
}

func (r *Router) putParams(ps *Params) {
r.paramsPool.Put(ps)
}

// URL creates an url with the given route name and arguments.
func (r *Router) URL(name string, args ...string) (*url.URL, error) {
if route, ok := r.routes[name]; ok {
Expand Down Expand Up @@ -271,14 +259,6 @@ func (r *Router) Handle(method, path string, handle Handle, opts ...RouteOption)
if pc := countParams(path); pc > r.maxParams {
r.maxParams = pc
}

// Lazy-init paramsPool alloc func
if r.paramsPool.New == nil && r.maxParams > 0 {
r.paramsPool.New = func() interface{} {
ps := make(Params, 0, r.maxParams)
return &ps
}
}
}

// Handler implements IRouter.Handler.
Expand Down Expand Up @@ -321,15 +301,16 @@ func (r *Router) ServeFiles(path string, root http.FileSystem, opts ...RouteOpti
// values. Otherwise the third return value indicates whether a redirection to
// the same path with an extra / without the trailing slash should be performed.
func (r *Router) Lookup(method, path string) (*Route, Params, bool) {
ps := make(Params, 0, r.maxParams)
if root := r.trees[method]; root != nil {
route, ps, tsr := root.getValue(path, r.getParams, r.UseRawPath)
route, tsr := root.getValue(path, &ps, r.UseRawPath)
if route == nil {
return nil, nil, tsr
}
if ps == nil {
return route, nil, tsr
}
return route, *ps, tsr
return route, ps, tsr
}
return nil, nil, false
}
Expand Down Expand Up @@ -357,7 +338,7 @@ func (r *Router) allowed(path, reqMethod string) (allow string) {
continue
}

handle, _, _ := r.trees[method].getValue(path, nil, r.UseRawPath)
handle, _ := r.trees[method].getValue(path, nil, r.UseRawPath)
if handle != nil {
// Add request method to list of allowed methods
allowed = append(allowed, method)
Expand Down Expand Up @@ -386,10 +367,7 @@ func (r *Router) allowed(path, reqMethod string) (allow string) {

// ServeHTTP makes the router implement the http.Handler interface.
func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
ctx := getContext()
ctx.router = r
ctx.Request = req
ctx.Response = w
ctx := getContext(r, w, req)
defer putContext(ctx)

var err error
Expand All @@ -410,13 +388,8 @@ func (r *Router) handleRequest(ctx *Context) (err error) {
}

if root := r.trees[ctx.Request.Method]; root != nil {
if route, ps, tsr := root.getValue(path, r.getParams, r.UseRawPath); route != nil {
if route, tsr := root.getValue(path, &ctx.Params, r.UseRawPath); route != nil {
ctx.Route = route
if ps != nil {
r.putParams(ps)
ctx.Params = *ps
}

err = route.handle(ctx)
return
} else if ctx.Request.Method != http.MethodConnect && path != "/" {
Expand Down
21 changes: 5 additions & 16 deletions router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,16 @@ func TestRouter(t *testing.T) {
routed := false
router.Handle(http.MethodGet, "/user/:name", func(ctx *Context) error {
routed = true
want := Params{Param{"name", "gopher"}}
if !reflect.DeepEqual(ctx.Params, want) {
t.Fatalf("wrong wildcard values: want %v, got %v", want, ctx.Params)
}
expected := Params{Param{"name", "gopher"}}
assert.Equal(t, expected, ctx.Params)
return nil
})

w := new(mockResponseWriter)

req, _ := http.NewRequest(http.MethodGet, "/user/gopher", nil)
router.ServeHTTP(w, req)

if !routed {
t.Fatal("routing failed")
}
assert.True(t, routed)
}

type handlerStruct struct {
Expand Down Expand Up @@ -547,9 +542,7 @@ func TestRouterLookup(t *testing.T) {
t.Fatal("Routing failed!")
}
}
if params != nil {
t.Fatalf("Wrong parameter values: want %v, got %v", nil, params)
}
assert.Len(t, params, 0)

route, _, tsr = router.Lookup(http.MethodGet, "/user/gopher/")
if route != nil {
Expand Down Expand Up @@ -581,12 +574,8 @@ func TestRouterParamsFromContext(t *testing.T) {
return nil
}

var nilParams Params
handlerFuncNil := func(ctx *Context) error {
if !reflect.DeepEqual(ctx.Params, nilParams) {
t.Fatalf("Wrong parameter values: want %v, got %v", nilParams, ctx.Params)
}

assert.Len(t, ctx.Params, 0)
routed = true
return nil
}
Expand Down
16 changes: 5 additions & 11 deletions tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func (n *node) insertChild(path, fullPath string, route *Route) {
// If no handle can be found, a TSR (trailing slash redirect) recommendation is
// made if a handle exists with an extra (without the) trailing slash for the
// given path.
func (n *node) getValue(path string, params func() *Params, useRawPath bool) (route *Route, ps *Params, tsr bool) {
func (n *node) getValue(path string, ps *Params, useRawPath bool) (route *Route, tsr bool) {
walk: // Outer loop for walking the tree
for {
prefix := n.path
Expand Down Expand Up @@ -363,13 +363,10 @@ walk: // Outer loop for walking the tree
}

// Save param value
if params != nil {
if ps == nil {
ps = params()
}
if ps != nil {
// Expand slice within preallocated capacity
i := len(*ps)
*ps = (*ps)[:i+1]
(*ps) = (*ps)[:i+1]
param := Param{
Key: n.path[1:],
}
Expand Down Expand Up @@ -407,13 +404,10 @@ walk: // Outer loop for walking the tree

case catchAll:
// Save param value
if params != nil {
if ps == nil {
ps = params()
}
if ps != nil {
// Expand slice within preallocated capacity
i := len(*ps)
*ps = (*ps)[:i+1]
(*ps) = (*ps)[:i+1]
param := Param{
Key: n.path[2:],
}
Expand Down
28 changes: 11 additions & 17 deletions tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ package clevergo

import (
"fmt"
"reflect"
"regexp"
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

func printChildren(n *node, prefix string) {
Expand Down Expand Up @@ -39,14 +40,10 @@ type testRequests []struct {
ps Params
}

func getParams() *Params {
ps := make(Params, 0, 20)
return &ps
}

func checkRequests(t *testing.T, tree *node, requests testRequests) {
for _, request := range requests {
handler, psp, _ := tree.getValue(request.path, getParams, false)
ps := make(Params, 0, 20)
handler, _ := tree.getValue(request.path, &ps, false)

if handler == nil {
if !request.nilHandler {
Expand All @@ -61,13 +58,10 @@ func checkRequests(t *testing.T, tree *node, requests testRequests) {
}
}

var ps Params
if psp != nil {
ps = *psp
}

if !reflect.DeepEqual(ps, request.ps) {
t.Errorf("Params mismatch for route '%s'", request.path)
if request.ps == nil {
assert.Len(t, ps, 0)
} else {
assert.Equal(t, request.ps, ps)
}
}
}
Expand Down Expand Up @@ -434,7 +428,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
"/doc/",
}
for _, route := range tsrRoutes {
handler, _, tsr := tree.getValue(route, nil, false)
handler, tsr := tree.getValue(route, nil, false)
if handler != nil {
t.Fatalf("non-nil handler for TSR route '%s", route)
} else if !tsr {
Expand All @@ -451,7 +445,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
"/api/world/abc",
}
for _, route := range noTsrRoutes {
handler, _, tsr := tree.getValue(route, nil, false)
handler, tsr := tree.getValue(route, nil, false)
if handler != nil {
t.Fatalf("non-nil handler for No-TSR route '%s", route)
} else if tsr {
Expand All @@ -470,7 +464,7 @@ func TestTreeRootTrailingSlashRedirect(t *testing.T) {
t.Fatalf("panic inserting test route: %v", recv)
}

handler, _, tsr := tree.getValue("/", nil, false)
handler, tsr := tree.getValue("/", nil, false)
if handler != nil {
t.Fatalf("non-nil handler")
} else if tsr {
Expand Down

0 comments on commit c2ecabe

Please sign in to comment.