Skip to content

Commit

Permalink
Olympus Mons Cherry-Picks (#2)
Browse files Browse the repository at this point in the history
* IAVL

* perf: store/cachekv: Correct the naming and implementation for existing benchmarks (cosmos#10116)

## Description

Cref discussion in cosmos#10026, updates the CacheKV Benchmark naming and implementation to correspond to whats actually going on, and remove many irrelevant/incorrect components from being in the benchmarks timing.

Basically the old Benchmark's iterator creation was very flawed, and never hit the complex case, only repeatedly performing the best case performance of the iterator. Instead it really benchmarked iterator set and next operations.

This PR splits out the benchmarks for those two accordingly.

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md` - N/A imo
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [x] confirmed all CI checks have passed - lint failure unrelated

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

* store cosmos#10486

* supply cosmos#10393

* supply cosmos#10393

* telemetry cosmos#10334

* module account

Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
Co-authored-by: likhita-809 <78951027+likhita-809@users.noreply.github.com>
  • Loading branch information
5 people authored Nov 15, 2021
1 parent e443fb7 commit 8692c10
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 265 deletions.
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,13 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Bug Fixes
### Improvements

* [\#10414](https://github.com/cosmos/cosmos-sdk/pull/10414) Use `sdk.GetConfig().GetFullBIP44Path()` instead `sdk.FullFundraiserPath` to generate key
* [\#10077](https://github.com/cosmos/cosmos-sdk/pull/10077),[\#10334](https://github.com/cosmos/cosmos-sdk/pull/10334) Remove telemetry on `GasKV` and `CacheKV` store operations, significantly improving their performance.
* [\#10486](https://github.com/cosmos/cosmos-sdk/pull/10486) store/cachekv's `Store.Write` conservatively looks up keys, but also uses the [map clearing idiom](https://bencher.orijtech.com/perfclinic/mapclearing/) to reduce the RAM usage, CPU time usage, and garbage collection pressure from clearing maps, instead of allocating new maps.
* [\#10393](https://github.com/cosmos/cosmos-sdk/pull/"10393") Add `HasSupply` method to bank keeper to ensure that input denom actually exists on chain.

### Bug Fixes

## [v0.44.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.44.3) - 2021-10-21

Expand Down
8 changes: 0 additions & 8 deletions docs/core/telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,6 @@ The following examples expose too much cardinality and may not even prove to be
| `store_iavl_delete` | Duration of an IAVL `Store#Delete` call | ms | summary |
| `store_iavl_commit` | Duration of an IAVL `Store#Commit` call | ms | summary |
| `store_iavl_query` | Duration of an IAVL `Store#Query` call | ms | summary |
| `store_gaskv_get` | Duration of a GasKV `Store#Get` call | ms | summary |
| `store_gaskv_set` | Duration of a GasKV `Store#Set` call | ms | summary |
| `store_gaskv_has` | Duration of a GasKV `Store#Has` call | ms | summary |
| `store_gaskv_delete` | Duration of a GasKV `Store#Delete` call | ms | summary |
| `store_cachekv_get` | Duration of a CacheKV `Store#Get` call | ms | summary |
| `store_cachekv_set` | Duration of a CacheKV `Store#Set` call | ms | summary |
| `store_cachekv_write` | Duration of a CacheKV `Store#Write` call | ms | summary |
| `store_cachekv_delete` | Duration of a CacheKV `Store#Delete` call | ms | summary |

## Next {hide}

Expand Down
12 changes: 2 additions & 10 deletions docs/ru/readme.md
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
---
parent:
order: false
---
# Cosmos SDK Documentation (Russian)

# RU

::: warning
**DEPRECATED**
This documentation is not complete and it's outdated. Please use the English version.
:::
A Russian translation of the Cosmos SDK documentation is not available for this version. If you would like to help with translating, please see [Internationalization](https://github.com/cosmos/cosmos-sdk/blob/master/docs/DOCS_README.md#internationalization). A `v0.39` version of the documentation can be found [here](https://github.com/cosmos/cosmos-sdk/tree/v0.39.3/docs/ru).
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/coinbase/rosetta-sdk-go v0.6.10
github.com/confio/ics23/go v0.6.6
github.com/cosmos/go-bip39 v1.0.0
github.com/cosmos/iavl v0.17.1
github.com/cosmos/iavl v0.17.2
github.com/cosmos/ledger-cosmos-go v0.11.1
github.com/enigmampc/btcutil v1.0.3-0.20200723161021-e2fb6adb2a25
github.com/gogo/gateway v1.1.0
Expand Down Expand Up @@ -49,9 +49,9 @@ require (
github.com/tendermint/go-amino v0.16.0
github.com/tendermint/tendermint v0.34.14
github.com/tendermint/tm-db v0.6.4
golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a
google.golang.org/genproto v0.0.0-20210602131652-f16073e35f0c
google.golang.org/grpc v1.40.0
golang.org/x/crypto v0.0.0-20210817164053-32db794688a5
google.golang.org/genproto v0.0.0-20210828152312-66f60bf46e71
google.golang.org/grpc v1.42.0
google.golang.org/protobuf v1.27.1
gopkg.in/yaml.v2 v2.4.0
)
Expand Down
44 changes: 10 additions & 34 deletions go.sum

Large diffs are not rendered by default.

44 changes: 44 additions & 0 deletions store/cachekv/bench_helper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package cachekv_test

import "crypto/rand"

func randSlice(sliceSize int) []byte {
bz := make([]byte, sliceSize)
_, _ = rand.Read(bz)
return bz
}

func incrementByteSlice(bz []byte) {
for index := len(bz) - 1; index >= 0; index-- {
if bz[index] < 255 {
bz[index]++
break
} else {
bz[index] = 0
}
}
}

// Generate many keys starting at startKey, and are in sequential order
func generateSequentialKeys(startKey []byte, numKeys int) [][]byte {
toReturn := make([][]byte, 0, numKeys)
cur := make([]byte, len(startKey))
copy(cur, startKey)
for i := 0; i < numKeys; i++ {
newKey := make([]byte, len(startKey))
copy(newKey, cur)
toReturn = append(toReturn, newKey)
incrementByteSlice(cur)
}
return toReturn
}

// Generate many random, unsorted keys
func generateRandomKeys(keySize int, numKeys int) [][]byte {
toReturn := make([][]byte, 0, numKeys)
for i := 0; i < numKeys; i++ {
newKey := randSlice(keySize)
toReturn = append(toReturn, newKey)
}
return toReturn
}
38 changes: 23 additions & 15 deletions store/cachekv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@ import (
"io"
"sort"
"sync"
"time"

dbm "github.com/tendermint/tm-db"

"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"
"github.com/cosmos/cosmos-sdk/types/kv"
)

Expand Down Expand Up @@ -91,7 +89,6 @@ func (store *Store) Has(key []byte) bool {
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)
Expand All @@ -101,7 +98,6 @@ func (store *Store) Delete(key []byte) {
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.
Expand All @@ -118,22 +114,34 @@ func (store *Store) Write() {
// TODO: Consider allowing usage of Batch, which would allow the write to
// at least happen atomically.
for _, key := range keys {
cacheValue := store.cache[key]

switch {
case store.isDeleted(key):
if store.isDeleted(key) {
// We use []byte(key) instead of conv.UnsafeStrToBytes because we cannot
// be sure if the underlying store might do a save with the byteslice or
// not. Once we get confirmation that .Delete is guaranteed not to
// save the byteslice, then we can assume only a read-only copy is sufficient.
store.parent.Delete([]byte(key))
case cacheValue.value == nil:
// Skip, it already doesn't exist in parent.
default:
continue
}

cacheValue := store.cache[key]
if cacheValue.value != nil {
// It already exists in the parent, hence delete it.
store.parent.Set([]byte(key), cacheValue.value)
}
}

// Clear the cache
store.cache = make(map[string]*cValue)
store.deleted = make(map[string]struct{})
store.unsortedCache = make(map[string]struct{})
// Clear the cache using the map clearing idiom
// and not allocating fresh objects.
// Please see https://bencher.orijtech.com/perfclinic/mapclearing/
for key := range store.cache {
delete(store.cache, key)
}
for key := range store.deleted {
delete(store.deleted, key)
}
for key := range store.unsortedCache {
delete(store.unsortedCache, key)
}
store.sortedCache = dbm.NewMemDB()
}

Expand Down
92 changes: 69 additions & 23 deletions store/cachekv/store_bench_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package cachekv_test

import (
"crypto/rand"
"sort"
"testing"

dbm "github.com/tendermint/tm-db"
Expand All @@ -11,37 +9,85 @@ import (
"github.com/cosmos/cosmos-sdk/store/dbadapter"
)

func benchmarkCacheKVStoreIterator(numKVs int, b *testing.B) {
var sink interface{}

const defaultValueSizeBz = 1 << 12

// This benchmark measures the time of iterator.Next() when the parent store is blank
func benchmarkBlankParentIteratorNext(b *testing.B, keysize int) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
kvstore := cachekv.NewStore(mem)
// Use a singleton for value, to not waste time computing it
value := randSlice(defaultValueSizeBz)
// Use simple values for keys, pick a random start,
// and take next b.N keys sequentially after.]
startKey := randSlice(32)

// Add 1 to avoid issues when b.N = 1
keys := generateSequentialKeys(startKey, b.N+1)
for _, k := range keys {
kvstore.Set(k, value)
}

b.ReportAllocs()
b.ResetTimer()

iter := kvstore.Iterator(keys[0], keys[b.N])
defer iter.Close()

for _ = iter.Key(); iter.Valid(); iter.Next() {
// deadcode elimination stub
sink = iter
}
}

// Benchmark setting New keys to a store, where the new keys are in sequence.
func benchmarkBlankParentAppend(b *testing.B, keysize int) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
cstore := cachekv.NewStore(mem)
keys := make([]string, numKVs)
kvstore := cachekv.NewStore(mem)

for i := 0; i < numKVs; i++ {
key := make([]byte, 32)
value := make([]byte, 32)
// Use a singleton for value, to not waste time computing it
value := randSlice(32)
// Use simple values for keys, pick a random start,
// and take next b.N keys sequentially after.
startKey := randSlice(32)

_, _ = rand.Read(key)
_, _ = rand.Read(value)
keys := generateSequentialKeys(startKey, b.N)

keys[i] = string(key)
cstore.Set(key, value)
b.ReportAllocs()
b.ResetTimer()

for _, k := range keys {
kvstore.Set(k, value)
}
}

sort.Strings(keys)
// Benchmark setting New keys to a store, where the new keys are random.
// the speed of this function does not depend on the values in the parent store
func benchmarkRandomSet(b *testing.B, keysize int) {
mem := dbadapter.Store{DB: dbm.NewMemDB()}
kvstore := cachekv.NewStore(mem)

for n := 0; n < b.N; n++ {
iter := cstore.Iterator([]byte(keys[0]), []byte(keys[numKVs-1]))
// Use a singleton for value, to not waste time computing it
value := randSlice(defaultValueSizeBz)
keys := generateRandomKeys(keysize, b.N)

for _ = iter.Key(); iter.Valid(); iter.Next() {
}
b.ReportAllocs()
b.ResetTimer()

iter.Close()
for _, k := range keys {
kvstore.Set(k, value)
}
}

func BenchmarkCacheKVStoreIterator500(b *testing.B) { benchmarkCacheKVStoreIterator(500, b) }
func BenchmarkCacheKVStoreIterator1000(b *testing.B) { benchmarkCacheKVStoreIterator(1000, b) }
func BenchmarkCacheKVStoreIterator10000(b *testing.B) { benchmarkCacheKVStoreIterator(10000, b) }
func BenchmarkCacheKVStoreIterator50000(b *testing.B) { benchmarkCacheKVStoreIterator(50000, b) }
func BenchmarkCacheKVStoreIterator100000(b *testing.B) { benchmarkCacheKVStoreIterator(100000, b) }
func BenchmarkBlankParentIteratorNextKeySize32(b *testing.B) {
benchmarkBlankParentIteratorNext(b, 32)
}

func BenchmarkBlankParentAppendKeySize32(b *testing.B) {
benchmarkBlankParentAppend(b, 32)
}

func BenchmarkSetKeySize32(b *testing.B) {
benchmarkRandomSet(b, 32)
}
4 changes: 0 additions & 4 deletions store/gaskv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package gaskv

import (
"io"
"time"

"github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/telemetry"
)

var _ types.KVStore = &Store{}
Expand Down Expand Up @@ -56,14 +54,12 @@ func (gs *Store) Set(key []byte, value []byte) {

// Implements KVStore.
func (gs *Store) Has(key []byte) bool {
defer telemetry.MeasureSince(time.Now(), "store", "gaskv", "has")
gs.gasMeter.ConsumeGas(gs.gasConfig.HasCost, types.GasHasDesc)
return gs.parent.Has(key)
}

// Implements KVStore.
func (gs *Store) Delete(key []byte) {
defer telemetry.MeasureSince(time.Now(), "store", "gaskv", "delete")
// charge gas to prevent certain attack vectors even though space is being freed
gs.gasMeter.ConsumeGas(gs.gasConfig.DeleteCost, types.GasDeleteDesc)
gs.parent.Delete(key)
Expand Down
Loading

0 comments on commit 8692c10

Please sign in to comment.