Skip to content

Commit

Permalink
Reuse bytes when cleaning the URL paths (gin-gonic#2179)
Browse files Browse the repository at this point in the history
* path: use stack buffer in CleanPath to avoid allocs in common case

Sync from julienschmidt/httprouter@8222db1

* path: sync test code from httprouter

* path: update path_test.go to the latest code

Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
  • Loading branch information
2 people authored and byebyebruce committed Mar 25, 2020
1 parent f926071 commit 94c85f4
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 15 deletions.
33 changes: 25 additions & 8 deletions path.go
Expand Up @@ -5,6 +5,8 @@

package gin

const stackBufSize = 128

// cleanPath is the URL version of path.Clean, it returns a canonical URL path
// for p, eliminating . and .. elements.
//
Expand All @@ -24,8 +26,11 @@ func cleanPath(p string) string {
return "/"
}

// Reasonably sized buffer on stack to avoid allocations in the common case.
// If a larger buffer is required, it gets allocated dynamically.
buf := make([]byte, 0, stackBufSize)

n := len(p)
var buf []byte

// Invariants:
// reading from path; r is index of next byte to process.
Expand All @@ -37,7 +42,12 @@ func cleanPath(p string) string {

if p[0] != '/' {
r = 0
buf = make([]byte, n+1)

if n+1 > stackBufSize {
buf = make([]byte, n+1)
} else {
buf = buf[:n+1]
}
buf[0] = '/'
}

Expand Down Expand Up @@ -69,7 +79,7 @@ func cleanPath(p string) string {
// can backtrack
w--

if buf == nil {
if len(buf) == 0 {
for w > 1 && p[w] != '/' {
w--
}
Expand Down Expand Up @@ -103,21 +113,28 @@ func cleanPath(p string) string {
w++
}

if buf == nil {
if len(buf) == 0 {
return p[:w]
}
return string(buf[:w])
}

// internal helper to lazily create a buffer if necessary.
func bufApp(buf *[]byte, s string, w int, c byte) {
if *buf == nil {
b := *buf
if len(b) == 0 {
if s[w] == c {
return
}

*buf = make([]byte, len(s))
copy(*buf, s[:w])
if l := len(s); l > cap(b) {
*buf = make([]byte, len(s))
} else {
*buf = (*buf)[:l]
}
b = *buf

copy(b, s[:w])
}
(*buf)[w] = c
b[w] = c
}
65 changes: 58 additions & 7 deletions path_test.go
Expand Up @@ -6,15 +6,17 @@
package gin

import (
"runtime"
"strings"
"testing"

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

var cleanTests = []struct {
type cleanPathTest struct {
path, result string
}{
}

var cleanTests = []cleanPathTest{
// Already clean
{"/", "/"},
{"/abc", "/abc"},
Expand Down Expand Up @@ -77,13 +79,62 @@ func TestPathCleanMallocs(t *testing.T) {
if testing.Short() {
t.Skip("skipping malloc count in short mode")
}
if runtime.GOMAXPROCS(0) > 1 {
t.Log("skipping AllocsPerRun checks; GOMAXPROCS>1")
return
}

for _, test := range cleanTests {
allocs := testing.AllocsPerRun(100, func() { cleanPath(test.result) })
assert.EqualValues(t, allocs, 0)
}
}

func BenchmarkPathClean(b *testing.B) {
b.ReportAllocs()

for i := 0; i < b.N; i++ {
for _, test := range cleanTests {
cleanPath(test.path)
}
}
}

func genLongPaths() (testPaths []cleanPathTest) {
for i := 1; i <= 1234; i++ {
ss := strings.Repeat("a", i)

correctPath := "/" + ss
testPaths = append(testPaths, cleanPathTest{
path: correctPath,
result: correctPath,
}, cleanPathTest{
path: ss,
result: correctPath,
}, cleanPathTest{
path: "//" + ss,
result: correctPath,
}, cleanPathTest{
path: "/" + ss + "/b/..",
result: correctPath,
})
}
return
}

func TestPathCleanLong(t *testing.T) {
cleanTests := genLongPaths()

for _, test := range cleanTests {
assert.Equal(t, test.result, cleanPath(test.path))
assert.Equal(t, test.result, cleanPath(test.result))
}
}

func BenchmarkPathCleanLong(b *testing.B) {
cleanTests := genLongPaths()
b.ResetTimer()
b.ReportAllocs()

for i := 0; i < b.N; i++ {
for _, test := range cleanTests {
cleanPath(test.path)
}
}
}

0 comments on commit 94c85f4

Please sign in to comment.