Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some loop don't need to transfer to bytes slice and some use the zero… #2572

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions internal/bytesconv/bytesconv.go
Expand Up @@ -5,16 +5,17 @@
package bytesconv

import (
"reflect"
"unsafe"
)

// StringToBytes converts string to byte slice without a memory allocation.
func StringToBytes(s string) (b []byte) {
sh := *(*reflect.StringHeader)(unsafe.Pointer(&s))
bh := (*reflect.SliceHeader)(unsafe.Pointer(&b))
bh.Data, bh.Len, bh.Cap = sh.Data, sh.Len, sh.Len
return b
func StringToBytes(s string) []byte {
Copy link
Contributor

@panjf2000 panjf2000 Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the theorem behind this change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@panjf2000 based on #2553, we also need to change it.

return *(*[]byte)(unsafe.Pointer(
&struct {
string
Cap int
}{s, len(s)},
))
}

// BytesToString converts byte slice to string without a memory allocation.
Expand Down
18 changes: 8 additions & 10 deletions tree.go
Expand Up @@ -205,10 +205,8 @@ walk:
"'")
}

c := path[0]

// slash after param
if n.nType == param && c == '/' && len(n.children) == 1 {
if n.nType == param && path[0] == '/' && len(n.children) == 1 {
parentFullPathIndex += len(n.path)
n = n.children[0]
n.priority++
Expand All @@ -217,7 +215,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] {
if path[0] == n.indices[i] {
parentFullPathIndex += len(n.path)
i = n.incrementChildPrio(i)
n = n.children[i]
Expand All @@ -226,9 +224,9 @@ walk:
}

// Otherwise insert it
if c != ':' && c != '*' {
if path[0] != ':' && path[0] != '*' {
// []byte for proper unicode char conversion, see #65
n.indices += bytesconv.BytesToString([]byte{c})
n.indices += path[0:1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here maybe we should add it, thanks!

child := &node{
fullPath: fullPath,
}
Expand All @@ -254,15 +252,15 @@ walk:
// 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) {
for start, c := range path {
// A wildcard starts with ':' (param) or '*' (catch-all)
if c != ':' && c != '*' {
continue
}

// Find end and check for invalid characters
valid = true
for end, c := range []byte(path[start+1:]) {
for end, c := range path[start+1:] {
switch c {
case '/':
return path[start : start+1+end], start, valid
Expand Down Expand Up @@ -528,7 +526,7 @@ 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
for i, c := range []byte(n.indices) {
for i, c := range n.indices {
if c == '/' {
n = n.children[i]
value.tsr = (len(n.path) == 1 && n.handlers != nil) ||
Expand Down Expand Up @@ -745,7 +743,7 @@ walk: // Outer loop for walking the tree
// No handle found.
// Try to fix the path by adding a trailing slash
if fixTrailingSlash {
for i, c := range []byte(n.indices) {
for i, c := range n.indices {
if c == '/' {
n = n.children[i]
if (len(n.path) == 1 && n.handlers != nil) ||
Expand Down