From 7e15e31e11e48a6db855e30ca9b07dbce3047577 Mon Sep 17 00:00:00 2001 From: Quan Tian Date: Fri, 27 Mar 2020 18:39:20 +0800 Subject: [PATCH] Improve fake clientset performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fake clientset used a slice to store each kind of objects, it's quite slow to init the clientset with massive objects because it checked existence of an object by traversing all objects before adding it, which leads to O(n^2) time complexity. Also, the Create, Update, Get, Delete methods needs to traverse all objects, which affects the time statistic of code that calls them. This patch changed to use a map to store each kind of objects, reduced the time complexity of initializing clientset to O(n) and the Create, Update, Get, Delete to O(1). For example: Before this patch, it took ~29s to init a clientset with 30000 Pods, and 2~4ms to create and get an Pod. After this patch, it took ~50ms to init a clientset with 30000 Pods, and tens of µs to create and get an Pod. --- .../client-go/dynamic/fake/simple_test.go | 2 +- .../client-go/metadata/fake/simple_test.go | 2 +- .../src/k8s.io/client-go/testing/fixture.go | 97 ++++++++----------- 3 files changed, 44 insertions(+), 57 deletions(-) diff --git a/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go b/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go index adefdf7c8bc..33037c3c597 100644 --- a/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go +++ b/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go @@ -75,9 +75,9 @@ func TestList(t *testing.T) { } expected := []unstructured.Unstructured{ - *newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), *newUnstructured("group/version", "TheKind", "ns-foo", "name-bar"), *newUnstructured("group/version", "TheKind", "ns-foo", "name-baz"), + *newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), } if !equality.Semantic.DeepEqual(listFirst.Items, expected) { t.Fatal(diff.ObjectGoPrintDiff(expected, listFirst.Items)) diff --git a/staging/src/k8s.io/client-go/metadata/fake/simple_test.go b/staging/src/k8s.io/client-go/metadata/fake/simple_test.go index 641fd55b79f..e6fdde3ebc4 100644 --- a/staging/src/k8s.io/client-go/metadata/fake/simple_test.go +++ b/staging/src/k8s.io/client-go/metadata/fake/simple_test.go @@ -79,9 +79,9 @@ func TestList(t *testing.T) { } expected := []metav1.PartialObjectMetadata{ - *newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-foo"), *newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-bar"), *newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-baz"), + *newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-foo"), } if !equality.Semantic.DeepEqual(listFirst.Items, expected) { t.Fatal(diff.ObjectGoPrintDiff(expected, listFirst.Items)) diff --git a/staging/src/k8s.io/client-go/testing/fixture.go b/staging/src/k8s.io/client-go/testing/fixture.go index 54f600ad3f7..d3b937247b2 100644 --- a/staging/src/k8s.io/client-go/testing/fixture.go +++ b/staging/src/k8s.io/client-go/testing/fixture.go @@ -19,6 +19,7 @@ package testing import ( "fmt" "reflect" + "sort" "sync" jsonpatch "github.com/evanphx/json-patch" @@ -197,7 +198,7 @@ type tracker struct { scheme ObjectScheme decoder runtime.Decoder lock sync.RWMutex - objects map[schema.GroupVersionResource][]runtime.Object + objects map[schema.GroupVersionResource]map[types.NamespacedName]runtime.Object // The value type of watchers is a map of which the key is either a namespace or // all/non namespace aka "" and its value is list of fake watchers. // Manipulations on resources will broadcast the notification events into the @@ -214,7 +215,7 @@ func NewObjectTracker(scheme ObjectScheme, decoder runtime.Decoder) ObjectTracke return &tracker{ scheme: scheme, decoder: decoder, - objects: make(map[schema.GroupVersionResource][]runtime.Object), + objects: make(map[schema.GroupVersionResource]map[types.NamespacedName]runtime.Object), watchers: make(map[schema.GroupVersionResource]map[string][]*watch.RaceFreeFakeWatcher), } } @@ -282,31 +283,15 @@ func (t *tracker) Get(gvr schema.GroupVersionResource, ns, name string) (runtime return nil, errNotFound } - var matchingObjs []runtime.Object - for _, obj := range objs { - acc, err := meta.Accessor(obj) - if err != nil { - return nil, err - } - if acc.GetNamespace() != ns { - continue - } - if acc.GetName() != name { - continue - } - matchingObjs = append(matchingObjs, obj) - } - if len(matchingObjs) == 0 { + matchingObj, ok := objs[types.NamespacedName{Namespace: ns, Name: name}] + if !ok { return nil, errNotFound } - if len(matchingObjs) > 1 { - return nil, fmt.Errorf("more than one object matched gvr %s, ns: %q name: %q", gvr, ns, name) - } // Only one object should match in the tracker if it works // correctly, as Add/Update methods enforce kind/namespace/name // uniqueness. - obj := matchingObjs[0].DeepCopyObject() + obj := matchingObj.DeepCopyObject() if status, ok := obj.(*metav1.Status); ok { if status.Status != metav1.StatusSuccess { return nil, &errors.StatusError{ErrStatus: *status} @@ -405,21 +390,21 @@ func (t *tracker) add(gvr schema.GroupVersionResource, obj runtime.Object, ns st return errors.NewBadRequest(msg) } - for i, existingObj := range t.objects[gvr] { - oldMeta, err := meta.Accessor(existingObj) - if err != nil { - return err - } - if oldMeta.GetNamespace() == newMeta.GetNamespace() && oldMeta.GetName() == newMeta.GetName() { - if replaceExisting { - for _, w := range t.getWatches(gvr, ns) { - w.Modify(obj) - } - t.objects[gvr][i] = obj - return nil + _, ok := t.objects[gvr] + if !ok { + t.objects[gvr] = make(map[types.NamespacedName]runtime.Object) + } + + namespacedName := types.NamespacedName{Namespace: newMeta.GetNamespace(), Name: newMeta.GetName()} + if _, ok = t.objects[gvr][namespacedName]; ok { + if replaceExisting { + for _, w := range t.getWatches(gvr, ns) { + w.Modify(obj) } - return errors.NewAlreadyExists(gr, newMeta.GetName()) + t.objects[gvr][namespacedName] = obj + return nil } + return errors.NewAlreadyExists(gr, newMeta.GetName()) } if replaceExisting { @@ -427,7 +412,7 @@ func (t *tracker) add(gvr schema.GroupVersionResource, obj runtime.Object, ns st return errors.NewNotFound(gr, newMeta.GetName()) } - t.objects[gvr] = append(t.objects[gvr], obj) + t.objects[gvr][namespacedName] = obj for _, w := range t.getWatches(gvr, ns) { w.Add(obj) @@ -457,35 +442,28 @@ func (t *tracker) Delete(gvr schema.GroupVersionResource, ns, name string) error t.lock.Lock() defer t.lock.Unlock() - found := false - - for i, existingObj := range t.objects[gvr] { - objMeta, err := meta.Accessor(existingObj) - if err != nil { - return err - } - if objMeta.GetNamespace() == ns && objMeta.GetName() == name { - obj := t.objects[gvr][i] - t.objects[gvr] = append(t.objects[gvr][:i], t.objects[gvr][i+1:]...) - for _, w := range t.getWatches(gvr, ns) { - w.Delete(obj) - } - found = true - break - } + objs, ok := t.objects[gvr] + if !ok { + return errors.NewNotFound(gvr.GroupResource(), name) } - if found { - return nil + namespacedName := types.NamespacedName{Namespace: ns, Name: name} + obj, ok := objs[namespacedName] + if !ok { + return errors.NewNotFound(gvr.GroupResource(), name) } - return errors.NewNotFound(gvr.GroupResource(), name) + delete(objs, namespacedName) + for _, w := range t.getWatches(gvr, ns) { + w.Delete(obj) + } + return nil } // filterByNamespace returns all objects in the collection that // match provided namespace. Empty namespace matches // non-namespaced objects. -func filterByNamespace(objs []runtime.Object, ns string) ([]runtime.Object, error) { +func filterByNamespace(objs map[types.NamespacedName]runtime.Object, ns string) ([]runtime.Object, error) { var res []runtime.Object for _, obj := range objs { @@ -499,6 +477,15 @@ func filterByNamespace(objs []runtime.Object, ns string) ([]runtime.Object, erro res = append(res, obj) } + // Sort res to get deterministic order. + sort.Slice(res, func(i, j int) bool { + acc1, _ := meta.Accessor(res[i]) + acc2, _ := meta.Accessor(res[j]) + if acc1.GetNamespace() != acc2.GetNamespace() { + return acc1.GetNamespace() < acc2.GetNamespace() + } + return acc1.GetName() < acc2.GetName() + }) return res, nil }