Merge pull request #35181 from wojtek-t/avoid_recomputing_fields

pkg/registry/core/endpoint/strategy.go

@@ -26,7 +26,7 @@ import (
 	"k8s.io/kubernetes/pkg/labels"
 	"k8s.io/kubernetes/pkg/registry/generic"
 	"k8s.io/kubernetes/pkg/runtime"
-	apistorage "k8s.io/kubernetes/pkg/storage"
+	pkgstorage "k8s.io/kubernetes/pkg/storage"
 	"k8s.io/kubernetes/pkg/util/validation/field"
 )
 
@@ -80,16 +80,31 @@ func (endpointsStrategy) AllowUnconditionalUpdate() bool {
 }
 
 // MatchEndpoints returns a generic matcher for a given label and field selector.
-func MatchEndpoints(label labels.Selector, field fields.Selector) apistorage.SelectionPredicate {
-	return apistorage.SelectionPredicate{Label: label, Field: field, GetAttrs: EndpointsAttributes}
+func MatchEndpoints(label labels.Selector, field fields.Selector) pkgstorage.SelectionPredicate {
+	return pkgstorage.SelectionPredicate{
+		Label: label,
+		Field: field,
+		GetAttrs: func(obj runtime.Object) (labels.Set, fields.Set, error) {
+			endpoints, ok := obj.(*api.Endpoints)
+			if !ok {
+				return nil, nil, fmt.Errorf("invalid object type %#v", obj)
+			}
+
+			// Compute fields only if field selectors is non-empty
+			// (otherwise those won't be used).
+			// Those are generally also not needed if label selector does
+			// not match labels, but additional computation of it is expensive.
+			var endpointsFields fields.Set
+			if !field.Empty() {
+				endpointsFields = EndpointsToSelectableFields(endpoints)
+			}
+			return endpoints.Labels, endpointsFields, nil
+		},
+	}
 }
 
-// EndpointsAttributes returns the attributes of an endpoint such that a
-// SelectionPredicate can match appropriately.
-func EndpointsAttributes(obj runtime.Object) (objLabels labels.Set, objFields fields.Set, err error) {
-	endpoints, ok := obj.(*api.Endpoints)
-	if !ok {
-		return nil, nil, fmt.Errorf("invalid object type %#v", obj)
-	}
-	return endpoints.Labels, generic.ObjectMetaFieldsSet(&endpoints.ObjectMeta, true), nil
+// EndpointsToSelectableFields returns a field set that represents the object
+// TODO: fields are not labels, and the validation rules for them do not apply.
+func EndpointsToSelectableFields(endpoints *api.Endpoints) fields.Set {
+	return generic.ObjectMetaFieldsSet(&endpoints.ObjectMeta, true)
 }

pkg/registry/core/endpoint/strategy_test.go

@@ -25,14 +25,10 @@ import (
 )
 
 func TestSelectableFieldLabelConversions(t *testing.T) {
-	_, fieldsSet, err := EndpointsAttributes(&api.Endpoints{})
-	if err != nil {
-		t.Fatal(err)
-	}
 	apitesting.TestSelectableFieldLabelConversionsOfKind(t,
 		registered.GroupOrDie(api.GroupName).GroupVersion.String(),
 		"Endpoints",
-		fieldsSet,
+		EndpointsToSelectableFields(&api.Endpoints{}),
 		nil,
 	)
 }

pkg/registry/core/node/strategy.go

@@ -154,7 +154,16 @@ func MatchNode(label labels.Selector, field fields.Selector) pkgstorage.Selectio
 			if !ok {
 				return nil, nil, fmt.Errorf("not a node")
 			}
-			return labels.Set(nodeObj.ObjectMeta.Labels), NodeToSelectableFields(nodeObj), nil
+
+			// Compute fields only if field selectors is non-empty
+			// (otherwise those won't be used).
+			// Those are generally also not needed if label selector does
+			// not match labels, but additional computation of it is expensive.
+			var nodeFields fields.Set
+			if !field.Empty() {
+				nodeFields = NodeToSelectableFields(nodeObj)
+			}
+			return labels.Set(nodeObj.ObjectMeta.Labels), nodeFields, nil
 		},
 		IndexFields: []string{"metadata.name"},
 	}

pkg/registry/core/pod/strategy.go

@@ -166,18 +166,15 @@ func MatchPod(label labels.Selector, field fields.Selector) storage.SelectionPre
 				return nil, nil, fmt.Errorf("not a pod")
 			}
 
-			// podLabels is already sitting there ready to be used.
-			// podFields is not available directly and requires allocation of a map.
-			// Only bother if the fields might be useful to determining the match.
-			// One common case is for a replication controller to set up a watch
-			// based on labels alone; in that case we can avoid allocating the field map.
-			// This is especially important in the apiserver.
-			podLabels := labels.Set(pod.ObjectMeta.Labels)
+			// Compute fields only if field selectors is non-empty
+			// (otherwise those won't be used).
+			// Those are generally also not needed if label selector does
+			// not match labels, but additional computation of it is expensive.
 			var podFields fields.Set
-			if !field.Empty() && label.Matches(podLabels) {
+			if !field.Empty() {
 				podFields = PodToSelectableFields(pod)
 			}
-			return podLabels, podFields, nil
+			return labels.Set(pod.ObjectMeta.Labels), podFields, nil
 		},
 		IndexFields: []string{"spec.nodeName"},
 	}