diff --git a/store/cache/cache.go b/store/cache/cache.go index cbaeaeb86..1d4054653 100644 --- a/store/cache/cache.go +++ b/store/cache/cache.go @@ -33,7 +33,7 @@ type ( // the same CommitKVStoreCache may be accessed concurrently by multiple // goroutines due to transaction parallelization - mtx sync.Mutex + mtx sync.RWMutex } // CommitKVStoreCacheManager maintains a mapping from a StoreKey to a @@ -102,27 +102,34 @@ func (ckv *CommitKVStoreCache) CacheWrap(storeKey types.StoreKey) types.CacheWra return cachekv.NewStore(ckv, storeKey, ckv.cacheKVSize) } +// getFromCache queries the write-through cache for a value by key. +func (ckv *CommitKVStoreCache) getFromCache(key []byte) ([]byte, bool) { + ckv.mtx.RLock() + defer ckv.mtx.RUnlock() + return ckv.cache.Get(string(key)) +} + +// getAndWriteToCache queries the underlying CommitKVStore and writes the result +func (ckv *CommitKVStoreCache) getAndWriteToCache(key []byte) []byte { + ckv.mtx.RLock() + defer ckv.mtx.RUnlock() + value := ckv.CommitKVStore.Get(key) + ckv.cache.Add(string(key), value) + return value +} + // Get retrieves a value by key. It will first look in the write-through cache. // If the value doesn't exist in the write-through cache, the query is delegated // to the underlying CommitKVStore. func (ckv *CommitKVStoreCache) Get(key []byte) []byte { - ckv.mtx.Lock() - defer ckv.mtx.Unlock() - types.AssertValidKey(key) - keyStr := string(key) - value, ok := ckv.cache.Get(keyStr) - if ok { - // cache hit + if value, ok := ckv.getFromCache(key); ok { return value } - // cache miss; write to cache - value = ckv.CommitKVStore.Get(key) - ckv.cache.Add(keyStr, value) - - return value + // if not found in the cache, query the underlying CommitKVStore and init cache value + return ckv.getAndWriteToCache(key) } // Set inserts a key/value pair into both the write-through cache and the diff --git a/store/cachekv/search_benchmark_test.go b/store/cachekv/search_benchmark_test.go deleted file mode 100644 index d31b0218f..000000000 --- a/store/cachekv/search_benchmark_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package cachekv - -import ( - "strconv" - "testing" - - "github.com/cosmos/cosmos-sdk/store/types" - db "github.com/tendermint/tm-db" -) - -func BenchmarkLargeUnsortedMisses(b *testing.B) { - for i := 0; i < b.N; i++ { - b.StopTimer() - store := generateStore() - b.StartTimer() - - for k := 0; k < 10000; k++ { - // cache has A + Z values - // these are within range, but match nothing - store.dirtyItems([]byte("B1"), []byte("B2")) - } - } -} - -func generateStore() *Store { - cache := types.NewBoundedCache(mapCacheBackend{make(map[string]*types.CValue)}, types.DefaultCacheSizeLimit) - unsorted := map[string]struct{}{} - for i := 0; i < 5000; i++ { - key := "A" + strconv.Itoa(i) - unsorted[key] = struct{}{} - cache.CacheBackend.Set(key, &types.CValue{}) - } - - for i := 0; i < 5000; i++ { - key := "Z" + strconv.Itoa(i) - unsorted[key] = struct{}{} - cache.CacheBackend.Set(key, &types.CValue{}) - } - - return &Store{ - cache: cache, - unsortedCache: unsorted, - sortedCache: db.NewMemDB(), - } -} diff --git a/store/cachekv/store.go b/store/cachekv/store.go index 59cb434b4..265c91a52 100644 --- a/store/cachekv/store.go +++ b/store/cachekv/store.go @@ -5,61 +5,23 @@ import ( "io" "sort" "sync" - "time" "github.com/cosmos/cosmos-sdk/internal/conv" "github.com/cosmos/cosmos-sdk/store/listenkv" "github.com/cosmos/cosmos-sdk/store/tracekv" "github.com/cosmos/cosmos-sdk/store/types" - "github.com/cosmos/cosmos-sdk/telemetry" sdktypes "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/kv" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/libs/math" dbm "github.com/tendermint/tm-db" ) -type mapCacheBackend struct { - m map[string]*types.CValue -} - -func (b mapCacheBackend) Get(key string) (val *types.CValue, ok bool) { - val, ok = b.m[key] - return -} - -func (b mapCacheBackend) Set(key string, val *types.CValue) { - b.m[key] = val -} - -func (b mapCacheBackend) Len() int { - return len(b.m) -} - -func (b mapCacheBackend) Delete(key string) { - delete(b.m, key) -} - -func (b mapCacheBackend) Range(f func(string, *types.CValue) bool) { - // this is always called within a mutex so all operations below are atomic - keys := []string{} - for k := range b.m { - keys = append(keys, k) - } - for _, key := range keys { - val, _ := b.Get(key) - if !f(key, val) { - break - } - } -} - // Store wraps an in-memory cache around an underlying types.KVStore. type Store struct { - mtx sync.Mutex - cache *types.BoundedCache + mtx sync.RWMutex + cache *sync.Map deleted *sync.Map - unsortedCache map[string]struct{} + unsortedCache *sync.Map sortedCache *dbm.MemDB // always ascending sorted parent types.KVStore eventManager *sdktypes.EventManager @@ -72,9 +34,9 @@ var _ types.CacheKVStore = (*Store)(nil) // NewStore creates a new Store object func NewStore(parent types.KVStore, storeKey types.StoreKey, cacheSize int) *Store { return &Store{ - cache: types.NewBoundedCache(mapCacheBackend{make(map[string]*types.CValue)}, cacheSize), + cache: &sync.Map{}, deleted: &sync.Map{}, - unsortedCache: make(map[string]struct{}), + unsortedCache: &sync.Map{}, sortedCache: dbm.NewMemDB(), parent: parent, eventManager: sdktypes.NewEventManager(), @@ -94,8 +56,6 @@ func (store *Store) GetEvents() []abci.Event { // Implements Store func (store *Store) ResetEvents() { - store.mtx.Lock() - defer store.mtx.Unlock() store.eventManager = sdktypes.NewEventManager() } @@ -104,76 +64,55 @@ func (store *Store) GetStoreType() types.StoreType { return store.parent.GetStoreType() } +// getFromCache queries the write-through cache for a value by key. +func (store *Store) getFromCache(key []byte) []byte { + if cv, ok := store.cache.Load(conv.UnsafeBytesToStr(key)); ok { + return cv.(*types.CValue).Value() + } + return store.parent.Get(key) +} + // Get implements types.KVStore. func (store *Store) Get(key []byte) (value []byte) { - store.mtx.Lock() - defer store.mtx.Unlock() - types.AssertValidKey(key) - - cacheValue, ok := store.cache.Get(conv.UnsafeBytesToStr(key)) - if !ok { - value = store.parent.Get(key) - store.setCacheValue(key, value, false, false) - } else { - value = cacheValue.Value() - } - store.eventManager.EmitResourceAccessReadEvent("get", store.storeKey, key, value) - - return value + return store.getFromCache(key) } // Set implements types.KVStore. func (store *Store) Set(key []byte, value []byte) { - store.mtx.Lock() - defer store.mtx.Unlock() - types.AssertValidKey(key) types.AssertValidValue(value) - store.setCacheValue(key, value, false, true) - store.eventManager.EmitResourceAccessWriteEvent("set", store.storeKey, key, value) } // Has implements types.KVStore. func (store *Store) Has(key []byte) bool { value := store.Get(key) - store.mtx.Lock() - defer store.mtx.Unlock() - store.eventManager.EmitResourceAccessReadEvent("has", store.storeKey, key, value) return value != nil } // Delete implements types.KVStore. func (store *Store) Delete(key []byte) { - store.mtx.Lock() - defer store.mtx.Unlock() - defer telemetry.MeasureSince(time.Now(), "store", "cachekv", "delete") - types.AssertValidKey(key) store.setCacheValue(key, nil, true, true) - store.eventManager.EmitResourceAccessWriteEvent("delete", store.storeKey, key, []byte{}) } // Implements Cachetypes.KVStore. func (store *Store) Write() { store.mtx.Lock() defer store.mtx.Unlock() - defer telemetry.MeasureSince(time.Now(), "store", "cachekv", "write") // We need a copy of all of the keys. // Not the best, but probably not a bottleneck depending. - keys := make([]string, 0, store.cache.Len()) + keys := []string{} - store.cache.Range(func(key string, dbValue *types.CValue) bool { - if dbValue.Dirty() { - keys = append(keys, key) + store.cache.Range(func(key, value any) bool { + if value.(*types.CValue).Dirty() { + keys = append(keys, key.(string)) } return true }) - sort.Strings(keys) - // TODO: Consider allowing usage of Batch, which would allow the write to // at least happen atomically. for _, key := range keys { @@ -186,24 +125,28 @@ func (store *Store) Write() { continue } - cacheValue, _ := store.cache.Get(key) - if cacheValue.Value() != nil { + cacheValue, ok := store.cache.Load(key) + if ok && cacheValue.(*types.CValue).Value() != nil { // It already exists in the parent, hence delete it. - store.parent.Set([]byte(key), cacheValue.Value()) + store.parent.Set([]byte(key), cacheValue.(*types.CValue).Value()) } } // Clear the cache using the map clearing idiom // and not allocating fresh objects. // Please see https://bencher.orijtech.com/perfclinic/mapclearing/ - store.cache.DeleteAll() + store.cache.Range(func(key, value any) bool { + store.cache.Delete(key) + return true + }) store.deleted.Range(func(key, value any) bool { store.deleted.Delete(key) return true }) - for key := range store.unsortedCache { - delete(store.unsortedCache, key) - } + store.unsortedCache.Range(func(key, value any) bool { + store.deleted.Delete(key) + return true + }) store.sortedCache = dbm.NewMemDB() } @@ -238,6 +181,7 @@ func (store *Store) ReverseIterator(start, end []byte) types.Iterator { func (store *Store) iterator(start, end []byte, ascending bool) types.Iterator { store.mtx.Lock() defer store.mtx.Unlock() + // TODO: (occ) Note that for iterators, we'll need to have special handling (discussed in RFC) to ensure proper validation var parent, cache types.Iterator @@ -350,77 +294,26 @@ func (store *Store) dirtyItems(start, end []byte) { return } - n := len(store.unsortedCache) unsorted := make([]*kv.Pair, 0) // If the unsortedCache is too big, its costs too much to determine - // whats in the subset we are concerned about. + // what's in the subset we are concerned about. // If you are interleaving iterator calls with writes, this can easily become an // O(N^2) overhead. // Even without that, too many range checks eventually becomes more expensive // than just not having the cache. - store.emitUnsortedCacheSizeMetric() - if n < minSortSize { - for key := range store.unsortedCache { - if dbm.IsKeyInDomain(conv.UnsafeStrToBytes(key), start, end) { - cacheValue, _ := store.cache.Get(key) - unsorted = append(unsorted, &kv.Pair{Key: []byte(key), Value: cacheValue.Value()}) + // store.emitUnsortedCacheSizeMetric() + store.unsortedCache.Range(func(key, value any) bool { + cKey := key.(string) + if dbm.IsKeyInDomain(conv.UnsafeStrToBytes(cKey), start, end) { + cacheValue, ok := store.cache.Load(key) + if ok { + unsorted = append(unsorted, &kv.Pair{Key: []byte(cKey), Value: cacheValue.(*types.CValue).Value()}) } } - store.clearUnsortedCacheSubset(unsorted, stateUnsorted) - return - } - - // Otherwise it is large so perform a modified binary search to find - // the target ranges for the keys that we should be looking for. - strL := make([]string, 0, n) - for key := range store.unsortedCache { - strL = append(strL, key) - } - sort.Strings(strL) - - startIndex, endIndex := findStartEndIndex(strL, startStr, endStr) - - // Since we spent cycles to sort the values, we should process and remove a reasonable amount - // ensure start to end is at least minSortSize in size - // if below minSortSize, expand it to cover additional values - // this amortizes the cost of processing elements across multiple calls - if endIndex-startIndex < minSortSize { - endIndex = math.MinInt(startIndex+minSortSize, len(strL)-1) - if endIndex-startIndex < minSortSize { - startIndex = math.MaxInt(endIndex-minSortSize, 0) - } - } - - kvL := make([]*kv.Pair, 0, 1+endIndex-startIndex) - for i := startIndex; i <= endIndex; i++ { - key := strL[i] - cacheValue, _ := store.cache.Get(key) - kvL = append(kvL, &kv.Pair{Key: []byte(key), Value: cacheValue.Value()}) - } - - // kvL was already sorted so pass it in as is. - store.clearUnsortedCacheSubset(kvL, stateAlreadySorted) - store.emitUnsortedCacheSizeMetric() -} - -func (store *Store) emitUnsortedCacheSizeMetric() { - n := len(store.unsortedCache) - telemetry.SetGauge(float32(n), "sei", "cosmos", "unsorted", "cache", "size") -} - -func findStartEndIndex(strL []string, startStr, endStr string) (int, int) { - // Now find the values within the domain - // [start, end) - startIndex := findStartIndex(strL, startStr) - endIndex := findEndIndex(strL, endStr) - - if endIndex < 0 { - endIndex = len(strL) - 1 - } - if startIndex < 0 { - startIndex = 0 - } - return startIndex, endIndex + return true + }) + store.clearUnsortedCacheSubset(unsorted, stateUnsorted) + return } func (store *Store) clearUnsortedCacheSubset(unsorted []*kv.Pair, sortState sortState) { @@ -449,18 +342,10 @@ func (store *Store) clearUnsortedCacheSubset(unsorted []*kv.Pair, sortState sort } func (store *Store) deleteKeysFromUnsortedCache(unsorted []*kv.Pair) { - n := len(store.unsortedCache) - store.emitUnsortedCacheSizeMetric() - if len(unsorted) == n { // This pattern allows the Go compiler to emit the map clearing idiom for the entire map. - for key := range store.unsortedCache { - delete(store.unsortedCache, key) - } - } else { // Otherwise, normally delete the unsorted keys from the map. - for _, kv := range unsorted { - delete(store.unsortedCache, conv.UnsafeBytesToStr(kv.Key)) - } + for _, kv := range unsorted { + keyStr := conv.UnsafeBytesToStr(kv.Key) + store.unsortedCache.Delete(keyStr) } - defer store.emitUnsortedCacheSizeMetric() } //---------------------------------------- @@ -471,14 +356,14 @@ func (store *Store) setCacheValue(key, value []byte, deleted bool, dirty bool) { types.AssertValidKey(key) keyStr := conv.UnsafeBytesToStr(key) - store.cache.Set(keyStr, types.NewCValue(value, dirty)) + store.cache.Store(keyStr, types.NewCValue(value, dirty)) if deleted { store.deleted.Store(keyStr, struct{}{}) } else { store.deleted.Delete(keyStr) } if dirty { - store.unsortedCache[keyStr] = struct{}{} + store.unsortedCache.Store(keyStr, struct{}{}) } }