From d1292a73976a35d298918280eecb422e0d6bf765 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Tue, 27 Dec 2016 15:45:13 +0100 Subject: [PATCH] Optimize memory allocations in controller manager --- pkg/controller/controller_ref_manager.go | 11 ++++++----- .../deployment/deployment_controller.go | 2 +- pkg/util/system/system_utils.go | 15 ++++++++++++--- test/e2e/garbage_collector.go | 2 +- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pkg/controller/controller_ref_manager.go b/pkg/controller/controller_ref_manager.go index 742dc4316d..f8c60c3e47 100644 --- a/pkg/controller/controller_ref_manager.go +++ b/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 diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 2a189a042e..cea38cf437 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/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 { diff --git a/pkg/util/system/system_utils.go b/pkg/util/system/system_utils.go index a806dac6cb..bf1a024b50 100644 --- a/pkg/util/system/system_utils.go +++ b/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 } diff --git a/test/e2e/garbage_collector.go b/test/e2e/garbage_collector.go index e1a8ee5f69..ba43f7ff7a 100644 --- a/test/e2e/garbage_collector.go +++ b/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) } }