From 17e7776e16eb40ebb04c8a1ef3ae417f725e9654 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 7 May 2020 13:24:09 +0800 Subject: [PATCH 01/15] update tree Signed-off-by: Bo-Yi Wu --- tree.go | 382 ++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 299 insertions(+), 83 deletions(-) diff --git a/tree.go b/tree.go index b687ec43d5..5a2d5b2ddf 100644 --- a/tree.go +++ b/tree.go @@ -8,6 +8,7 @@ import ( "net/url" "strings" "unicode" + "unicode/utf8" ) // Param is a single URL parameter, consisting of a key and a value. @@ -566,105 +567,321 @@ walk: // Outer loop for walking the tree // It can optionally also fix trailing slashes. // It returns the case-corrected path and a bool indicating whether the lookup // was successful. -func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPath []byte, found bool) { - ciPath = make([]byte, 0, len(path)+1) // preallocate enough memory +func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) ([]byte, bool) { + const stackBufSize = 128 + + // Use a static sized buffer on the stack in the common case. + // If the path is too long, allocate a buffer on the heap instead. + buf := make([]byte, 0, stackBufSize) + if l := len(path) + 1; l > stackBufSize { + buf = make([]byte, 0, l) + } + + ciPath := n.findCaseInsensitivePathRec( + path, + buf, // Preallocate enough memory for new path + [4]byte{}, // Empty rune buffer + fixTrailingSlash, + ) + + return ciPath, ciPath != nil + // ciPath = make([]byte, 0, len(path)+1) // preallocate enough memory + + // // Outer loop for walking the tree + // for len(path) >= len(n.path) && strings.EqualFold(path[:len(n.path)], n.path) { + // path = path[len(n.path):] + // ciPath = append(ciPath, n.path...) + + // if len(path) == 0 { + // // We should have reached the node containing the handle. + // // Check if this node has a handle registered. + // if n.handlers != nil { + // return ciPath, true + // } + + // // No handle found. + // // Try to fix the path by adding a trailing slash + // if fixTrailingSlash { + // for i := 0; i < len(n.indices); i++ { + // if n.indices[i] == '/' { + // n = n.children[i] + // if (len(n.path) == 1 && n.handlers != nil) || + // (n.nType == catchAll && n.children[0].handlers != nil) { + // return append(ciPath, '/'), true + // } + // return + // } + // } + // } + // return + // } + + // // If this node does not have a wildcard (param or catchAll) child, + // // we can just look up the next child node and continue to walk down + // // the tree + // if !n.wildChild { + // r := unicode.ToLower(rune(path[0])) + // for i, index := range n.indices { + // // must use recursive approach since both index and + // // ToLower(index) could exist. We must check both. + // if r == unicode.ToLower(index) { + // out, found := n.children[i].findCaseInsensitivePath(path, fixTrailingSlash) + // if found { + // return append(ciPath, out...), true + // } + // } + // } + + // // Nothing found. We can recommend to redirect to the same URL + // // without a trailing slash if a leaf exists for that path + // found = fixTrailingSlash && path == "/" && n.handlers != nil + // return + // } + + // n = n.children[0] + // switch n.nType { + // case param: + // // Find param end (either '/' or path end) + // end := 0 + // for end < len(path) && path[end] != '/' { + // end++ + // } + + // // add param value to case insensitive path + // ciPath = append(ciPath, path[:end]...) + + // // we need to go deeper! + // if end < len(path) { + // if len(n.children) > 0 { + // path = path[end:] + // n = n.children[0] + // continue + // } + + // // ... but we can't + // if fixTrailingSlash && len(path) == end+1 { + // return ciPath, true + // } + // return + // } + + // if n.handlers != nil { + // return ciPath, true + // } + // if fixTrailingSlash && len(n.children) == 1 { + // // No handle found. Check if a handle for this path + a + // // trailing slash exists + // n = n.children[0] + // if n.path == "/" && n.handlers != nil { + // return append(ciPath, '/'), true + // } + // } + // return + + // case catchAll: + // return append(ciPath, path...), true + + // default: + // panic("invalid node type") + // } + // } + + // // Nothing found. + // // Try to fix the path by adding / removing a trailing slash + // if fixTrailingSlash { + // if path == "/" { + // return ciPath, true + // } + // if len(path)+1 == len(n.path) && n.path[len(path)] == '/' && + // strings.EqualFold(path, n.path[:len(path)]) && + // n.handlers != nil { + // return append(ciPath, n.path...), true + // } + // } + // return +} + +// Shift bytes in array by n bytes left +func shiftNRuneBytes(rb [4]byte, n int) [4]byte { + switch n { + case 0: + return rb + case 1: + return [4]byte{rb[1], rb[2], rb[3], 0} + case 2: + return [4]byte{rb[2], rb[3]} + case 3: + return [4]byte{rb[3]} + default: + return [4]byte{} + } +} + +// Recursive case-insensitive lookup function used by n.findCaseInsensitivePath +func (n *node) findCaseInsensitivePathRec(path string, ciPath []byte, rb [4]byte, fixTrailingSlash bool) []byte { + npLen := len(n.path) - // Outer loop for walking the tree - for len(path) >= len(n.path) && strings.EqualFold(path[:len(n.path)], n.path) { - path = path[len(n.path):] +walk: // Outer loop for walking the tree + for len(path) >= npLen && (npLen == 0 || strings.EqualFold(path[1:npLen], n.path[1:])) { + // Add common prefix to result + oldPath := path + path = path[npLen:] ciPath = append(ciPath, n.path...) - if len(path) == 0 { - // We should have reached the node containing the handle. - // Check if this node has a handle registered. - if n.handlers != nil { - return ciPath, true - } + if len(path) > 0 { + // If this node does not have a wildcard (param or catchAll) child, + // we can just look up the next child node and continue to walk down + // the tree + if !n.wildChild { + // Skip rune bytes already processed + rb = shiftNRuneBytes(rb, npLen) + + if rb[0] != 0 { + // Old rune not finished + idxc := rb[0] + for i, c := range []byte(n.indices) { + if c == idxc { + // continue with child node + n = n.children[i] + npLen = len(n.path) + continue walk + } + } + } else { + // Process a new rune + var rv rune + + // Find rune start. + // Runes are up to 4 byte long, + // -4 would definitely be another rune. + var off int + for max := min(npLen, 3); off < max; off++ { + if i := npLen - off; utf8.RuneStart(oldPath[i]) { + // read rune from cached path + rv, _ = utf8.DecodeRuneInString(oldPath[i:]) + break + } + } - // No handle found. - // Try to fix the path by adding a trailing slash - if fixTrailingSlash { - for i := 0; i < len(n.indices); i++ { - if n.indices[i] == '/' { - n = n.children[i] - if (len(n.path) == 1 && n.handlers != nil) || - (n.nType == catchAll && n.children[0].handlers != nil) { - return append(ciPath, '/'), true + // Calculate lowercase bytes of current rune + lo := unicode.ToLower(rv) + utf8.EncodeRune(rb[:], lo) + + // Skip already processed bytes + rb = shiftNRuneBytes(rb, off) + + idxc := rb[0] + for i, c := range []byte(n.indices) { + // Lowercase matches + if c == idxc { + // must use a recursive approach since both the + // uppercase byte and the lowercase byte might exist + // as an index + if out := n.children[i].findCaseInsensitivePathRec( + path, ciPath, rb, fixTrailingSlash, + ); out != nil { + return out + } + break } - return } - } - } - return - } - // If this node does not have a wildcard (param or catchAll) child, - // we can just look up the next child node and continue to walk down - // the tree - if !n.wildChild { - r := unicode.ToLower(rune(path[0])) - for i, index := range n.indices { - // must use recursive approach since both index and - // ToLower(index) could exist. We must check both. - if r == unicode.ToLower(index) { - out, found := n.children[i].findCaseInsensitivePath(path, fixTrailingSlash) - if found { - return append(ciPath, out...), true + // If we found no match, the same for the uppercase rune, + // if it differs + if up := unicode.ToUpper(rv); up != lo { + utf8.EncodeRune(rb[:], up) + rb = shiftNRuneBytes(rb, off) + + idxc := rb[0] + for i, c := range []byte(n.indices) { + // Uppercase matches + if c == idxc { + // Continue with child node + n = n.children[i] + npLen = len(n.path) + continue walk + } + } } } + + // Nothing found. We can recommend to redirect to the same URL + // without a trailing slash if a leaf exists for that path + if fixTrailingSlash && path == "/" && n.handlers != nil { + return ciPath + } + return nil } - // Nothing found. We can recommend to redirect to the same URL - // without a trailing slash if a leaf exists for that path - found = fixTrailingSlash && path == "/" && n.handlers != nil - return - } + n = n.children[0] + switch n.nType { + case param: + // Find param end (either '/' or path end) + end := 0 + for end < len(path) && path[end] != '/' { + end++ + } - n = n.children[0] - switch n.nType { - case param: - // Find param end (either '/' or path end) - end := 0 - for end < len(path) && path[end] != '/' { - end++ - } + // Add param value to case insensitive path + ciPath = append(ciPath, path[:end]...) - // add param value to case insensitive path - ciPath = append(ciPath, path[:end]...) + // We need to go deeper! + if end < len(path) { + if len(n.children) > 0 { + // Continue with child node + n = n.children[0] + npLen = len(n.path) + path = path[end:] + continue + } - // we need to go deeper! - if end < len(path) { - if len(n.children) > 0 { - path = path[end:] - n = n.children[0] - continue + // ... but we can't + if fixTrailingSlash && len(path) == end+1 { + return ciPath + } + return nil } - // ... but we can't - if fixTrailingSlash && len(path) == end+1 { - return ciPath, true + if n.handlers != nil { + return ciPath + } else if fixTrailingSlash && len(n.children) == 1 { + // No handle found. Check if a handle for this path + a + // trailing slash exists + n = n.children[0] + if n.path == "/" && n.handlers != nil { + return append(ciPath, '/') + } } - return - } + return nil + + case catchAll: + return append(ciPath, path...) + default: + panic("invalid node type") + } + } else { + // We should have reached the node containing the handle. + // Check if this node has a handle registered. if n.handlers != nil { - return ciPath, true + return ciPath } - if fixTrailingSlash && len(n.children) == 1 { - // No handle found. Check if a handle for this path + a - // trailing slash exists - n = n.children[0] - if n.path == "/" && n.handlers != nil { - return append(ciPath, '/'), true + + // No handle found. + // Try to fix the path by adding a trailing slash + if fixTrailingSlash { + for i, c := range []byte(n.indices) { + if c == '/' { + n = n.children[i] + if (len(n.path) == 1 && n.handlers != nil) || + (n.nType == catchAll && n.children[0].handlers != nil) { + return append(ciPath, '/') + } + return nil + } } } - return - - case catchAll: - return append(ciPath, path...), true - - default: - panic("invalid node type") + return nil } } @@ -672,13 +889,12 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPa // Try to fix the path by adding / removing a trailing slash if fixTrailingSlash { if path == "/" { - return ciPath, true + return ciPath } - if len(path)+1 == len(n.path) && n.path[len(path)] == '/' && - strings.EqualFold(path, n.path[:len(path)]) && - n.handlers != nil { - return append(ciPath, n.path...), true + if len(path)+1 == npLen && n.path[len(path)] == '/' && + strings.EqualFold(path[1:], n.path[1:len(path)]) && n.handlers != nil { + return append(ciPath, n.path...) } } - return + return nil } From e952fe3517b32fbef42e37c4635bd58acf857ef6 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 7 May 2020 15:19:54 +0800 Subject: [PATCH 02/15] update Signed-off-by: Bo-Yi Wu --- tree.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tree.go b/tree.go index 5a2d5b2ddf..fc024f7b7a 100644 --- a/tree.go +++ b/tree.go @@ -434,6 +434,9 @@ walk: // Outer loop for walking the tree return } + // If there is no handle for this route, but this route has a + // wildcard child, there must be a handle for this path with an + // additional trailing slash if path == "/" && n.wildChild && n.nType != root { value.tsr = true return @@ -441,9 +444,8 @@ walk: // Outer loop for walking the tree // No handle found. Check if a handle for this path + a // trailing slash exists for trailing slash recommendation - indices := n.indices - for i, max := 0, len(indices); i < max; i++ { - if indices[i] == '/' { + for i, c := range []byte(n.indices) { + if c == '/' { n = n.children[i] value.tsr = (len(n.path) == 1 && n.handlers != nil) || (n.nType == catchAll && n.children[0].handlers != nil) @@ -563,7 +565,7 @@ walk: // Outer loop for walking the tree } } -// findCaseInsensitivePath makes a case-insensitive lookup of the given path and tries to find a handler. +// Makes a case-insensitive lookup of the given path and tries to find a handler. // It can optionally also fix trailing slashes. // It returns the case-corrected path and a bool indicating whether the lookup // was successful. From ac41b9b48645f4ff991e5bf34f6ecb79b7969d12 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 7 May 2020 15:29:28 +0800 Subject: [PATCH 03/15] update Signed-off-by: Bo-Yi Wu --- gin.go | 4 ++-- tree.go | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/gin.go b/gin.go index 4e72f81dbb..7c337d23d1 100644 --- a/gin.go +++ b/gin.go @@ -259,7 +259,7 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { root := engine.trees.get(method) if root == nil { root = new(node) - root.fullPath = "/" + // root.fullPath = "/" engine.trees = append(engine.trees, methodTree{method: method, root: root}) } root.addRoute(path, handlers) @@ -406,7 +406,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) { if value.handlers != nil { c.handlers = value.handlers c.Params = value.params - c.fullPath = value.fullPath + // c.fullPath = value.fullPath c.Next() c.writermem.WriteHeaderNow() return diff --git a/tree.go b/tree.go index fc024f7b7a..e9349ea9b8 100644 --- a/tree.go +++ b/tree.go @@ -103,7 +103,7 @@ type node struct { nType nodeType maxParams uint8 wildChild bool - fullPath string + // fullPath string } // increments priority of the given child and reorders if necessary. @@ -166,7 +166,7 @@ walk: children: n.children, handlers: n.handlers, priority: n.priority - 1, - fullPath: n.fullPath, + // fullPath: n.fullPath, } // Update maxParams (max of all children) @@ -182,7 +182,7 @@ walk: n.path = path[:i] n.handlers = nil n.wildChild = false - n.fullPath = fullPath[:parentFullPathIndex+i] + // n.fullPath = fullPath[:parentFullPathIndex+i] } // Make new node a child of this node @@ -246,7 +246,7 @@ walk: n.indices += string([]byte{c}) child := &node{ maxParams: numParams, - fullPath: fullPath, + // fullPath: fullPath, } n.children = append(n.children, child) n.incrementChildPrio(len(n.indices) - 1) @@ -328,7 +328,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle nType: param, path: wildcard, maxParams: numParams, - fullPath: fullPath, + // fullPath: fullPath, } n.children = []*node{child} n = child @@ -343,7 +343,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle child := &node{ maxParams: numParams, priority: 1, - fullPath: fullPath, + // fullPath: fullPath, } n.children = []*node{child} n = child @@ -377,7 +377,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle wildChild: true, nType: catchAll, maxParams: 1, - fullPath: fullPath, + // fullPath: fullPath, } // update maxParams of the parent node if n.maxParams < 1 { @@ -392,10 +392,10 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle child = &node{ path: path[i:], nType: catchAll, - maxParams: 1, handlers: handlers, priority: 1, - fullPath: fullPath, + maxParams: 1, + // fullPath: fullPath, } n.children = []*node{child} @@ -405,7 +405,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle // If no wildcard was found, simply insert the path and handle n.path = path n.handlers = handlers - n.fullPath = fullPath + // n.fullPath = fullPath } // nodeValue holds return values of (*Node).getValue method @@ -413,7 +413,7 @@ type nodeValue struct { handlers HandlersChain params Params tsr bool - fullPath string + // fullPath string } // getValue returns the handle registered with the given path (key). The values of @@ -430,7 +430,7 @@ walk: // Outer loop for walking the tree // We should have reached the node containing the handle. // Check if this node has a handle registered. if value.handlers = n.handlers; value.handlers != nil { - value.fullPath = n.fullPath + // value.fullPath = n.fullPath return } @@ -519,7 +519,7 @@ walk: // Outer loop for walking the tree } if value.handlers = n.handlers; value.handlers != nil { - value.fullPath = n.fullPath + // value.fullPath = n.fullPath return } if len(n.children) == 1 { @@ -548,7 +548,7 @@ walk: // Outer loop for walking the tree } value.handlers = n.handlers - value.fullPath = n.fullPath + // value.fullPath = n.fullPath return default: From dc85754c121b98c6bec2a3206691c64611f87056 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 7 May 2020 15:40:25 +0800 Subject: [PATCH 04/15] update countParams Signed-off-by: Bo-Yi Wu --- tree.go | 18 ++++++++---------- tree_test.go | 4 ++-- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/tree.go b/tree.go index e9349ea9b8..e3180f47e0 100644 --- a/tree.go +++ b/tree.go @@ -72,17 +72,15 @@ func longestCommonPrefix(a, b string) int { return i } -func countParams(path string) uint8 { +func countParams(path string) uint16 { var n uint - for i := 0; i < len(path); i++ { - if path[i] == ':' || path[i] == '*' { + for i := range []byte(path) { + switch path[i] { + case ':', '*': n++ } } - if n >= 255 { - return 255 - } - return uint8(n) + return uint16(n) } type nodeType uint8 @@ -101,7 +99,7 @@ type node struct { handlers HandlersChain priority uint32 nType nodeType - maxParams uint8 + maxParams uint16 wildChild bool // fullPath string } @@ -266,7 +264,7 @@ walk: } // Search for a wildcard segment and check the name for invalid characters. -// Returns -1 as index, if no wildcard war found. +// Returns -1 as index, if no wildcard was found. func findWildcard(path string) (wildcard string, i int, valid bool) { // Find start for start, c := range []byte(path) { @@ -290,7 +288,7 @@ func findWildcard(path string) (wildcard string, i int, valid bool) { return "", -1, false } -func (n *node) insertChild(numParams uint8, path string, fullPath string, handlers HandlersChain) { +func (n *node) insertChild(numParams uint16, path string, fullPath string, handlers HandlersChain) { for numParams > 0 { // Find prefix until first wildcard wildcard, i, valid := findWildcard(path) diff --git a/tree_test.go b/tree_test.go index 0fe2fe0011..993af08e0e 100644 --- a/tree_test.go +++ b/tree_test.go @@ -76,8 +76,8 @@ func checkPriorities(t *testing.T, n *node) uint32 { return prio } -func checkMaxParams(t *testing.T, n *node) uint8 { - var maxParams uint8 +func checkMaxParams(t *testing.T, n *node) uint16 { + var maxParams uint16 for i := range n.children { params := checkMaxParams(t, n.children[i]) if params > maxParams { From 717b2145d41327d6eb1c9981130ea93b4c2d2878 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 7 May 2020 16:10:54 +0800 Subject: [PATCH 05/15] fix testing Signed-off-by: Bo-Yi Wu --- routes_test.go | 78 +++++++++++++++++++++++++------------------------- tree_test.go | 8 +++--- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/routes_test.go b/routes_test.go index 360ade6215..d6c513379f 100644 --- a/routes_test.go +++ b/routes_test.go @@ -579,42 +579,42 @@ func TestRouteServeErrorWithWriteHeader(t *testing.T) { assert.Equal(t, 0, w.Body.Len()) } -func TestRouteContextHoldsFullPath(t *testing.T) { - router := New() - - // Test routes - routes := []string{ - "/simple", - "/project/:name", - "/", - "/news/home", - "/news", - "/simple-two/one", - "/simple-two/one-two", - "/project/:name/build/*params", - "/project/:name/bui", - } - - for _, route := range routes { - actualRoute := route - router.GET(route, func(c *Context) { - // For each defined route context should contain its full path - assert.Equal(t, actualRoute, c.FullPath()) - c.AbortWithStatus(http.StatusOK) - }) - } - - for _, route := range routes { - w := performRequest(router, http.MethodGet, route) - assert.Equal(t, http.StatusOK, w.Code) - } - - // Test not found - router.Use(func(c *Context) { - // For not found routes full path is empty - assert.Equal(t, "", c.FullPath()) - }) - - w := performRequest(router, http.MethodGet, "/not-found") - assert.Equal(t, http.StatusNotFound, w.Code) -} +// func TestRouteContextHoldsFullPath(t *testing.T) { +// router := New() + +// // Test routes +// routes := []string{ +// "/simple", +// "/project/:name", +// "/", +// "/news/home", +// "/news", +// "/simple-two/one", +// "/simple-two/one-two", +// "/project/:name/build/*params", +// "/project/:name/bui", +// } + +// for _, route := range routes { +// actualRoute := route +// router.GET(route, func(c *Context) { +// // For each defined route context should contain its full path +// assert.Equal(t, actualRoute, c.FullPath()) +// c.AbortWithStatus(http.StatusOK) +// }) +// } + +// for _, route := range routes { +// w := performRequest(router, http.MethodGet, route) +// assert.Equal(t, http.StatusOK, w.Code) +// } + +// // Test not found +// router.Use(func(c *Context) { +// // For not found routes full path is empty +// assert.Equal(t, "", c.FullPath()) +// }) + +// w := performRequest(router, http.MethodGet, "/not-found") +// assert.Equal(t, http.StatusNotFound, w.Code) +// } diff --git a/tree_test.go b/tree_test.go index 993af08e0e..473a101199 100644 --- a/tree_test.go +++ b/tree_test.go @@ -99,12 +99,12 @@ func checkMaxParams(t *testing.T, n *node) uint16 { } func TestCountParams(t *testing.T) { - if countParams("/path/:param1/static/*catch-all") != 2 { - t.Fail() - } - if countParams(strings.Repeat("/:param", 256)) != 255 { + if countParams("/path/:param1/static/*catch-all") != uint16(2) { t.Fail() } + // if countParams(strings.Repeat("/:param", 256)) != 255 { + // t.Fail() + // } } func TestTreeAddAndGet(t *testing.T) { From 9e8404975e3326de4d29bd6a6a36e6e219bc98fe Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Fri, 8 May 2020 00:13:40 +0800 Subject: [PATCH 06/15] update Signed-off-by: Bo-Yi Wu --- gin.go | 44 +++++++++- tree.go | 246 ++++++++++++++++++++++++++++++++------------------------ 2 files changed, 183 insertions(+), 107 deletions(-) diff --git a/gin.go b/gin.go index 7c337d23d1..49091303a9 100644 --- a/gin.go +++ b/gin.go @@ -113,6 +113,8 @@ type Engine struct { noMethod HandlersChain pool sync.Pool trees methodTrees + // paramsPool sync.Pool + maxParams uint16 } var _ IRouter = &Engine{} @@ -256,6 +258,8 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { assert1(len(handlers) > 0, "there must be at least one handler") debugPrintRoute(method, path, handlers) + // varsCount := uint16(0) + root := engine.trees.get(method) if root == nil { root = new(node) @@ -263,6 +267,19 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { engine.trees = append(engine.trees, methodTree{method: method, root: root}) } root.addRoute(path, handlers) + + // Update maxParams + // if paramsCount := countParams(path); paramsCount+varsCount > root.maxParams { + // engine.maxParams = paramsCount + varsCount + // } + + // Lazy-init paramsPool alloc func + // if engine.paramsPool.New == nil && engine.maxParams > 0 { + // engine.paramsPool.New = func() interface{} { + // ps := make(Params, 0, engine.maxParams) + // return &ps + // } + // } } // Routes returns a slice of registered routes, including some useful information, such as: @@ -381,6 +398,23 @@ func (engine *Engine) HandleContext(c *Context) { c.index = oldIndexValue } +// func (engine *Engine) getParams() *Params { +// // c := engine.pool.Get().(*Context) +// // c.Params = c.Params[0:0] +// ps := engine.paramsPool.Get().(*Params) +// *ps = (*ps)[0:0] // reset slice +// return ps +// } + +// func (engine *Engine) putParams(ps *Params) { +// // c := engine.pool.Get().(*Context) +// // c.Params = *ps +// // engine.pool.Put(c) +// if ps != nil { +// engine.paramsPool.Put(ps) +// } +// } + func (engine *Engine) handleHTTPRequest(c *Context) { httpMethod := c.Request.Method rPath := c.Request.URL.Path @@ -402,10 +436,16 @@ func (engine *Engine) handleHTTPRequest(c *Context) { } root := t[i].root // Find route in tree - value := root.getValue(rPath, c.Params, unescape) + value := root.getValue(rPath, &c.Params, unescape) + if value.params != nil { + // engine.putParams(value.params) + c.Params = *value.params + } if value.handlers != nil { c.handlers = value.handlers - c.Params = value.params + // if value.params != nil { + // c.Params = *value.params + // } // c.fullPath = value.fullPath c.Next() c.writermem.WriteHeaderNow() diff --git a/tree.go b/tree.go index e3180f47e0..d2aec06c56 100644 --- a/tree.go +++ b/tree.go @@ -5,7 +5,6 @@ package gin import ( - "net/url" "strings" "unicode" "unicode/utf8" @@ -132,23 +131,23 @@ func (n *node) incrementChildPrio(pos int) int { func (n *node) addRoute(path string, handlers HandlersChain) { fullPath := path n.priority++ - numParams := countParams(path) + // numParams := countParams(path) // Empty tree if len(n.path) == 0 && len(n.children) == 0 { - n.insertChild(numParams, path, fullPath, handlers) + n.insertChild(path, fullPath, handlers) n.nType = root return } - parentFullPathIndex := 0 + // parentFullPathIndex := 0 walk: for { // Update maxParams of the current node - if numParams > n.maxParams { - n.maxParams = numParams - } + // if numParams > n.maxParams { + // n.maxParams = numParams + // } // Find the longest common prefix. // This also implies that the common prefix contains no ':' or '*' @@ -168,11 +167,11 @@ walk: } // Update maxParams (max of all children) - for _, v := range child.children { - if v.maxParams > child.maxParams { - child.maxParams = v.maxParams - } - } + // for _, v := range child.children { + // if v.maxParams > child.maxParams { + // child.maxParams = v.maxParams + // } + // } n.children = []*node{&child} // []byte for proper unicode char conversion, see #65 @@ -188,15 +187,15 @@ walk: path = path[i:] if n.wildChild { - parentFullPathIndex += len(n.path) + // parentFullPathIndex += len(n.path) n = n.children[0] n.priority++ // Update maxParams of the child node - if numParams > n.maxParams { - n.maxParams = numParams - } - numParams-- + // if numParams > n.maxParams { + // n.maxParams = numParams + // } + // numParams-- // Check if the wildcard matches if len(path) >= len(n.path) && n.path == path[:len(n.path)] { @@ -222,7 +221,7 @@ walk: // slash after param if n.nType == param && c == '/' && len(n.children) == 1 { - parentFullPathIndex += len(n.path) + // parentFullPathIndex += len(n.path) n = n.children[0] n.priority++ continue walk @@ -231,7 +230,7 @@ walk: // Check if a child with the next path byte exists for i, max := 0, len(n.indices); i < max; i++ { if c == n.indices[i] { - parentFullPathIndex += len(n.path) + // parentFullPathIndex += len(n.path) i = n.incrementChildPrio(i) n = n.children[i] continue walk @@ -243,14 +242,14 @@ walk: // []byte for proper unicode char conversion, see #65 n.indices += string([]byte{c}) child := &node{ - maxParams: numParams, + // maxParams: numParams, // fullPath: fullPath, } n.children = append(n.children, child) n.incrementChildPrio(len(n.indices) - 1) n = child } - n.insertChild(numParams, path, fullPath, handlers) + n.insertChild(path, fullPath, handlers) return } @@ -288,8 +287,8 @@ func findWildcard(path string) (wildcard string, i int, valid bool) { return "", -1, false } -func (n *node) insertChild(numParams uint16, path string, fullPath string, handlers HandlersChain) { - for numParams > 0 { +func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) { + for { // Find prefix until first wildcard wildcard, i, valid := findWildcard(path) if i < 0 { // No wildcard found @@ -323,15 +322,15 @@ func (n *node) insertChild(numParams uint16, path string, fullPath string, handl n.wildChild = true child := &node{ - nType: param, - path: wildcard, - maxParams: numParams, + nType: param, + path: wildcard, + // maxParams: numParams, // fullPath: fullPath, } n.children = []*node{child} n = child n.priority++ - numParams-- + // numParams-- // if the path doesn't end with the wildcard, then there // will be another non-wildcard subpath starting with '/' @@ -339,8 +338,8 @@ func (n *node) insertChild(numParams uint16, path string, fullPath string, handl path = path[len(wildcard):] child := &node{ - maxParams: numParams, - priority: 1, + // maxParams: numParams, + priority: 1, // fullPath: fullPath, } n.children = []*node{child} @@ -354,7 +353,7 @@ func (n *node) insertChild(numParams uint16, path string, fullPath string, handl } // catchAll - if i+len(wildcard) != len(path) || numParams > 1 { + if i+len(wildcard) != len(path) { panic("catch-all routes are only allowed at the end of the path in path '" + fullPath + "'") } @@ -374,13 +373,13 @@ func (n *node) insertChild(numParams uint16, path string, fullPath string, handl child := &node{ wildChild: true, nType: catchAll, - maxParams: 1, + // maxParams: 1, // fullPath: fullPath, } // update maxParams of the parent node - if n.maxParams < 1 { - n.maxParams = 1 - } + // if n.maxParams < 1 { + // n.maxParams = 1 + // } n.children = []*node{child} n.indices = string('/') n = child @@ -388,11 +387,11 @@ func (n *node) insertChild(numParams uint16, path string, fullPath string, handl // second node: node holding the variable child = &node{ - path: path[i:], - nType: catchAll, - handlers: handlers, - priority: 1, - maxParams: 1, + path: path[i:], + nType: catchAll, + handlers: handlers, + priority: 1, + // maxParams: 1, // fullPath: fullPath, } n.children = []*node{child} @@ -409,7 +408,7 @@ func (n *node) insertChild(numParams uint16, path string, fullPath string, handl // nodeValue holds return values of (*Node).getValue method type nodeValue struct { handlers HandlersChain - params Params + params *Params tsr bool // fullPath string } @@ -419,51 +418,21 @@ type nodeValue struct { // 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, po Params, unescape bool) (value nodeValue) { - value.params = po +func (n *node) getValue(path string, params *Params, unescape bool) (value nodeValue) { + value = nodeValue{} walk: // Outer loop for walking the tree for { prefix := n.path - if path == prefix { - // We should have reached the node containing the handle. - // Check if this node has a handle registered. - if value.handlers = n.handlers; value.handlers != nil { - // value.fullPath = n.fullPath - return - } - - // If there is no handle for this route, but this route has a - // wildcard child, there must be a handle for this path with an - // additional trailing slash - if path == "/" && n.wildChild && n.nType != root { - value.tsr = true - return - } - - // No handle found. Check if a handle for this path + a - // trailing slash exists for trailing slash recommendation - for i, c := range []byte(n.indices) { - if c == '/' { - n = n.children[i] - value.tsr = (len(n.path) == 1 && n.handlers != nil) || - (n.nType == catchAll && n.children[0].handlers != nil) - return - } - } - - return - } if len(path) > len(prefix) && path[:len(prefix)] == prefix { path = path[len(prefix):] // If this node does not have a wildcard (param or catchAll) - // child, we can just look up the next child node and continue + // child, we can just look up the next child node and continue // to walk down the tree if !n.wildChild { - c := path[0] - indices := n.indices - for i, max := 0, len(indices); i < max; i++ { - if c == indices[i] { + idxc := path[0] + for i, c := range []byte(n.indices) { + if c == idxc { n = n.children[i] continue walk } @@ -472,7 +441,7 @@ walk: // Outer loop for walking the tree // Nothing found. // We can recommend to redirect to the same URL without a // trailing slash if a leaf exists for that path. - value.tsr = path == "/" && n.handlers != nil + value.tsr = (path == "/" && n.handlers != nil) return } @@ -487,20 +456,38 @@ walk: // Outer loop for walking the tree } // save param value - if cap(value.params) < int(n.maxParams) { - value.params = make(Params, 0, n.maxParams) - } - i := len(value.params) - value.params = value.params[:i+1] // expand slice within preallocated capacity - value.params[i].Key = n.path[1:] - val := path[:end] - if unescape { - var err error - if value.params[i].Value, err = url.QueryUnescape(val); err != nil { - value.params[i].Value = val // fallback, in case of error + // if cap(value.params) < int(n.maxParams) { + // value.params = make(Params, 0, n.maxParams) + // } + // i := len(value.params) + // value.params = value.params[:i+1] // expand slice within preallocated capacity + // value.params[i].Key = n.path[1:] + // val := path[:end] + // if unescape { + // var err error + // if value.params[i].Value, err = url.QueryUnescape(val); err != nil { + // value.params[i].Value = val // fallback, in case of error + // } + // } else { + // value.params[i].Value = val + // } + if params != nil { + if value.params == nil { + value.params = params + } + // Expand slice within preallocated capacity + i := len(*value.params) + *value.params = (*value.params)[:i+1] + // val := path[:end] + // if unescape { + // if v, err := url.QueryUnescape(val); err == nil { + // val = v + // } + // } + (*value.params)[i] = Param{ + Key: n.path[1:], + Value: path, } - } else { - value.params[i].Value = val } // we need to go deeper! @@ -512,7 +499,7 @@ walk: // Outer loop for walking the tree } // ... but we can't - value.tsr = len(path) == end+1 + value.tsr = (len(path) == end+1) return } @@ -524,25 +511,44 @@ walk: // Outer loop for walking the tree // No handle found. Check if a handle for this path + a // trailing slash exists for TSR recommendation n = n.children[0] - value.tsr = n.path == "/" && n.handlers != nil + value.tsr = (n.path == "/" && n.handlers != nil) } return case catchAll: - // save param value - if cap(value.params) < int(n.maxParams) { - value.params = make(Params, 0, n.maxParams) - } - i := len(value.params) - value.params = value.params[:i+1] // expand slice within preallocated capacity - value.params[i].Key = n.path[2:] - if unescape { - var err error - if value.params[i].Value, err = url.QueryUnescape(path); err != nil { - value.params[i].Value = path // fallback, in case of error + // // save param value + // if cap(value.params) < int(n.maxParams) { + // value.params = make(Params, 0, n.maxParams) + // } + // i := len(value.params) + // value.params = value.params[:i+1] // expand slice within preallocated capacity + // value.params[i].Key = n.path[2:] + // if unescape { + // var err error + // if value.params[i].Value, err = url.QueryUnescape(path); err != nil { + // value.params[i].Value = path // fallback, in case of error + // } + // } else { + // value.params[i].Value = path + // } + + // Save param value + if params != nil { + if value.params == nil { + value.params = params + } + // Expand slice within preallocated capacity + i := len(*value.params) + *value.params = (*value.params)[:i+1] + // if unescape { + // if v, err := url.QueryUnescape(path); err == nil { + // path = v + // } + // } + (*value.params)[i] = Param{ + Key: n.path[2:], + Value: path, } - } else { - value.params[i].Value = path } value.handlers = n.handlers @@ -554,6 +560,36 @@ walk: // Outer loop for walking the tree } } + if path == prefix { + // We should have reached the node containing the handle. + // Check if this node has a handle registered. + if value.handlers = n.handlers; value.handlers != nil { + // value.fullPath = n.fullPath + return + } + + // If there is no handle for this route, but this route has a + // wildcard child, there must be a handle for this path with an + // additional trailing slash + if path == "/" && n.wildChild && n.nType != root { + value.tsr = true + return + } + + // No handle found. Check if a handle for this path + a + // trailing slash exists for trailing slash recommendation + for i, c := range []byte(n.indices) { + if c == '/' { + n = n.children[i] + value.tsr = (len(n.path) == 1 && n.handlers != nil) || + (n.nType == catchAll && n.children[0].handlers != nil) + return + } + } + + return + } + // Nothing found. We can recommend to redirect to the same URL with an // extra trailing slash if a leaf exists for that path value.tsr = (path == "/") || From 71f841a1f5c083322d0ad7cc3fbf337631b9a326 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Fri, 8 May 2020 02:13:52 +0800 Subject: [PATCH 07/15] update Signed-off-by: Bo-Yi Wu --- gin.go | 58 ++++++++++++++++++++++++++++----------------------------- tree.go | 9 ++++++--- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/gin.go b/gin.go index 49091303a9..292aea3c89 100644 --- a/gin.go +++ b/gin.go @@ -113,8 +113,8 @@ type Engine struct { noMethod HandlersChain pool sync.Pool trees methodTrees - // paramsPool sync.Pool - maxParams uint16 + paramsPool sync.Pool + maxParams uint16 } var _ IRouter = &Engine{} @@ -258,7 +258,7 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { assert1(len(handlers) > 0, "there must be at least one handler") debugPrintRoute(method, path, handlers) - // varsCount := uint16(0) + varsCount := uint16(0) root := engine.trees.get(method) if root == nil { @@ -269,17 +269,17 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { root.addRoute(path, handlers) // Update maxParams - // if paramsCount := countParams(path); paramsCount+varsCount > root.maxParams { - // engine.maxParams = paramsCount + varsCount - // } + if paramsCount := countParams(path); paramsCount+varsCount > root.maxParams { + engine.maxParams = paramsCount + varsCount + } // Lazy-init paramsPool alloc func - // if engine.paramsPool.New == nil && engine.maxParams > 0 { - // engine.paramsPool.New = func() interface{} { - // ps := make(Params, 0, engine.maxParams) - // return &ps - // } - // } + if engine.paramsPool.New == nil && engine.maxParams > 0 { + engine.paramsPool.New = func() interface{} { + ps := make(Params, 0, engine.maxParams) + return &ps + } + } } // Routes returns a slice of registered routes, including some useful information, such as: @@ -398,22 +398,22 @@ func (engine *Engine) HandleContext(c *Context) { c.index = oldIndexValue } -// func (engine *Engine) getParams() *Params { -// // c := engine.pool.Get().(*Context) -// // c.Params = c.Params[0:0] -// ps := engine.paramsPool.Get().(*Params) -// *ps = (*ps)[0:0] // reset slice -// return ps -// } +func (engine *Engine) getParams() *Params { + // c := engine.pool.Get().(*Context) + // c.Params = c.Params[0:0] + ps := engine.paramsPool.Get().(*Params) + *ps = (*ps)[0:0] // reset slice + return ps +} -// func (engine *Engine) putParams(ps *Params) { -// // c := engine.pool.Get().(*Context) -// // c.Params = *ps -// // engine.pool.Put(c) -// if ps != nil { -// engine.paramsPool.Put(ps) -// } -// } +func (engine *Engine) putParams(ps *Params) { + // c := engine.pool.Get().(*Context) + // c.Params = *ps + // engine.pool.Put(c) + if ps != nil { + engine.paramsPool.Put(ps) + } +} func (engine *Engine) handleHTTPRequest(c *Context) { httpMethod := c.Request.Method @@ -436,9 +436,9 @@ func (engine *Engine) handleHTTPRequest(c *Context) { } root := t[i].root // Find route in tree - value := root.getValue(rPath, &c.Params, unescape) + value := root.getValue(rPath, engine.getParams, unescape) if value.params != nil { - // engine.putParams(value.params) + engine.putParams(value.params) c.Params = *value.params } if value.handlers != nil { diff --git a/tree.go b/tree.go index d2aec06c56..c4ccc71c30 100644 --- a/tree.go +++ b/tree.go @@ -5,6 +5,7 @@ package gin import ( + "log" "strings" "unicode" "unicode/utf8" @@ -418,7 +419,7 @@ type nodeValue struct { // 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 *Params, unescape bool) (value nodeValue) { +func (n *node) getValue(path string, params func() *Params, unescape bool) (value nodeValue) { value = nodeValue{} walk: // Outer loop for walking the tree for { @@ -473,10 +474,12 @@ walk: // Outer loop for walking the tree // } if params != nil { if value.params == nil { - value.params = params + value.params = params() } // Expand slice within preallocated capacity i := len(*value.params) + log.Println(i) + log.Println(*value.params) *value.params = (*value.params)[:i+1] // val := path[:end] // if unescape { @@ -535,7 +538,7 @@ walk: // Outer loop for walking the tree // Save param value if params != nil { if value.params == nil { - value.params = params + value.params = params() } // Expand slice within preallocated capacity i := len(*value.params) From fe436bf2c07e310864f9016ea2215520645d47ee Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Fri, 8 May 2020 13:12:41 +0800 Subject: [PATCH 08/15] udpate Signed-off-by: Bo-Yi Wu --- gin.go | 5 +- tree.go | 234 ++++++++++++++++++++++++++++---------------------------- 2 files changed, 119 insertions(+), 120 deletions(-) diff --git a/gin.go b/gin.go index 292aea3c89..ce12219e05 100644 --- a/gin.go +++ b/gin.go @@ -253,6 +253,7 @@ func (engine *Engine) rebuild405Handlers() { } func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { + // log.Println(method, path) assert1(path[0] == '/', "path must begin with '/'") assert1(method != "", "HTTP method can not be empty") assert1(len(handlers) > 0, "there must be at least one handler") @@ -269,13 +270,15 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { root.addRoute(path, handlers) // Update maxParams - if paramsCount := countParams(path); paramsCount+varsCount > root.maxParams { + // log.Println("countParams(path):", countParams(path)) + if paramsCount := countParams(path); paramsCount+varsCount > engine.maxParams { engine.maxParams = paramsCount + varsCount } // Lazy-init paramsPool alloc func if engine.paramsPool.New == nil && engine.maxParams > 0 { engine.paramsPool.New = func() interface{} { + // log.Println("engine.maxParams:", engine.maxParams) ps := make(Params, 0, engine.maxParams) return &ps } diff --git a/tree.go b/tree.go index c4ccc71c30..cd376c33ad 100644 --- a/tree.go +++ b/tree.go @@ -5,7 +5,6 @@ package gin import ( - "log" "strings" "unicode" "unicode/utf8" @@ -424,146 +423,143 @@ func (n *node) getValue(path string, params func() *Params, unescape bool) (valu walk: // Outer loop for walking the tree for { prefix := n.path - - if len(path) > len(prefix) && path[:len(prefix)] == prefix { - path = path[len(prefix):] - // If this node does not have a wildcard (param or catchAll) - // child, we can just look up the next child node and continue - // to walk down the tree - if !n.wildChild { - idxc := path[0] - for i, c := range []byte(n.indices) { - if c == idxc { - n = n.children[i] - continue walk + if len(path) > len(prefix) { + if path[:len(prefix)] == prefix { + path = path[len(prefix):] + // If this node does not have a wildcard (param or catchAll) + // child, we can just look up the next child node and continue + // to walk down the tree + if !n.wildChild { + idxc := path[0] + for i, c := range []byte(n.indices) { + if c == idxc { + n = n.children[i] + continue walk + } } - } - - // Nothing found. - // We can recommend to redirect to the same URL without a - // trailing slash if a leaf exists for that path. - value.tsr = (path == "/" && n.handlers != nil) - return - } - // handle wildcard child - n = n.children[0] - switch n.nType { - case param: - // find param end (either '/' or path end) - end := 0 - for end < len(path) && path[end] != '/' { - end++ + // Nothing found. + // We can recommend to redirect to the same URL without a + // trailing slash if a leaf exists for that path. + value.tsr = (path == "/" && n.handlers != nil) + return } - // save param value - // if cap(value.params) < int(n.maxParams) { - // value.params = make(Params, 0, n.maxParams) - // } - // i := len(value.params) - // value.params = value.params[:i+1] // expand slice within preallocated capacity - // value.params[i].Key = n.path[1:] - // val := path[:end] - // if unescape { - // var err error - // if value.params[i].Value, err = url.QueryUnescape(val); err != nil { - // value.params[i].Value = val // fallback, in case of error - // } - // } else { - // value.params[i].Value = val - // } - if params != nil { - if value.params == nil { - value.params = params() + // handle wildcard child + n = n.children[0] + switch n.nType { + case param: + // find param end (either '/' or path end) + end := 0 + for end < len(path) && path[end] != '/' { + end++ } - // Expand slice within preallocated capacity - i := len(*value.params) - log.Println(i) - log.Println(*value.params) - *value.params = (*value.params)[:i+1] + + // save param value + // if cap(value.params) < int(n.maxParams) { + // value.params = make(Params, 0, n.maxParams) + // } + // i := len(value.params) + // value.params = value.params[:i+1] // expand slice within preallocated capacity + // value.params[i].Key = n.path[1:] // val := path[:end] // if unescape { - // if v, err := url.QueryUnescape(val); err == nil { - // val = v + // var err error + // if value.params[i].Value, err = url.QueryUnescape(val); err != nil { + // value.params[i].Value = val // fallback, in case of error // } + // } else { + // value.params[i].Value = val // } - (*value.params)[i] = Param{ - Key: n.path[1:], - Value: path, + if params != nil { + if value.params == nil { + value.params = params() + } + // Expand slice within preallocated capacity + i := len(*value.params) + *value.params = (*value.params)[:i+1] + // val := path[:end] + // if unescape { + // if v, err := url.QueryUnescape(val); err == nil { + // val = v + // } + // } + (*value.params)[i] = Param{ + Key: n.path[1:], + Value: path[:end], + } } - } - // we need to go deeper! - if end < len(path) { - if len(n.children) > 0 { - path = path[end:] - n = n.children[0] - continue walk - } + // we need to go deeper! + if end < len(path) { + if len(n.children) > 0 { + path = path[end:] + n = n.children[0] + continue walk + } - // ... but we can't - value.tsr = (len(path) == end+1) - return - } + // ... but we can't + value.tsr = (len(path) == end+1) + return + } - if value.handlers = n.handlers; value.handlers != nil { - // value.fullPath = n.fullPath + if value.handlers = n.handlers; value.handlers != nil { + // value.fullPath = n.fullPath + return + } + if len(n.children) == 1 { + // No handle found. Check if a handle for this path + a + // trailing slash exists for TSR recommendation + n = n.children[0] + value.tsr = (n.path == "/" && n.handlers != nil) + } return - } - if len(n.children) == 1 { - // No handle found. Check if a handle for this path + a - // trailing slash exists for TSR recommendation - n = n.children[0] - value.tsr = (n.path == "/" && n.handlers != nil) - } - return - - case catchAll: - // // save param value - // if cap(value.params) < int(n.maxParams) { - // value.params = make(Params, 0, n.maxParams) - // } - // i := len(value.params) - // value.params = value.params[:i+1] // expand slice within preallocated capacity - // value.params[i].Key = n.path[2:] - // if unescape { - // var err error - // if value.params[i].Value, err = url.QueryUnescape(path); err != nil { - // value.params[i].Value = path // fallback, in case of error - // } - // } else { - // value.params[i].Value = path - // } - // Save param value - if params != nil { - if value.params == nil { - value.params = params() - } - // Expand slice within preallocated capacity - i := len(*value.params) - *value.params = (*value.params)[:i+1] + case catchAll: + // // save param value + // if cap(value.params) < int(n.maxParams) { + // value.params = make(Params, 0, n.maxParams) + // } + // i := len(value.params) + // value.params = value.params[:i+1] // expand slice within preallocated capacity + // value.params[i].Key = n.path[2:] // if unescape { - // if v, err := url.QueryUnescape(path); err == nil { - // path = v + // var err error + // if value.params[i].Value, err = url.QueryUnescape(path); err != nil { + // value.params[i].Value = path // fallback, in case of error // } + // } else { + // value.params[i].Value = path // } - (*value.params)[i] = Param{ - Key: n.path[2:], - Value: path, + + // Save param value + if params != nil { + if value.params == nil { + value.params = params() + } + // Expand slice within preallocated capacity + i := len(*value.params) + *value.params = (*value.params)[:i+1] + // if unescape { + // if v, err := url.QueryUnescape(path); err == nil { + // path = v + // } + // } + (*value.params)[i] = Param{ + Key: n.path[2:], + Value: path, + } } - } - value.handlers = n.handlers - // value.fullPath = n.fullPath - return + value.handlers = n.handlers + // value.fullPath = n.fullPath + return - default: - panic("invalid node type") + default: + panic("invalid node type") + } } - } - - if path == prefix { + } else if path == prefix { // We should have reached the node containing the handle. // Check if this node has a handle registered. if value.handlers = n.handlers; value.handlers != nil { From 0a09a5bb561659b18987714aaa9a9f266355989e Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Fri, 8 May 2020 14:13:10 +0800 Subject: [PATCH 09/15] fix testing Signed-off-by: Bo-Yi Wu --- tree.go | 28 ++++++++++---------- tree_test.go | 72 ++++++++++++++++++++++++++++++---------------------- 2 files changed, 56 insertions(+), 44 deletions(-) diff --git a/tree.go b/tree.go index cd376c33ad..83d59dd41e 100644 --- a/tree.go +++ b/tree.go @@ -5,6 +5,7 @@ package gin import ( + "net/url" "strings" "unicode" "unicode/utf8" @@ -478,15 +479,15 @@ walk: // Outer loop for walking the tree // Expand slice within preallocated capacity i := len(*value.params) *value.params = (*value.params)[:i+1] - // val := path[:end] - // if unescape { - // if v, err := url.QueryUnescape(val); err == nil { - // val = v - // } - // } + val := path[:end] + if unescape { + if v, err := url.QueryUnescape(val); err == nil { + val = v + } + } (*value.params)[i] = Param{ Key: n.path[1:], - Value: path[:end], + Value: val, } } @@ -540,14 +541,15 @@ walk: // Outer loop for walking the tree // Expand slice within preallocated capacity i := len(*value.params) *value.params = (*value.params)[:i+1] - // if unescape { - // if v, err := url.QueryUnescape(path); err == nil { - // path = v - // } - // } + val := path + if unescape { + if v, err := url.QueryUnescape(path); err == nil { + val = v + } + } (*value.params)[i] = Param{ Key: n.path[2:], - Value: path, + Value: val, } } diff --git a/tree_test.go b/tree_test.go index 473a101199..dd7a18d634 100644 --- a/tree_test.go +++ b/tree_test.go @@ -28,6 +28,11 @@ type testRequests []struct { ps Params } +func getParams() *Params { + ps := make(Params, 0, 20) + return &ps +} + func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes ...bool) { unescape := false if len(unescapes) >= 1 { @@ -35,7 +40,7 @@ func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes .. } for _, request := range requests { - value := tree.getValue(request.path, nil, unescape) + value := tree.getValue(request.path, getParams, unescape) if value.handlers == nil { if !request.nilHandler { @@ -50,9 +55,12 @@ func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes .. } } - if !reflect.DeepEqual(value.params, request.ps) { - t.Errorf("Params mismatch for route '%s'", request.path) + if value.params != nil { + if !reflect.DeepEqual(*value.params, request.ps) { + t.Errorf("Params mismatch for route '%s'", request.path) + } } + } } @@ -76,27 +84,27 @@ func checkPriorities(t *testing.T, n *node) uint32 { return prio } -func checkMaxParams(t *testing.T, n *node) uint16 { - var maxParams uint16 - for i := range n.children { - params := checkMaxParams(t, n.children[i]) - if params > maxParams { - maxParams = params - } - } - if n.nType > root && !n.wildChild { - maxParams++ - } - - if n.maxParams != maxParams { - t.Errorf( - "maxParams mismatch for node '%s': is %d, should be %d", - n.path, n.maxParams, maxParams, - ) - } - - return maxParams -} +// func checkMaxParams(t *testing.T, n *node) uint16 { +// var maxParams uint16 +// for i := range n.children { +// params := checkMaxParams(t, n.children[i]) +// if params > maxParams { +// maxParams = params +// } +// } +// if n.nType > root && !n.wildChild { +// maxParams++ +// } + +// if n.maxParams != maxParams { +// t.Errorf( +// "maxParams mismatch for node '%s': is %d, should be %d", +// n.path, n.maxParams, maxParams, +// ) +// } + +// return maxParams +// } func TestCountParams(t *testing.T) { if countParams("/path/:param1/static/*catch-all") != uint16(2) { @@ -142,7 +150,7 @@ func TestTreeAddAndGet(t *testing.T) { }) checkPriorities(t, tree) - checkMaxParams(t, tree) + // checkMaxParams(t, tree) } func TestTreeWildcard(t *testing.T) { @@ -186,7 +194,7 @@ func TestTreeWildcard(t *testing.T) { }) checkPriorities(t, tree) - checkMaxParams(t, tree) + // checkMaxParams(t, tree) } func TestUnescapeParameters(t *testing.T) { @@ -224,7 +232,7 @@ func TestUnescapeParameters(t *testing.T) { }, unescape) checkPriorities(t, tree) - checkMaxParams(t, tree) + // checkMaxParams(t, tree) } func catchPanic(testFunc func()) (recv interface{}) { @@ -323,12 +331,14 @@ func TestTreeDupliatePath(t *testing.T) { } } + //printChildren(tree, "") + checkRequests(t, tree, testRequests{ {"/", false, "/", nil}, {"/doc/", false, "/doc/", nil}, - {"/src/some/file.png", false, "/src/*filepath", Params{Param{Key: "filepath", Value: "/some/file.png"}}}, - {"/search/someth!ng+in+ünìcodé", false, "/search/:query", Params{Param{Key: "query", Value: "someth!ng+in+ünìcodé"}}}, - {"/user_gopher", false, "/user_:name", Params{Param{Key: "name", Value: "gopher"}}}, + {"/src/some/file.png", false, "/src/*filepath", Params{Param{"filepath", "/some/file.png"}}}, + {"/search/someth!ng+in+ünìcodé", false, "/search/:query", Params{Param{"query", "someth!ng+in+ünìcodé"}}}, + {"/user_gopher", false, "/user_:name", Params{Param{"name", "gopher"}}}, }) } @@ -372,7 +382,7 @@ func TestTreeCatchMaxParams(t *testing.T) { tree := &node{} var route = "/cmd/*filepath" tree.addRoute(route, fakeHandler(route)) - checkMaxParams(t, tree) + // checkMaxParams(t, tree) } func TestTreeDoubleWildcard(t *testing.T) { From 7035cc2dd775d5d3cdc43efa961f1f1955820c10 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 9 May 2020 00:46:47 +0800 Subject: [PATCH 10/15] refactor gin context Signed-off-by: Bo-Yi Wu --- context.go | 2 ++ gin.go | 53 ++++++++++++++++++++++++++-------------------------- tree.go | 6 +++--- tree_test.go | 2 +- 4 files changed, 32 insertions(+), 31 deletions(-) diff --git a/context.go b/context.go index 4ebcc29411..2ea1a02869 100644 --- a/context.go +++ b/context.go @@ -54,6 +54,7 @@ type Context struct { fullPath string engine *Engine + params *Params // This mutex protect Keys map mu sync.RWMutex @@ -95,6 +96,7 @@ func (c *Context) reset() { c.Accepted = nil c.queryCache = nil c.formCache = nil + *c.params = (*c.params)[0:0] } // Copy returns a copy of the current context that can be safely used outside the request's scope. diff --git a/gin.go b/gin.go index ce12219e05..30fa5ac536 100644 --- a/gin.go +++ b/gin.go @@ -113,8 +113,8 @@ type Engine struct { noMethod HandlersChain pool sync.Pool trees methodTrees - paramsPool sync.Pool - maxParams uint16 + // paramsPool sync.Pool + maxParams uint16 } var _ IRouter = &Engine{} @@ -165,7 +165,8 @@ func Default() *Engine { } func (engine *Engine) allocateContext() *Context { - return &Context{engine: engine} + v := make(Params, 0, engine.maxParams) + return &Context{engine: engine, params: &v} } // Delims sets template left and right delims and returns a Engine instance. @@ -276,13 +277,13 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { } // Lazy-init paramsPool alloc func - if engine.paramsPool.New == nil && engine.maxParams > 0 { - engine.paramsPool.New = func() interface{} { - // log.Println("engine.maxParams:", engine.maxParams) - ps := make(Params, 0, engine.maxParams) - return &ps - } - } + // if engine.paramsPool.New == nil && engine.maxParams > 0 { + // engine.paramsPool.New = func() interface{} { + // // log.Println("engine.maxParams:", engine.maxParams) + // ps := make(Params, 0, engine.maxParams) + // return &ps + // } + // } } // Routes returns a slice of registered routes, including some useful information, such as: @@ -401,22 +402,20 @@ func (engine *Engine) HandleContext(c *Context) { c.index = oldIndexValue } -func (engine *Engine) getParams() *Params { - // c := engine.pool.Get().(*Context) - // c.Params = c.Params[0:0] - ps := engine.paramsPool.Get().(*Params) - *ps = (*ps)[0:0] // reset slice - return ps -} +// func (engine *Engine) getParams() *Params { +// ps := engine.paramsPool.Get().(*Params) +// *ps = (*ps)[0:0] // reset slice +// return ps +// } -func (engine *Engine) putParams(ps *Params) { - // c := engine.pool.Get().(*Context) - // c.Params = *ps - // engine.pool.Put(c) - if ps != nil { - engine.paramsPool.Put(ps) - } -} +// func (engine *Engine) putParams(ps *Params) { +// c := engine.pool.Get().(*Context) +// c.Params = *ps +// engine.pool.Put(c) +// if ps != nil { +// engine.paramsPool.Put(ps) +// } +// } func (engine *Engine) handleHTTPRequest(c *Context) { httpMethod := c.Request.Method @@ -439,9 +438,9 @@ func (engine *Engine) handleHTTPRequest(c *Context) { } root := t[i].root // Find route in tree - value := root.getValue(rPath, engine.getParams, unescape) + value := root.getValue(rPath, c.params, unescape) if value.params != nil { - engine.putParams(value.params) + // engine.putParams(value.params) c.Params = *value.params } if value.handlers != nil { diff --git a/tree.go b/tree.go index 83d59dd41e..74481be815 100644 --- a/tree.go +++ b/tree.go @@ -419,7 +419,7 @@ type nodeValue struct { // 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, unescape bool) (value nodeValue) { +func (n *node) getValue(path string, params *Params, unescape bool) (value nodeValue) { value = nodeValue{} walk: // Outer loop for walking the tree for { @@ -474,7 +474,7 @@ walk: // Outer loop for walking the tree // } if params != nil { if value.params == nil { - value.params = params() + value.params = params } // Expand slice within preallocated capacity i := len(*value.params) @@ -536,7 +536,7 @@ walk: // Outer loop for walking the tree // Save param value if params != nil { if value.params == nil { - value.params = params() + value.params = params } // Expand slice within preallocated capacity i := len(*value.params) diff --git a/tree_test.go b/tree_test.go index dd7a18d634..a1bd223986 100644 --- a/tree_test.go +++ b/tree_test.go @@ -40,7 +40,7 @@ func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes .. } for _, request := range requests { - value := tree.getValue(request.path, getParams, unescape) + value := tree.getValue(request.path, getParams(), unescape) if value.handlers == nil { if !request.nilHandler { From 513bc5ef430449e715b009c2244e97d8ceb6c6c3 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 9 May 2020 01:14:07 +0800 Subject: [PATCH 11/15] add fullPath Signed-off-by: Bo-Yi Wu --- gin.go | 4 +-- routes_test.go | 78 +++++++++++++++++++++++++------------------------- tree.go | 34 +++++++++++----------- 3 files changed, 58 insertions(+), 58 deletions(-) diff --git a/gin.go b/gin.go index 30fa5ac536..159646a4ec 100644 --- a/gin.go +++ b/gin.go @@ -265,7 +265,7 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { root := engine.trees.get(method) if root == nil { root = new(node) - // root.fullPath = "/" + root.fullPath = "/" engine.trees = append(engine.trees, methodTree{method: method, root: root}) } root.addRoute(path, handlers) @@ -448,7 +448,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) { // if value.params != nil { // c.Params = *value.params // } - // c.fullPath = value.fullPath + c.fullPath = value.fullPath c.Next() c.writermem.WriteHeaderNow() return diff --git a/routes_test.go b/routes_test.go index d6c513379f..360ade6215 100644 --- a/routes_test.go +++ b/routes_test.go @@ -579,42 +579,42 @@ func TestRouteServeErrorWithWriteHeader(t *testing.T) { assert.Equal(t, 0, w.Body.Len()) } -// func TestRouteContextHoldsFullPath(t *testing.T) { -// router := New() - -// // Test routes -// routes := []string{ -// "/simple", -// "/project/:name", -// "/", -// "/news/home", -// "/news", -// "/simple-two/one", -// "/simple-two/one-two", -// "/project/:name/build/*params", -// "/project/:name/bui", -// } - -// for _, route := range routes { -// actualRoute := route -// router.GET(route, func(c *Context) { -// // For each defined route context should contain its full path -// assert.Equal(t, actualRoute, c.FullPath()) -// c.AbortWithStatus(http.StatusOK) -// }) -// } - -// for _, route := range routes { -// w := performRequest(router, http.MethodGet, route) -// assert.Equal(t, http.StatusOK, w.Code) -// } - -// // Test not found -// router.Use(func(c *Context) { -// // For not found routes full path is empty -// assert.Equal(t, "", c.FullPath()) -// }) - -// w := performRequest(router, http.MethodGet, "/not-found") -// assert.Equal(t, http.StatusNotFound, w.Code) -// } +func TestRouteContextHoldsFullPath(t *testing.T) { + router := New() + + // Test routes + routes := []string{ + "/simple", + "/project/:name", + "/", + "/news/home", + "/news", + "/simple-two/one", + "/simple-two/one-two", + "/project/:name/build/*params", + "/project/:name/bui", + } + + for _, route := range routes { + actualRoute := route + router.GET(route, func(c *Context) { + // For each defined route context should contain its full path + assert.Equal(t, actualRoute, c.FullPath()) + c.AbortWithStatus(http.StatusOK) + }) + } + + for _, route := range routes { + w := performRequest(router, http.MethodGet, route) + assert.Equal(t, http.StatusOK, w.Code) + } + + // Test not found + router.Use(func(c *Context) { + // For not found routes full path is empty + assert.Equal(t, "", c.FullPath()) + }) + + w := performRequest(router, http.MethodGet, "/not-found") + assert.Equal(t, http.StatusNotFound, w.Code) +} diff --git a/tree.go b/tree.go index 74481be815..0a6ad013d4 100644 --- a/tree.go +++ b/tree.go @@ -101,7 +101,7 @@ type node struct { nType nodeType maxParams uint16 wildChild bool - // fullPath string + fullPath string } // increments priority of the given child and reorders if necessary. @@ -141,7 +141,7 @@ func (n *node) addRoute(path string, handlers HandlersChain) { return } - // parentFullPathIndex := 0 + parentFullPathIndex := 0 walk: for { @@ -164,7 +164,7 @@ walk: children: n.children, handlers: n.handlers, priority: n.priority - 1, - // fullPath: n.fullPath, + fullPath: n.fullPath, } // Update maxParams (max of all children) @@ -180,7 +180,7 @@ walk: n.path = path[:i] n.handlers = nil n.wildChild = false - // n.fullPath = fullPath[:parentFullPathIndex+i] + n.fullPath = fullPath[:parentFullPathIndex+i] } // Make new node a child of this node @@ -188,7 +188,7 @@ walk: path = path[i:] if n.wildChild { - // parentFullPathIndex += len(n.path) + parentFullPathIndex += len(n.path) n = n.children[0] n.priority++ @@ -222,7 +222,7 @@ walk: // slash after param if n.nType == param && c == '/' && len(n.children) == 1 { - // parentFullPathIndex += len(n.path) + parentFullPathIndex += len(n.path) n = n.children[0] n.priority++ continue walk @@ -231,7 +231,7 @@ walk: // Check if a child with the next path byte exists for i, max := 0, len(n.indices); i < max; i++ { if c == n.indices[i] { - // parentFullPathIndex += len(n.path) + parentFullPathIndex += len(n.path) i = n.incrementChildPrio(i) n = n.children[i] continue walk @@ -244,7 +244,7 @@ walk: n.indices += string([]byte{c}) child := &node{ // maxParams: numParams, - // fullPath: fullPath, + fullPath: fullPath, } n.children = append(n.children, child) n.incrementChildPrio(len(n.indices) - 1) @@ -326,7 +326,7 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) nType: param, path: wildcard, // maxParams: numParams, - // fullPath: fullPath, + fullPath: fullPath, } n.children = []*node{child} n = child @@ -341,7 +341,7 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) child := &node{ // maxParams: numParams, priority: 1, - // fullPath: fullPath, + fullPath: fullPath, } n.children = []*node{child} n = child @@ -375,7 +375,7 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) wildChild: true, nType: catchAll, // maxParams: 1, - // fullPath: fullPath, + fullPath: fullPath, } // update maxParams of the parent node // if n.maxParams < 1 { @@ -393,7 +393,7 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) handlers: handlers, priority: 1, // maxParams: 1, - // fullPath: fullPath, + fullPath: fullPath, } n.children = []*node{child} @@ -403,7 +403,7 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) // If no wildcard was found, simply insert the path and handle n.path = path n.handlers = handlers - // n.fullPath = fullPath + n.fullPath = fullPath } // nodeValue holds return values of (*Node).getValue method @@ -411,7 +411,7 @@ type nodeValue struct { handlers HandlersChain params *Params tsr bool - // fullPath string + fullPath string } // getValue returns the handle registered with the given path (key). The values of @@ -505,7 +505,7 @@ walk: // Outer loop for walking the tree } if value.handlers = n.handlers; value.handlers != nil { - // value.fullPath = n.fullPath + value.fullPath = n.fullPath return } if len(n.children) == 1 { @@ -554,7 +554,7 @@ walk: // Outer loop for walking the tree } value.handlers = n.handlers - // value.fullPath = n.fullPath + value.fullPath = n.fullPath return default: @@ -565,7 +565,7 @@ walk: // Outer loop for walking the tree // We should have reached the node containing the handle. // Check if this node has a handle registered. if value.handlers = n.handlers; value.handlers != nil { - // value.fullPath = n.fullPath + value.fullPath = n.fullPath return } From c3bf4923de957ed4fcd667f3bebd7e3ea754b4f3 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 9 May 2020 19:57:08 +0800 Subject: [PATCH 12/15] chore: refactor --- gin.go | 33 +------- tree.go | 222 ++++++--------------------------------------------- tree_test.go | 68 ++++++++-------- 3 files changed, 63 insertions(+), 260 deletions(-) diff --git a/gin.go b/gin.go index 159646a4ec..7175061c7a 100644 --- a/gin.go +++ b/gin.go @@ -113,8 +113,7 @@ type Engine struct { noMethod HandlersChain pool sync.Pool trees methodTrees - // paramsPool sync.Pool - maxParams uint16 + maxParams uint16 } var _ IRouter = &Engine{} @@ -254,7 +253,6 @@ func (engine *Engine) rebuild405Handlers() { } func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { - // log.Println(method, path) assert1(path[0] == '/', "path must begin with '/'") assert1(method != "", "HTTP method can not be empty") assert1(len(handlers) > 0, "there must be at least one handler") @@ -271,19 +269,9 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { root.addRoute(path, handlers) // Update maxParams - // log.Println("countParams(path):", countParams(path)) if paramsCount := countParams(path); paramsCount+varsCount > engine.maxParams { engine.maxParams = paramsCount + varsCount } - - // Lazy-init paramsPool alloc func - // if engine.paramsPool.New == nil && engine.maxParams > 0 { - // engine.paramsPool.New = func() interface{} { - // // log.Println("engine.maxParams:", engine.maxParams) - // ps := make(Params, 0, engine.maxParams) - // return &ps - // } - // } } // Routes returns a slice of registered routes, including some useful information, such as: @@ -402,21 +390,6 @@ func (engine *Engine) HandleContext(c *Context) { c.index = oldIndexValue } -// func (engine *Engine) getParams() *Params { -// ps := engine.paramsPool.Get().(*Params) -// *ps = (*ps)[0:0] // reset slice -// return ps -// } - -// func (engine *Engine) putParams(ps *Params) { -// c := engine.pool.Get().(*Context) -// c.Params = *ps -// engine.pool.Put(c) -// if ps != nil { -// engine.paramsPool.Put(ps) -// } -// } - func (engine *Engine) handleHTTPRequest(c *Context) { httpMethod := c.Request.Method rPath := c.Request.URL.Path @@ -440,14 +413,10 @@ func (engine *Engine) handleHTTPRequest(c *Context) { // Find route in tree value := root.getValue(rPath, c.params, unescape) if value.params != nil { - // engine.putParams(value.params) c.Params = *value.params } if value.handlers != nil { c.handlers = value.handlers - // if value.params != nil { - // c.Params = *value.params - // } c.fullPath = value.fullPath c.Next() c.writermem.WriteHeaderNow() diff --git a/tree.go b/tree.go index 0a6ad013d4..8f33390c33 100644 --- a/tree.go +++ b/tree.go @@ -95,16 +95,15 @@ const ( type node struct { path string indices string + wildChild bool + nType nodeType + priority uint32 children []*node handlers HandlersChain - priority uint32 - nType nodeType - maxParams uint16 - wildChild bool fullPath string } -// increments priority of the given child and reorders if necessary. +// Increments priority of the given child and reorders if necessary func (n *node) incrementChildPrio(pos int) int { cs := n.children cs[pos].priority++ @@ -115,13 +114,14 @@ func (n *node) incrementChildPrio(pos int) int { for ; newPos > 0 && cs[newPos-1].priority < prio; newPos-- { // Swap node positions cs[newPos-1], cs[newPos] = cs[newPos], cs[newPos-1] + } - // build new index char string + // Build new index char string if newPos != pos { - n.indices = n.indices[:newPos] + // unchanged prefix, might be empty - n.indices[pos:pos+1] + // the index char we move - n.indices[newPos:pos] + n.indices[pos+1:] // rest without char at 'pos' + n.indices = n.indices[:newPos] + // Unchanged prefix, might be empty + n.indices[pos:pos+1] + // The index char we move + n.indices[newPos:pos] + n.indices[pos+1:] // Rest without char at 'pos' } return newPos @@ -132,7 +132,6 @@ func (n *node) incrementChildPrio(pos int) int { func (n *node) addRoute(path string, handlers HandlersChain) { fullPath := path n.priority++ - // numParams := countParams(path) // Empty tree if len(n.path) == 0 && len(n.children) == 0 { @@ -145,11 +144,6 @@ func (n *node) addRoute(path string, handlers HandlersChain) { walk: for { - // Update maxParams of the current node - // if numParams > n.maxParams { - // n.maxParams = numParams - // } - // Find the longest common prefix. // This also implies that the common prefix contains no ':' or '*' // since the existing key can't contain those chars. @@ -167,13 +161,6 @@ walk: fullPath: n.fullPath, } - // Update maxParams (max of all children) - // for _, v := range child.children { - // if v.maxParams > child.maxParams { - // child.maxParams = v.maxParams - // } - // } - n.children = []*node{&child} // []byte for proper unicode char conversion, see #65 n.indices = string([]byte{n.path[i]}) @@ -192,18 +179,13 @@ walk: n = n.children[0] n.priority++ - // Update maxParams of the child node - // if numParams > n.maxParams { - // n.maxParams = numParams - // } - // numParams-- - // Check if the wildcard matches - if len(path) >= len(n.path) && n.path == path[:len(n.path)] { - // check for longer wildcard, e.g. :name and :names - if len(n.path) >= len(path) || path[len(n.path)] == '/' { - continue walk - } + if len(path) >= len(n.path) && n.path == path[:len(n.path)] && + // Adding a child to a catchAll is not possible + n.nType != catchAll && + // Check for longer wildcard, e.g. :name and :names + (len(n.path) >= len(path) || path[len(n.path)] == '/') { + continue walk } pathSeg := path @@ -243,7 +225,6 @@ walk: // []byte for proper unicode char conversion, see #65 n.indices += string([]byte{c}) child := &node{ - // maxParams: numParams, fullPath: fullPath, } n.children = append(n.children, child) @@ -323,9 +304,8 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) n.wildChild = true child := &node{ - nType: param, - path: wildcard, - // maxParams: numParams, + nType: param, + path: wildcard, fullPath: fullPath, } n.children = []*node{child} @@ -339,7 +319,6 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) path = path[len(wildcard):] child := &node{ - // maxParams: numParams, priority: 1, fullPath: fullPath, } @@ -374,13 +353,9 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) child := &node{ wildChild: true, nType: catchAll, - // maxParams: 1, - fullPath: fullPath, + fullPath: fullPath, } - // update maxParams of the parent node - // if n.maxParams < 1 { - // n.maxParams = 1 - // } + n.children = []*node{child} n.indices = string('/') n = child @@ -392,7 +367,6 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) nType: catchAll, handlers: handlers, priority: 1, - // maxParams: 1, fullPath: fullPath, } n.children = []*node{child} @@ -414,13 +388,12 @@ type nodeValue struct { fullPath string } -// getValue returns the handle registered with the given path (key). The values of +// Returns the handle registered with the given path (key). The values of // wildcards are saved to a map. // 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 *Params, unescape bool) (value nodeValue) { - value = nodeValue{} walk: // Outer loop for walking the tree for { prefix := n.path @@ -446,32 +419,17 @@ walk: // Outer loop for walking the tree return } - // handle wildcard child + // Handle wildcard child n = n.children[0] switch n.nType { case param: - // find param end (either '/' or path end) + // Find param end (either '/' or path end) end := 0 for end < len(path) && path[end] != '/' { end++ } - // save param value - // if cap(value.params) < int(n.maxParams) { - // value.params = make(Params, 0, n.maxParams) - // } - // i := len(value.params) - // value.params = value.params[:i+1] // expand slice within preallocated capacity - // value.params[i].Key = n.path[1:] - // val := path[:end] - // if unescape { - // var err error - // if value.params[i].Value, err = url.QueryUnescape(val); err != nil { - // value.params[i].Value = val // fallback, in case of error - // } - // } else { - // value.params[i].Value = val - // } + // Save param value if params != nil { if value.params == nil { value.params = params @@ -517,22 +475,6 @@ walk: // Outer loop for walking the tree return case catchAll: - // // save param value - // if cap(value.params) < int(n.maxParams) { - // value.params = make(Params, 0, n.maxParams) - // } - // i := len(value.params) - // value.params = value.params[:i+1] // expand slice within preallocated capacity - // value.params[i].Key = n.path[2:] - // if unescape { - // var err error - // if value.params[i].Value, err = url.QueryUnescape(path); err != nil { - // value.params[i].Value = path // fallback, in case of error - // } - // } else { - // value.params[i].Value = path - // } - // Save param value if params != nil { if value.params == nil { @@ -561,7 +503,9 @@ walk: // Outer loop for walking the tree panic("invalid node type") } } - } else if path == prefix { + } + + if path == prefix { // We should have reached the node containing the handle. // Check if this node has a handle registered. if value.handlers = n.handlers; value.handlers != nil { @@ -622,120 +566,6 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) ([]by ) return ciPath, ciPath != nil - // ciPath = make([]byte, 0, len(path)+1) // preallocate enough memory - - // // Outer loop for walking the tree - // for len(path) >= len(n.path) && strings.EqualFold(path[:len(n.path)], n.path) { - // path = path[len(n.path):] - // ciPath = append(ciPath, n.path...) - - // if len(path) == 0 { - // // We should have reached the node containing the handle. - // // Check if this node has a handle registered. - // if n.handlers != nil { - // return ciPath, true - // } - - // // No handle found. - // // Try to fix the path by adding a trailing slash - // if fixTrailingSlash { - // for i := 0; i < len(n.indices); i++ { - // if n.indices[i] == '/' { - // n = n.children[i] - // if (len(n.path) == 1 && n.handlers != nil) || - // (n.nType == catchAll && n.children[0].handlers != nil) { - // return append(ciPath, '/'), true - // } - // return - // } - // } - // } - // return - // } - - // // If this node does not have a wildcard (param or catchAll) child, - // // we can just look up the next child node and continue to walk down - // // the tree - // if !n.wildChild { - // r := unicode.ToLower(rune(path[0])) - // for i, index := range n.indices { - // // must use recursive approach since both index and - // // ToLower(index) could exist. We must check both. - // if r == unicode.ToLower(index) { - // out, found := n.children[i].findCaseInsensitivePath(path, fixTrailingSlash) - // if found { - // return append(ciPath, out...), true - // } - // } - // } - - // // Nothing found. We can recommend to redirect to the same URL - // // without a trailing slash if a leaf exists for that path - // found = fixTrailingSlash && path == "/" && n.handlers != nil - // return - // } - - // n = n.children[0] - // switch n.nType { - // case param: - // // Find param end (either '/' or path end) - // end := 0 - // for end < len(path) && path[end] != '/' { - // end++ - // } - - // // add param value to case insensitive path - // ciPath = append(ciPath, path[:end]...) - - // // we need to go deeper! - // if end < len(path) { - // if len(n.children) > 0 { - // path = path[end:] - // n = n.children[0] - // continue - // } - - // // ... but we can't - // if fixTrailingSlash && len(path) == end+1 { - // return ciPath, true - // } - // return - // } - - // if n.handlers != nil { - // return ciPath, true - // } - // if fixTrailingSlash && len(n.children) == 1 { - // // No handle found. Check if a handle for this path + a - // // trailing slash exists - // n = n.children[0] - // if n.path == "/" && n.handlers != nil { - // return append(ciPath, '/'), true - // } - // } - // return - - // case catchAll: - // return append(ciPath, path...), true - - // default: - // panic("invalid node type") - // } - // } - - // // Nothing found. - // // Try to fix the path by adding / removing a trailing slash - // if fixTrailingSlash { - // if path == "/" { - // return ciPath, true - // } - // if len(path)+1 == len(n.path) && n.path[len(path)] == '/' && - // strings.EqualFold(path, n.path[:len(path)]) && - // n.handlers != nil { - // return append(ciPath, n.path...), true - // } - // } - // return } // Shift bytes in array by n bytes left diff --git a/tree_test.go b/tree_test.go index a1bd223986..1cb4f5598d 100644 --- a/tree_test.go +++ b/tree_test.go @@ -84,35 +84,13 @@ func checkPriorities(t *testing.T, n *node) uint32 { return prio } -// func checkMaxParams(t *testing.T, n *node) uint16 { -// var maxParams uint16 -// for i := range n.children { -// params := checkMaxParams(t, n.children[i]) -// if params > maxParams { -// maxParams = params -// } -// } -// if n.nType > root && !n.wildChild { -// maxParams++ -// } - -// if n.maxParams != maxParams { -// t.Errorf( -// "maxParams mismatch for node '%s': is %d, should be %d", -// n.path, n.maxParams, maxParams, -// ) -// } - -// return maxParams -// } - func TestCountParams(t *testing.T) { - if countParams("/path/:param1/static/*catch-all") != uint16(2) { + if countParams("/path/:param1/static/*catch-all") != 2 { + t.Fail() + } + if countParams(strings.Repeat("/:param", 256)) != 256 { t.Fail() } - // if countParams(strings.Repeat("/:param", 256)) != 255 { - // t.Fail() - // } } func TestTreeAddAndGet(t *testing.T) { @@ -150,7 +128,6 @@ func TestTreeAddAndGet(t *testing.T) { }) checkPriorities(t, tree) - // checkMaxParams(t, tree) } func TestTreeWildcard(t *testing.T) { @@ -194,7 +171,6 @@ func TestTreeWildcard(t *testing.T) { }) checkPriorities(t, tree) - // checkMaxParams(t, tree) } func TestUnescapeParameters(t *testing.T) { @@ -232,7 +208,6 @@ func TestUnescapeParameters(t *testing.T) { }, unescape) checkPriorities(t, tree) - // checkMaxParams(t, tree) } func catchPanic(testFunc func()) (recv interface{}) { @@ -366,6 +341,8 @@ func TestTreeCatchAllConflict(t *testing.T) { {"/src/*filepath/x", true}, {"/src2/", false}, {"/src2/*filepath/x", true}, + {"/src3/*filepath", false}, + {"/src3/*filepath/x", true}, } testRoutes(t, routes) } @@ -382,7 +359,6 @@ func TestTreeCatchMaxParams(t *testing.T) { tree := &node{} var route = "/cmd/*filepath" tree.addRoute(route, fakeHandler(route)) - // checkMaxParams(t, tree) } func TestTreeDoubleWildcard(t *testing.T) { @@ -518,6 +494,9 @@ func TestTreeRootTrailingSlashRedirect(t *testing.T) { func TestTreeFindCaseInsensitivePath(t *testing.T) { tree := &node{} + longPath := "/l" + strings.Repeat("o", 128) + "ng" + lOngPath := "/l" + strings.Repeat("O", 128) + "ng/" + routes := [...]string{ "/hi", "/b/", @@ -541,6 +520,17 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) { "/doc/go/away", "/no/a", "/no/b", + "/Π", + "/u/apfêl/", + "/u/äpfêl/", + "/u/öpfêl", + "/v/Äpfêl/", + "/v/Öpfêl", + "/w/♬", // 3 byte + "/w/♭/", // 3 byte, last byte differs + "/w/𠜎", // 4 byte + "/w/𠜏/", // 4 byte + longPath, } for _, route := range routes { @@ -619,6 +609,21 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) { {"/DOC/", "/doc", true, true}, {"/NO", "", false, true}, {"/DOC/GO", "", false, true}, + {"/π", "/Π", true, false}, + {"/π/", "/Π", true, true}, + {"/u/ÄPFÊL/", "/u/äpfêl/", true, false}, + {"/u/ÄPFÊL", "/u/äpfêl/", true, true}, + {"/u/ÖPFÊL/", "/u/öpfêl", true, true}, + {"/u/ÖPFÊL", "/u/öpfêl", true, false}, + {"/v/äpfêL/", "/v/Äpfêl/", true, false}, + {"/v/äpfêL", "/v/Äpfêl/", true, true}, + {"/v/öpfêL/", "/v/Öpfêl", true, true}, + {"/v/öpfêL", "/v/Öpfêl", true, false}, + {"/w/♬/", "/w/♬", true, true}, + {"/w/♭", "/w/♭/", true, true}, + {"/w/𠜎/", "/w/𠜎", true, true}, + {"/w/𠜏", "/w/𠜏/", true, true}, + {lOngPath, longPath, true, true}, } // With fixTrailingSlash = true for _, test := range tests { @@ -706,8 +711,7 @@ func TestTreeWildcardConflictEx(t *testing.T) { tree.addRoute(conflict.route, fakeHandler(conflict.route)) }) - if !regexp.MustCompile(fmt.Sprintf("'%s' in new path .* conflicts with existing wildcard '%s' in existing prefix '%s'", - conflict.segPath, conflict.existSegPath, conflict.existPath)).MatchString(fmt.Sprint(recv)) { + if !regexp.MustCompile(fmt.Sprintf("'%s' in new path .* conflicts with existing wildcard '%s' in existing prefix '%s'", conflict.segPath, conflict.existSegPath, conflict.existPath)).MatchString(fmt.Sprint(recv)) { t.Fatalf("invalid wildcard conflict error (%v)", recv) } } From 450ff5e15316b7ef2f79d6dfcd7399b5cb1b78a0 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sun, 10 May 2020 10:56:20 +0800 Subject: [PATCH 13/15] remove unused code Signed-off-by: Bo-Yi Wu --- tree.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tree.go b/tree.go index 8f33390c33..ace1c33a0f 100644 --- a/tree.go +++ b/tree.go @@ -311,7 +311,6 @@ func (n *node) insertChild(path string, fullPath string, handlers HandlersChain) n.children = []*node{child} n = child n.priority++ - // numParams-- // if the path doesn't end with the wildcard, then there // will be another non-wildcard subpath starting with '/' From 9b0fe9690a10e5fcc2fcb7d326ea8ca28ba5cd21 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sun, 10 May 2020 12:02:35 +0800 Subject: [PATCH 14/15] remove varsCount Signed-off-by: Bo-Yi Wu --- gin.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gin.go b/gin.go index 7175061c7a..1e12617904 100644 --- a/gin.go +++ b/gin.go @@ -258,7 +258,6 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { assert1(len(handlers) > 0, "there must be at least one handler") debugPrintRoute(method, path, handlers) - varsCount := uint16(0) root := engine.trees.get(method) if root == nil { @@ -269,8 +268,8 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { root.addRoute(path, handlers) // Update maxParams - if paramsCount := countParams(path); paramsCount+varsCount > engine.maxParams { - engine.maxParams = paramsCount + varsCount + if paramsCount := countParams(path); paramsCount > engine.maxParams { + engine.maxParams = paramsCount } } From e0c3f7978b54f3cf805d885f69a3aa1942a819ac Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sun, 10 May 2020 12:05:39 +0800 Subject: [PATCH 15/15] refactor Signed-off-by: Bo-Yi Wu --- tree.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tree.go b/tree.go index ace1c33a0f..e3aa9190a3 100644 --- a/tree.go +++ b/tree.go @@ -710,7 +710,9 @@ walk: // Outer loop for walking the tree if n.handlers != nil { return ciPath - } else if fixTrailingSlash && len(n.children) == 1 { + } + + if fixTrailingSlash && len(n.children) == 1 { // No handle found. Check if a handle for this path + a // trailing slash exists n = n.children[0] @@ -718,6 +720,7 @@ walk: // Outer loop for walking the tree return append(ciPath, '/') } } + return nil case catchAll: