From 025950afe980be14531c51c2790dcb966da1d3fd Mon Sep 17 00:00:00 2001
From: Andy Pan <panjf2000@gmail.com>
Date: Tue, 7 Jan 2020 17:37:18 +0800
Subject: [PATCH] Reuse bytes when cleaning the URL paths (#2179)
* path: use stack buffer in CleanPath to avoid allocs in common case
Sync from https://github.com/julienschmidt/httprouter/commit/8222db13dbb3b3ab1eb84edb61a7030708b93bfa
* 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>
---
path.go | 33 +++++++++++++++++++-------
path_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 83 insertions(+), 15 deletions(-)
diff --git a/path.go b/path.go
index d1f59622..a6bac7bd 100644
--- a/path.go
+++ b/path.go
@@ -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.
//
@@ -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.
@@ -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] = '/'
}
@@ -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--
}
@@ -103,7 +113,7 @@ func cleanPath(p string) string {
w++
}
- if buf == nil {
+ if len(buf) == 0 {
return p[:w]
}
return string(buf[:w])
@@ -111,13 +121,20 @@ func cleanPath(p string) string {
// 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
}
diff --git a/path_test.go b/path_test.go
index c1e6ed4f..caefd63a 100644
--- a/path_test.go
+++ b/path_test.go
@@ -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"},
@@ -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)
+ }
+ }
+}