Merge pull request #39247 from wojtek-t/optimize_controller_manager_memory

Automatic merge from submit-queue

Avoid unnecessary memory allocations

Low-hanging fruits in saving memory allocations. During our 5000-node kubemark runs I've see this:

ControllerManager:
- 40.17% k8s.io/kubernetes/pkg/util/system.IsMasterNode
- 19.04% k8s.io/kubernetes/pkg/controller.(*PodControllerRefManager).Classify

Scheduler:
- 42.74% k8s.io/kubernetes/plugin/pkg/scheduler/algrorithm/predicates.(*MaxPDVolumeCountChecker).filterVolumes

This PR is eliminating all of those.

pkg/controller/controller_ref_manager.go

@@ -66,7 +66,7 @@ func (m *PodControllerRefManager) Classify(pods []*v1.Pod) (
 				pod.Namespace, pod.Name, pod.Status.Phase, pod.DeletionTimestamp)
 			continue
 		}
-		controllerRef := GetControllerOf(pod.ObjectMeta)
+		controllerRef := GetControllerOf(&pod.ObjectMeta)
 		if controllerRef != nil {
 			if controllerRef.UID == m.controllerObject.UID {
 				// already controlled
@@ -93,11 +93,12 @@ func (m *PodControllerRefManager) Classify(pods []*v1.Pod) (
 
 // GetControllerOf returns the controllerRef if controllee has a controller,
 // otherwise returns nil.
-func GetControllerOf(controllee v1.ObjectMeta) *metav1.OwnerReference {
-	for _, owner := range controllee.OwnerReferences {
+func GetControllerOf(controllee *v1.ObjectMeta) *metav1.OwnerReference {
+	for i := range controllee.OwnerReferences {
+		owner := &controllee.OwnerReferences[i]
 		// controlled by other controller
 		if owner.Controller != nil && *owner.Controller == true {
-			return &owner
+			return owner
 		}
 	}
 	return nil
@@ -183,7 +184,7 @@ func (m *ReplicaSetControllerRefManager) Classify(replicaSets []*extensions.Repl
 	controlledDoesNotMatch []*extensions.ReplicaSet) {
 	for i := range replicaSets {
 		replicaSet := replicaSets[i]
-		controllerRef := GetControllerOf(replicaSet.ObjectMeta)
+		controllerRef := GetControllerOf(&replicaSet.ObjectMeta)
 		if controllerRef != nil {
 			if controllerRef.UID != m.controllerObject.UID {
 				// ignoring the ReplicaSet controlled by other Deployment

pkg/controller/deployment/deployment_controller.go

@@ -372,7 +372,7 @@ func (dc *DeploymentController) getDeploymentForPod(pod *v1.Pod) *extensions.Dep
 	var rs *extensions.ReplicaSet
 	var err error
 	// Look at the owner reference
-	controllerRef := controller.GetControllerOf(pod.ObjectMeta)
+	controllerRef := controller.GetControllerOf(&pod.ObjectMeta)
 	if controllerRef != nil {
 		// Not a pod owned by a replica set.
 		if controllerRef.Kind != extensions.SchemeGroupVersion.WithKind("ReplicaSet").Kind {

pkg/util/system/system_utils.go

@@ -17,11 +17,20 @@ limitations under the License.
 package system
 
 import (
-	"regexp"
+	"strings"
 )
 
 // TODO: find a better way of figuring out if given node is a registered master.
 func IsMasterNode(nodeName string) bool {
-	r := regexp.MustCompile("master(-...)?$")
-	return r.MatchString(nodeName)
+	// We are trying to capture "master(-...)?$" regexp.
+	// However, using regexp.MatchString() results even in more than 35%
+	// of all space allocations in ControllerManager spent in this function.
+	// That's why we are trying to be a bit smarter.
+	if strings.HasSuffix(nodeName, "master") {
+		return true
+	}
+	if len(nodeName) >= 10 {
+		return strings.HasSuffix(nodeName[:len(nodeName)-3], "master-")
+	}
+	return false
 }

plugin/pkg/scheduler/algorithm/predicates/predicates.go

@@ -217,8 +217,9 @@ func NewMaxPDVolumeCountPredicate(filter VolumeFilter, maxVolumes int, pvInfo Pe
 }
 
 func (c *MaxPDVolumeCountChecker) filterVolumes(volumes []v1.Volume, namespace string, filteredVolumes map[string]bool) error {
-	for _, vol := range volumes {
-		if id, ok := c.filter.FilterVolume(&vol); ok {
+	for i := range volumes {
+		vol := &volumes[i]
+		if id, ok := c.filter.FilterVolume(vol); ok {
 			filteredVolumes[id] = true
 		} else if vol.PersistentVolumeClaim != nil {
 			pvcName := vol.PersistentVolumeClaim.ClaimName

test/e2e/garbage_collector.go

@@ -433,7 +433,7 @@ var _ = framework.KubeDescribe("Garbage collector", func() {
 			framework.Failf("Failed to list ReplicaSet %v", err)
 		}
 		for _, replicaSet := range rs.Items {
-			if controller.GetControllerOf(replicaSet.ObjectMeta) != nil {
+			if controller.GetControllerOf(&replicaSet.ObjectMeta) != nil {
 				framework.Failf("Found ReplicaSet with non nil ownerRef %v", replicaSet)
 			}
 		}