From d36c6be529c79c6d6288f2735fe5f1371b6015fc Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 13 Jul 2020 03:49:09 -0700 Subject: [PATCH] store/cache: speed up + conserve memory by using map clearing idiom in Reset (#6700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Noticed during my audit, the code for cache.CommitKVStoreCacheManager.Reset() discarded the prior on every invocation i.e. m = make(map[T(key)]T(value)) However, this can be made fast and conserve memory by using the map clearing idiom that the Go compiler recognizes i.e. for key := range m { delete(m, key) } and turns into very fast code, instead of the extraneous map discarding. The speed up generated is: ```shell $ benchstat before after name old time/op new time/op delta Reset-8 204ns ± 2% 66ns ± 9% -67.86% (p=0.000 n=20+20) name old alloc/op new alloc/op delta Reset-8 384B ± 0% 0B -100.00% (p=0.000 n=20+20) name old allocs/op new allocs/op delta Reset-8 3.00 ± 0% 0.00 -100.00% (p=0.000 n=20+20) ``` Fixes #6681 --- store/cache/benchmark_test.go | 48 +++++++++++++++++++++++++++++++++++ store/cache/cache.go | 7 ++++- 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 store/cache/benchmark_test.go diff --git a/store/cache/benchmark_test.go b/store/cache/benchmark_test.go new file mode 100644 index 0000000000..cf8206272c --- /dev/null +++ b/store/cache/benchmark_test.go @@ -0,0 +1,48 @@ +package cache + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/store/types" +) + +func freshMgr() *CommitKVStoreCacheManager { + return &CommitKVStoreCacheManager{ + caches: map[string]types.CommitKVStore{ + "a1": nil, + "alalalalalal": nil, + }, + } +} + +func populate(mgr *CommitKVStoreCacheManager) { + mgr.caches["this one"] = (types.CommitKVStore)(nil) + mgr.caches["those ones are the ones"] = (types.CommitKVStore)(nil) + mgr.caches["very huge key right here and there are we going to ones are the ones"] = (types.CommitKVStore)(nil) +} + +func BenchmarkReset(b *testing.B) { + mgr := freshMgr() + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + mgr.Reset() + if len(mgr.caches) != 0 { + b.Fatal("Reset failed") + } + populate(mgr) + if len(mgr.caches) == 0 { + b.Fatal("populate failed") + } + mgr.Reset() + if len(mgr.caches) != 0 { + b.Fatal("Reset failed") + } + } + + if mgr == nil { + b.Fatal("Impossible condition") + } +} diff --git a/store/cache/cache.go b/store/cache/cache.go index 9eea90ad94..b58f0531fc 100644 --- a/store/cache/cache.go +++ b/store/cache/cache.go @@ -81,7 +81,12 @@ func (cmgr *CommitKVStoreCacheManager) Unwrap(key types.StoreKey) types.CommitKV // Reset resets in the internal caches. func (cmgr *CommitKVStoreCacheManager) Reset() { - cmgr.caches = make(map[string]types.CommitKVStore) + // Clear the map. + // Please note that we are purposefully using the map clearing idiom. + // See https://github.com/cosmos/cosmos-sdk/issues/6681. + for key := range cmgr.caches { + delete(cmgr.caches, key) + } } // CacheWrap returns the inter-block cache as a cache-wrapped CommitKVStore.