From 2b17666b447b85365df40a92132524fa87fc3100 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Wed, 14 Oct 2015 13:17:00 +0200 Subject: [PATCH] Avoid unnecessary copies in LIST --- pkg/storage/etcd/etcd_helper.go | 13 +++++++++---- pkg/storage/etcd/etcd_watcher.go | 2 +- pkg/storage/etcd/etcd_watcher_test.go | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/storage/etcd/etcd_helper.go b/pkg/storage/etcd/etcd_helper.go index ab771250e56..2767e47e84b 100644 --- a/pkg/storage/etcd/etcd_helper.go +++ b/pkg/storage/etcd/etcd_helper.go @@ -323,8 +323,9 @@ func (h *etcdHelper) decodeNodeList(nodes []*etcd.Node, filter storage.FilterFun trace.Step("Decoding dir " + node.Key + " END") continue } - if obj, found := h.getFromCache(node.ModifiedIndex); found { - if filter(obj) { + if obj, found := h.getFromCache(node.ModifiedIndex, filter); found { + // obj != nil iff it matches the filter function. + if obj != nil { v.Set(reflect.Append(v, reflect.ValueOf(obj).Elem())) } } else { @@ -498,7 +499,7 @@ func (h *etcdHelper) prefixEtcdKey(key string) string { // their Node.ModifiedIndex, which is unique across all types. // All implementations must be thread-safe. type etcdCache interface { - getFromCache(index uint64) (runtime.Object, bool) + getFromCache(index uint64, filter storage.FilterFunc) (runtime.Object, bool) addToCache(index uint64, obj runtime.Object) } @@ -508,18 +509,22 @@ func getTypeName(obj interface{}) string { return reflect.TypeOf(obj).String() } -func (h *etcdHelper) getFromCache(index uint64) (runtime.Object, bool) { +func (h *etcdHelper) getFromCache(index uint64, filter storage.FilterFunc) (runtime.Object, bool) { startTime := time.Now() defer func() { metrics.ObserveGetCache(startTime) }() obj, found := h.cache.Get(index) if found { + if !filter(obj.(runtime.Object)) { + return nil, true + } // We should not return the object itself to avoid polluting the cache if someone // modifies returned values. objCopy, err := h.copier.Copy(obj.(runtime.Object)) if err != nil { glog.Errorf("Error during DeepCopy of cached object: %q", err) + // We can't return a copy, thus we report the object as not found. return nil, false } metrics.ObserveCacheHit() diff --git a/pkg/storage/etcd/etcd_watcher.go b/pkg/storage/etcd/etcd_watcher.go index 2c94ada2564..9dae526586d 100644 --- a/pkg/storage/etcd/etcd_watcher.go +++ b/pkg/storage/etcd/etcd_watcher.go @@ -208,7 +208,7 @@ func (w *etcdWatcher) translate() { } func (w *etcdWatcher) decodeObject(node *etcd.Node) (runtime.Object, error) { - if obj, found := w.cache.getFromCache(node.ModifiedIndex); found { + if obj, found := w.cache.getFromCache(node.ModifiedIndex, storage.Everything); found { return obj, nil } diff --git a/pkg/storage/etcd/etcd_watcher_test.go b/pkg/storage/etcd/etcd_watcher_test.go index 1d197f6983b..3f95b864f2e 100644 --- a/pkg/storage/etcd/etcd_watcher_test.go +++ b/pkg/storage/etcd/etcd_watcher_test.go @@ -40,7 +40,7 @@ var versioner = APIObjectVersioner{} // Implements etcdCache interface as empty methods (i.e. does not cache any objects) type fakeEtcdCache struct{} -func (f *fakeEtcdCache) getFromCache(index uint64) (runtime.Object, bool) { +func (f *fakeEtcdCache) getFromCache(index uint64, filter storage.FilterFunc) (runtime.Object, bool) { return nil, false }