From 389a8436b52db4936b56e08f07984da362c91f6b Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 25 Mar 2019 18:06:57 -0400 Subject: [PATCH] Avoid allocations when building SelfLinks and fast path escape A self link should only require one allocation, and we should skip url.PathEscape() except when the path actually needs it. Add a fuzz test to build random strings and verify them against the optimized implementation. Add a new BenchmarkWatchHTTP_UTF8 that covers when we have unicode names in the self link. ``` > before BenchmarkGet-12 10000 118863 ns/op 17482 B/op 130 allocs/op BenchmarkWatchHTTP-12 30000 38346 ns/op 1893 B/op 29 allocs/op > after BenchmarkGet-12 10000 116218 ns/op 17456 B/op 130 allocs/op BenchmarkWatchHTTP-12 50000 35988 ns/op 1571 B/op 26 allocs/op BenchmarkWatchHTTP_UTF8-12 50000 41467 ns/op 1928 B/op 28 allocs/op ``` Saves 3 allocations in the fast path and 1 in the slow path (the slow path has to build the buffer and then call url.EscapedPath which always allocates). --- .../apiserver/pkg/endpoints/handlers/BUILD | 1 + .../apiserver/pkg/endpoints/handlers/namer.go | 34 ++++++++-- .../pkg/endpoints/handlers/namer_test.go | 65 ++++++++++++++++++- .../apiserver/pkg/endpoints/watch_test.go | 24 +++++++ 4 files changed, 116 insertions(+), 8 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 55998af532bc..20733ff3848a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -34,6 +34,7 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//vendor/github.com/evanphx/json-patch:go_default_library", + "//vendor/github.com/google/gofuzz:go_default_library", "//vendor/k8s.io/utils/trace:go_default_library", ], ) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go index 16b4199c2b33..755da22eea5f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer.go @@ -20,6 +20,7 @@ import ( "fmt" "net/http" "net/url" + "strings" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -86,6 +87,21 @@ func (n ContextBasedNaming) Name(req *http.Request) (namespace, name string, err return ns, requestInfo.Name, nil } +// fastURLPathEncode encodes the provided path as a URL path +func fastURLPathEncode(path string) string { + for _, r := range []byte(path) { + switch { + case r >= '-' && r <= '9', r >= 'A' && r <= 'Z', r >= 'a' && r <= 'z': + // characters within this range do not require escaping + default: + var u url.URL + u.Path = path + return u.EscapedPath() + } + } + return path +} + func (n ContextBasedNaming) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) { namespace, name, err := n.ObjectName(obj) if err == errEmptyName && len(requestInfo.Name) > 0 { @@ -101,19 +117,23 @@ func (n ContextBasedNaming) GenerateLink(requestInfo *request.RequestInfo, obj r return n.SelfLinkPathPrefix + url.QueryEscape(name) + n.SelfLinkPathSuffix, nil } - return n.SelfLinkPathPrefix + - url.QueryEscape(namespace) + - "/" + url.QueryEscape(requestInfo.Resource) + "/" + - url.QueryEscape(name) + - n.SelfLinkPathSuffix, - nil + builder := strings.Builder{} + builder.Grow(len(n.SelfLinkPathPrefix) + len(namespace) + len(requestInfo.Resource) + len(name) + len(n.SelfLinkPathSuffix) + 8) + builder.WriteString(n.SelfLinkPathPrefix) + builder.WriteString(namespace) + builder.WriteByte('/') + builder.WriteString(requestInfo.Resource) + builder.WriteByte('/') + builder.WriteString(name) + builder.WriteString(n.SelfLinkPathSuffix) + return fastURLPathEncode(builder.String()), nil } func (n ContextBasedNaming) GenerateListLink(req *http.Request) (uri string, err error) { if len(req.URL.RawPath) > 0 { return req.URL.RawPath, nil } - return req.URL.EscapedPath(), nil + return fastURLPathEncode(req.URL.Path), nil } func (n ContextBasedNaming) ObjectName(obj runtime.Object) (namespace, name string, err error) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer_test.go index 15c88189313b..861296278507 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/namer_test.go @@ -17,9 +17,13 @@ limitations under the License. package handlers import ( + "math/rand" + "net/url" "testing" - "k8s.io/api/core/v1" + fuzz "github.com/google/gofuzz" + + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -114,3 +118,62 @@ func TestGenerateLink(t *testing.T) { } } } + +func Test_fastURLPathEncode_fuzz(t *testing.T) { + specialCases := []string{"/", "//", ".", "*", "/abc%"} + for _, test := range specialCases { + got := fastURLPathEncode(test) + u := url.URL{Path: test} + expected := u.EscapedPath() + if got != expected { + t.Errorf("%q did not match %q", got, expected) + } + } + f := fuzz.New().Funcs( + func(s *string, c fuzz.Continue) { + *s = randString(c.Rand) + }, + ) + for i := 0; i < 2000; i++ { + var test string + f.Fuzz(&test) + + got := fastURLPathEncode(test) + u := url.URL{Path: test} + expected := u.EscapedPath() + if got != expected { + t.Errorf("%q did not match %q", got, expected) + } + } +} + +// Unicode range fuzzer from github.com/google/gofuzz/fuzz.go + +type charRange struct { + first, last rune +} + +var unicodeRanges = []charRange{ + {0x00, 0x255}, + {' ', '~'}, // ASCII characters + {'\u00a0', '\u02af'}, // Multi-byte encoded characters + {'\u4e00', '\u9fff'}, // Common CJK (even longer encodings) +} + +// randString makes a random string up to 20 characters long. The returned string +// may include a variety of (valid) UTF-8 encodings. +func randString(r *rand.Rand) string { + n := r.Intn(20) + runes := make([]rune, n) + for i := range runes { + runes[i] = unicodeRanges[r.Intn(len(unicodeRanges))].choose(r) + } + return string(runes) +} + +// choose returns a random unicode character from the given range, using the +// given randomness source. +func (r *charRange) choose(rand *rand.Rand) rune { + count := int64(r.last - r.first) + return r.first + rune(rand.Int63n(count)) +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/watch_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/watch_test.go index 5d8789a46862..24acbf717f97 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/watch_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/watch_test.go @@ -757,6 +757,30 @@ func TestWatchHTTPTimeout(t *testing.T) { func BenchmarkWatchHTTP(b *testing.B) { items := benchmarkItems(b) + // use ASCII names to capture the cost of handling ASCII only self-links + for i := range items { + item := &items[i] + item.Namespace = fmt.Sprintf("namespace-%d", i) + item.Name = fmt.Sprintf("reasonable-name-%d", i) + } + + runWatchHTTPBenchmark(b, items) +} + +func BenchmarkWatchHTTP_UTF8(b *testing.B) { + items := benchmarkItems(b) + + // use UTF names to capture the cost of handling UTF-8 escaping in self-links + for i := range items { + item := &items[i] + item.Namespace = fmt.Sprintf("躀痢疈蜧í柢-%d", i) + item.Name = fmt.Sprintf("翏Ŏ熡韐-%d", i) + } + + runWatchHTTPBenchmark(b, items) +} + +func runWatchHTTPBenchmark(b *testing.B, items []example.Pod) { simpleStorage := &SimpleRESTStorage{} handler := handle(map[string]rest.Storage{"simples": simpleStorage}) server := httptest.NewServer(handler)