Merge pull request kubernetes#75570 from smarterclayton/fix_performance

Fix a regression in watch performance and reduce allocations in the GET path

Author: k8s-ci-robot

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -644,6 +644,8 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
 	return []runtime.SerializerInfo{
 		{
 			MediaType:        "application/json",
+			MediaTypeType:    "application",
+			MediaTypeSubType: "json",
 			EncodesAsText:    true,
 			Serializer:       json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, false),
 			PrettySerializer: json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, true),
@@ -654,9 +656,11 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
 			},
 		},
 		{
-			MediaType:     "application/yaml",
-			EncodesAsText: true,
-			Serializer:    json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
+			MediaType:        "application/yaml",
+			MediaTypeType:    "application",
+			MediaTypeSubType: "yaml",
+			EncodesAsText:    true,
+			Serializer:       json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
 		},
 	}
 }

staging/src/k8s.io/apimachinery/pkg/api/meta/help.go

@@ -17,50 +17,96 @@ limitations under the License.
 package meta
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
+	"sync"
 
 	"k8s.io/apimachinery/pkg/conversion"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
-// IsListType returns true if the provided Object has a slice called Items
+var (
+	// isListCache maintains a cache of types that are checked for lists
+	// which is used by IsListType.
+	// TODO: remove and replace with an interface check
+	isListCache = struct {
+		lock   sync.RWMutex
+		byType map[reflect.Type]bool
+	}{
+		byType: make(map[reflect.Type]bool, 1024),
+	}
+)
+
+// IsListType returns true if the provided Object has a slice called Items.
+// TODO: Replace the code in this check with an interface comparison by
+//   creating and enforcing that lists implement a list accessor.
 func IsListType(obj runtime.Object) bool {
-	// if we're a runtime.Unstructured, check whether this is a list.
-	// TODO: refactor GetItemsPtr to use an interface that returns []runtime.Object
-	if unstructured, ok := obj.(runtime.Unstructured); ok {
-		return unstructured.IsList()
+	switch t := obj.(type) {
+	case runtime.Unstructured:
+		return t.IsList()
 	}
+	t := reflect.TypeOf(obj)
 
-	_, err := GetItemsPtr(obj)
-	return err == nil
+	isListCache.lock.RLock()
+	ok, exists := isListCache.byType[t]
+	isListCache.lock.RUnlock()
+
+	if !exists {
+		_, err := getItemsPtr(obj)
+		ok = err == nil
+
+		// cache only the first 1024 types
+		isListCache.lock.Lock()
+		if len(isListCache.byType) < 1024 {
+			isListCache.byType[t] = ok
+		}
+		isListCache.lock.Unlock()
+	}
+
+	return ok
 }
 
+var (
+	errExpectFieldItems = errors.New("no Items field in this object")
+	errExpectSliceItems = errors.New("Items field must be a slice of objects")
+)
+
 // GetItemsPtr returns a pointer to the list object's Items member.
 // If 'list' doesn't have an Items member, it's not really a list type
 // and an error will be returned.
 // This function will either return a pointer to a slice, or an error, but not both.
+// TODO: this will be replaced with an interface in the future
 func GetItemsPtr(list runtime.Object) (interface{}, error) {
+	obj, err := getItemsPtr(list)
+	if err != nil {
+		return nil, fmt.Errorf("%T is not a list: %v", err)
+	}
+	return obj, nil
+}
+
+// getItemsPtr returns a pointer to the list object's Items member or an error.
+func getItemsPtr(list runtime.Object) (interface{}, error) {
 	v, err := conversion.EnforcePtr(list)
 	if err != nil {
 		return nil, err
 	}
 
 	items := v.FieldByName("Items")
 	if !items.IsValid() {
-		return nil, fmt.Errorf("no Items field in %#v", list)
+		return nil, errExpectFieldItems
 	}
 	switch items.Kind() {
 	case reflect.Interface, reflect.Ptr:
 		target := reflect.TypeOf(items.Interface()).Elem()
 		if target.Kind() != reflect.Slice {
-			return nil, fmt.Errorf("items: Expected slice, got %s", target.Kind())
+			return nil, errExpectSliceItems
 		}
 		return items.Interface(), nil
 	case reflect.Slice:
 		return items.Addr().Interface(), nil
 	default:
-		return nil, fmt.Errorf("items: Expected slice, got %s", items.Kind())
+		return nil, errExpectSliceItems
 	}
 }
 

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructuredscheme/scheme.go

@@ -51,6 +51,8 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
 	return []runtime.SerializerInfo{
 		{
 			MediaType:        "application/json",
+			MediaTypeType:    "application",
+			MediaTypeSubType: "json",
 			EncodesAsText:    true,
 			Serializer:       json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, false),
 			PrettySerializer: json.NewSerializer(json.DefaultMetaFactory, s.creator, s.typer, true),
@@ -61,9 +63,11 @@ func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.Serial
 			},
 		},
 		{
-			MediaType:     "application/yaml",
-			EncodesAsText: true,
-			Serializer:    json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
+			MediaType:        "application/yaml",
+			MediaTypeType:    "application",
+			MediaTypeSubType: "yaml",
+			EncodesAsText:    true,
+			Serializer:       json.NewYAMLSerializer(json.DefaultMetaFactory, s.creator, s.typer),
 		},
 	}
 }

staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go

@@ -91,6 +91,10 @@ type Framer interface {
 type SerializerInfo struct {
 	// MediaType is the value that represents this serializer over the wire.
 	MediaType string
+	// MediaTypeType is the first part of the MediaType ("application" in "application/json").
+	MediaTypeType string
+	// MediaTypeSubType is the second part of the MediaType ("json" in "application/json").
+	MediaTypeSubType string
 	// EncodesAsText indicates this serializer can be encoded to UTF-8 safely.
 	EncodesAsText bool
 	// Serializer is the individual object serializer for this media type.

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/codec_factory.go

@@ -17,6 +17,9 @@ limitations under the License.
 package serializer
 
 import (
+	"mime"
+	"strings"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
@@ -120,6 +123,15 @@ func newCodecFactory(scheme *runtime.Scheme, serializers []serializerType) Codec
 				Serializer:       d.Serializer,
 				PrettySerializer: d.PrettySerializer,
 			}
+
+			mediaType, _, err := mime.ParseMediaType(info.MediaType)
+			if err != nil {
+				panic(err)
+			}
+			parts := strings.SplitN(mediaType, "/", 2)
+			info.MediaTypeType = parts[0]
+			info.MediaTypeSubType = parts[1]
+
 			if d.StreamSerializer != nil {
 				info.StreamSerializer = &runtime.StreamSerializerInfo{
 					Serializer:    d.StreamSerializer,

staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

@@ -1616,6 +1616,41 @@ func TestGet(t *testing.T) {
 	}
 }
 
+func BenchmarkGet(b *testing.B) {
+	storage := map[string]rest.Storage{}
+	simpleStorage := SimpleRESTStorage{
+		item: genericapitesting.Simple{
+			Other: "foo",
+		},
+	}
+	selfLinker := &setTestSelfLinker{
+		expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id",
+		name:        "id",
+		namespace:   "default",
+	}
+	storage["simple"] = &simpleStorage
+	handler := handleLinker(storage, selfLinker)
+	server := httptest.NewServer(handler)
+	defer server.Close()
+
+	u := server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id"
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		resp, err := http.Get(u)
+		if err != nil {
+			b.Fatalf("unexpected error: %v", err)
+		}
+		if resp.StatusCode != http.StatusOK {
+			b.Fatalf("unexpected response: %#v", resp)
+		}
+		if _, err := io.Copy(ioutil.Discard, resp.Body); err != nil {
+			b.Fatalf("unable to read body")
+		}
+	}
+	b.StopTimer()
+}
+
 func TestGetCompression(t *testing.T) {
 	storage := map[string]rest.Storage{}
 	simpleStorage := SimpleRESTStorage{

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/negotiation/negotiate.go

@@ -43,13 +43,13 @@ func MediaTypesForSerializer(ns runtime.NegotiatedSerializer) (mediaTypes, strea
 // NegotiateOutputMediaType negotiates the output structured media type and a serializer, or
 // returns an error.
 func NegotiateOutputMediaType(req *http.Request, ns runtime.NegotiatedSerializer, restrictions EndpointRestrictions) (MediaTypeOptions, runtime.SerializerInfo, error) {
-	mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), AcceptedMediaTypesForEndpoint(ns), restrictions)
+	mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), ns.SupportedMediaTypes(), restrictions)
 	if !ok {
 		supported, _ := MediaTypesForSerializer(ns)
 		return mediaType, runtime.SerializerInfo{}, NewNotAcceptableError(supported)
 	}
 	// TODO: move into resthandler
-	info := mediaType.Accepted.Serializer
+	info := mediaType.Accepted
 	if (mediaType.Pretty || isPrettyPrint(req)) && info.PrettySerializer != nil {
 		info.Serializer = info.PrettySerializer
 	}
@@ -58,12 +58,12 @@ func NegotiateOutputMediaType(req *http.Request, ns runtime.NegotiatedSerializer
 
 // NegotiateOutputMediaTypeStream returns a stream serializer for the given request.
 func NegotiateOutputMediaTypeStream(req *http.Request, ns runtime.NegotiatedSerializer, restrictions EndpointRestrictions) (runtime.SerializerInfo, error) {
-	mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), AcceptedMediaTypesForEndpoint(ns), restrictions)
-	if !ok || mediaType.Accepted.Serializer.StreamSerializer == nil {
+	mediaType, ok := NegotiateMediaTypeOptions(req.Header.Get("Accept"), ns.SupportedMediaTypes(), restrictions)
+	if !ok || mediaType.Accepted.StreamSerializer == nil {
 		_, supported := MediaTypesForSerializer(ns)
 		return runtime.SerializerInfo{}, NewNotAcceptableError(supported)
 	}
-	return mediaType.Accepted.Serializer, nil
+	return mediaType.Accepted, nil
 }
 
 // NegotiateInputSerializer returns the input serializer for the provided request.
@@ -99,10 +99,13 @@ func NegotiateInputSerializerForMediaType(mediaType string, streaming bool, ns r
 func isPrettyPrint(req *http.Request) bool {
 	// DEPRECATED: should be part of the content type
 	if req.URL != nil {
-		pp := req.URL.Query().Get("pretty")
-		if len(pp) > 0 {
-			pretty, _ := strconv.ParseBool(pp)
-			return pretty
+		// avoid an allocation caused by parsing the URL query
+		if strings.Contains(req.URL.RawQuery, "pretty") {
+			pp := req.URL.Query().Get("pretty")
+			if len(pp) > 0 {
+				pretty, _ := strconv.ParseBool(pp)
+				return pretty
+			}
 		}
 	}
 	userAgent := req.UserAgent()
@@ -139,17 +142,6 @@ func (emptyEndpointRestrictions) AllowsConversion(schema.GroupVersionKind, strin
 func (emptyEndpointRestrictions) AllowsServerVersion(string) bool  { return false }
 func (emptyEndpointRestrictions) AllowsStreamSchema(s string) bool { return s == "watch" }
 
-// AcceptedMediaType contains information about a valid media type that the
-// server can serialize.
-type AcceptedMediaType struct {
-	// Type is the first part of the media type ("application")
-	Type string
-	// SubType is the second part of the media type ("json")
-	SubType string
-	// Serializer is the serialization info this object accepts
-	Serializer runtime.SerializerInfo
-}
-
 // MediaTypeOptions describes information for a given media type that may alter
 // the server response
 type MediaTypeOptions struct {
@@ -176,13 +168,13 @@ type MediaTypeOptions struct {
 	Unrecognized []string
 
 	// the accepted media type from the client
-	Accepted *AcceptedMediaType
+	Accepted runtime.SerializerInfo
 }
 
 // acceptMediaTypeOptions returns an options object that matches the provided media type params. If
 // it returns false, the provided options are not allowed and the media type must be skipped.  These
 // parameters are unversioned and may not be changed.
-func acceptMediaTypeOptions(params map[string]string, accepts *AcceptedMediaType, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
+func acceptMediaTypeOptions(params map[string]string, accepts *runtime.SerializerInfo, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
 	var options MediaTypeOptions
 
 	// extract all known parameters
@@ -208,7 +200,7 @@ func acceptMediaTypeOptions(params map[string]string, accepts *AcceptedMediaType
 
 		// controls the streaming schema
 		case "stream":
-			if len(v) > 0 && (accepts.Serializer.StreamSerializer == nil || !endpoint.AllowsStreamSchema(v)) {
+			if len(v) > 0 && (accepts.StreamSerializer == nil || !endpoint.AllowsStreamSchema(v)) {
 				return MediaTypeOptions{}, false
 			}
 			options.Stream = v
@@ -236,27 +228,27 @@ func acceptMediaTypeOptions(params map[string]string, accepts *AcceptedMediaType
 		}
 	}
 
-	if options.Convert != nil && !endpoint.AllowsConversion(*options.Convert, accepts.Type, accepts.SubType) {
+	if options.Convert != nil && !endpoint.AllowsConversion(*options.Convert, accepts.MediaTypeType, accepts.MediaTypeSubType) {
 		return MediaTypeOptions{}, false
 	}
 
-	options.Accepted = accepts
+	options.Accepted = *accepts
 	return options, true
 }
 
 type candidateMediaType struct {
-	accepted *AcceptedMediaType
+	accepted *runtime.SerializerInfo
 	clauses  goautoneg.Accept
 }
 
 type candidateMediaTypeSlice []candidateMediaType
 
 // NegotiateMediaTypeOptions returns the most appropriate content type given the accept header and
 // a list of alternatives along with the accepted media type parameters.
-func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
+func NegotiateMediaTypeOptions(header string, accepted []runtime.SerializerInfo, endpoint EndpointRestrictions) (MediaTypeOptions, bool) {
 	if len(header) == 0 && len(accepted) > 0 {
 		return MediaTypeOptions{
-			Accepted: &accepted[0],
+			Accepted: accepted[0],
 		}, true
 	}
 
@@ -266,8 +258,8 @@ func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endp
 		for i := range accepted {
 			accepts := &accepted[i]
 			switch {
-			case clause.Type == accepts.Type && clause.SubType == accepts.SubType,
-				clause.Type == accepts.Type && clause.SubType == "*",
+			case clause.Type == accepts.MediaTypeType && clause.SubType == accepts.MediaTypeSubType,
+				clause.Type == accepts.MediaTypeType && clause.SubType == "*",
 				clause.Type == "*" && clause.SubType == "*":
 				candidates = append(candidates, candidateMediaType{accepted: accepts, clauses: clause})
 			}
@@ -282,22 +274,3 @@ func NegotiateMediaTypeOptions(header string, accepted []AcceptedMediaType, endp
 
 	return MediaTypeOptions{}, false
 }
-
-// AcceptedMediaTypesForEndpoint returns an array of structs that are used to efficiently check which
-// allowed media types the server exposes.
-func AcceptedMediaTypesForEndpoint(ns runtime.NegotiatedSerializer) []AcceptedMediaType {
-	var acceptedMediaTypes []AcceptedMediaType
-	for _, info := range ns.SupportedMediaTypes() {
-		segments := strings.SplitN(info.MediaType, "/", 2)
-		if len(segments) == 1 {
-			segments = append(segments, "*")
-		}
-		t := AcceptedMediaType{
-			Type:       segments[0],
-			SubType:    segments[1],
-			Serializer: info,
-		}
-		acceptedMediaTypes = append(acceptedMediaTypes, t)
-	}
-	return acceptedMediaTypes
-}