From 9006e59e3b42ed2d80f12bc5b88fa8fa54d72ef9 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 21 Mar 2019 20:43:26 -0400 Subject: [PATCH] IsListType uses reflection and is expensive for hot paths IsListType was causing ~100 allocations for a non list object. It is used in a wide range of code, so perform a more targeted check. The use of `%#v` in a hot return path for `fmt.Errorf()` was the main victim. Replace `%#v` with a typed error and create a cache of types that are lists with a bounded size (probably not necessary, but safer). ``` BenchmarkGet-12 100000 119635 ns/op 20110 B/op 206 allocs/op BenchmarkWatchHTTP-12 100000 65761 ns/op 7296 B/op 139 allocs/op BenchmarkGet-12 100000 109085 ns/op 17831 B/op 152 allocs/op BenchmarkWatchHTTP-12 200000 33966 ns/op 1913 B/op 30 allocs/op ``` Kubernetes-commit: 58fb665646aa4c1b63f1322a50e3af7a60e39886 --- pkg/endpoints/handlers/rest.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/endpoints/handlers/rest.go b/pkg/endpoints/handlers/rest.go index f482ae317..c184ce5f9 100644 --- a/pkg/endpoints/handlers/rest.go +++ b/pkg/endpoints/handlers/rest.go @@ -292,7 +292,12 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err } // setObjectSelfLink sets the self link of an object as needed. +// TODO: remove the need for the namer LinkSetters by requiring objects implement either Object or List +// interfaces func setObjectSelfLink(ctx context.Context, obj runtime.Object, req *http.Request, namer ScopeNamer) error { + // We only generate list links on objects that implement ListInterface - historically we duck typed this + // check via reflection, but as we move away from reflection we require that you not only carry Items but + // ListMeta into order to be identified as a list. if !meta.IsListType(obj) { requestInfo, ok := request.RequestInfoFrom(ctx) if !ok {