From a991a2e1c4b328cb70af86fb5b794a1967813c5a Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 31 Aug 2018 00:37:25 -0700 Subject: [PATCH 1/4] Improve GetValidator speed In simulation, this was shown to cause a 4x speedup. There are no safety concerns here, as amino encoding is deterministic. --- PENDING.md | 2 +- x/stake/keeper/validator.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index 0aef9fd193..5557a8771e 100644 --- a/PENDING.md +++ b/PENDING.md @@ -78,7 +78,7 @@ IMPROVEMENTS * [x/stake] [#2023](https://github.com/cosmos/cosmos-sdk/pull/2023) Terminate iteration loop in `UpdateBondedValidators` and `UpdateBondedValidatorsFull` when the first revoked validator is encountered and perform a sanity check. * [x/auth] Signature verification's gas cost now accounts for pubkey type. [#2046](https://github.com/tendermint/tendermint/pull/2046) * [x/stake] [x/slashing] Ensure delegation invariants to jailed validators [#1883](https://github.com/cosmos/cosmos-sdk/issues/1883). - + * [x/stake] Improve speed of GetValidator, which was shown to be a performance bottleneck. [#2046](https://github.com/tendermint/tendermint/pull/2200) * SDK * [tools] Make get_vendor_deps deletes `.vendor-new` directories, in case scratch files are present. * [cli] \#1632 Add integration tests to ensure `basecoind init && basecoind` start sequences run successfully for both `democoin` and `basecoin` examples. diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 0ea24e6396..5dc075742e 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -2,6 +2,7 @@ package keeper import ( "bytes" + "container/list" "fmt" abci "github.com/tendermint/tendermint/abci/types" @@ -11,6 +12,14 @@ import ( "github.com/cosmos/cosmos-sdk/x/stake/types" ) +type cachedValidator struct { + val types.Validator + marshalled string +} + +var validatorCache = make(map[string]cachedValidator, 1000) +var validatorCacheList = list.New() + // get a single validator func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator types.Validator, found bool) { store := ctx.KVStore(k.storeKey) @@ -18,6 +27,23 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty if value == nil { return validator, false } + // return cached validator + strValue := string(value) + if val, ok := validatorCache[strValue]; ok { + valToReturn := val.val + // Doesn't mutate the cache's value + valToReturn.Operator = addr + return valToReturn, true + } else { // get validator from cache + validator = types.MustUnmarshalValidator(k.cdc, addr, value) + cachedVal := cachedValidator{validator, strValue} + validatorCache[strValue] = cachedValidator{validator, strValue} + validatorCacheList.PushBack(cachedVal) + if validatorCacheList.Len() > 500 { + valToRemove := validatorCacheList.Remove(validatorCacheList.Front()).(cachedValidator) + delete(validatorCache, valToRemove.marshalled) + } + } validator = types.MustUnmarshalValidator(k.cdc, addr, value) return validator, true } From 3b4caa5dd2bf218f24fd53616a41aefea79c9f20 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 31 Aug 2018 08:29:32 -0700 Subject: [PATCH 2/4] fix lint --- x/stake/keeper/validator.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 5dc075742e..a12801dc9e 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -34,16 +34,17 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty // Doesn't mutate the cache's value valToReturn.Operator = addr return valToReturn, true - } else { // get validator from cache - validator = types.MustUnmarshalValidator(k.cdc, addr, value) - cachedVal := cachedValidator{validator, strValue} - validatorCache[strValue] = cachedValidator{validator, strValue} - validatorCacheList.PushBack(cachedVal) - if validatorCacheList.Len() > 500 { - valToRemove := validatorCacheList.Remove(validatorCacheList.Front()).(cachedValidator) - delete(validatorCache, valToRemove.marshalled) - } } + // get validator from cache + validator = types.MustUnmarshalValidator(k.cdc, addr, value) + cachedVal := cachedValidator{validator, strValue} + validatorCache[strValue] = cachedValidator{validator, strValue} + validatorCacheList.PushBack(cachedVal) + if validatorCacheList.Len() > 500 { + valToRemove := validatorCacheList.Remove(validatorCacheList.Front()).(cachedValidator) + delete(validatorCache, valToRemove.marshalled) + } + validator = types.MustUnmarshalValidator(k.cdc, addr, value) return validator, true } From f29fdcafddd84e1faf290305b840aca1cddd7ec2 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Sat, 1 Sep 2018 13:21:42 -0700 Subject: [PATCH 3/4] Add comments --- x/stake/keeper/validator.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index a12801dc9e..d5fba14337 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -12,12 +12,16 @@ import ( "github.com/cosmos/cosmos-sdk/x/stake/types" ) +// Cache the amino decoding of validators, as it can be the case that repeated slashing calls +// cause many calls to GetValidator, which were shown to throttle the state machine in our +// simulation. Note this is quite biased though, as the simulator does more slashes than a +// live chain should, however we require the slashing to be fast as noone pays gas for it. type cachedValidator struct { val types.Validator marshalled string } -var validatorCache = make(map[string]cachedValidator, 1000) +var validatorCache = make(map[string]cachedValidator, 500) var validatorCacheList = list.New() // get a single validator @@ -27,7 +31,7 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty if value == nil { return validator, false } - // return cached validator + // If these amino encoded bytes are in the cache, return the cached validator strValue := string(value) if val, ok := validatorCache[strValue]; ok { valToReturn := val.val @@ -35,11 +39,12 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty valToReturn.Operator = addr return valToReturn, true } - // get validator from cache + // amino bytes weren't found in cache, so amino unmarshal and add it to the cache validator = types.MustUnmarshalValidator(k.cdc, addr, value) cachedVal := cachedValidator{validator, strValue} validatorCache[strValue] = cachedValidator{validator, strValue} validatorCacheList.PushBack(cachedVal) + // if the cache is too big, pop off the last element from it if validatorCacheList.Len() > 500 { valToRemove := validatorCacheList.Remove(validatorCacheList.Front()).(cachedValidator) delete(validatorCache, valToRemove.marshalled) From 2c66ba0bd4e47a4692f55105b0c405a63b004875 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Sun, 2 Sep 2018 15:42:25 -0400 Subject: [PATCH 4/4] extra comment on cache key usage --- x/stake/keeper/validator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index d5fba14337..2d8f947fc0 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -18,9 +18,10 @@ import ( // live chain should, however we require the slashing to be fast as noone pays gas for it. type cachedValidator struct { val types.Validator - marshalled string + marshalled string // marshalled amino bytes for the validator object (not operator address) } +// validatorCache-key: validator amino bytes var validatorCache = make(map[string]cachedValidator, 500) var validatorCacheList = list.New() @@ -31,6 +32,7 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty if value == nil { return validator, false } + // If these amino encoded bytes are in the cache, return the cached validator strValue := string(value) if val, ok := validatorCache[strValue]; ok { @@ -39,11 +41,13 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty valToReturn.Operator = addr return valToReturn, true } + // amino bytes weren't found in cache, so amino unmarshal and add it to the cache validator = types.MustUnmarshalValidator(k.cdc, addr, value) cachedVal := cachedValidator{validator, strValue} validatorCache[strValue] = cachedValidator{validator, strValue} validatorCacheList.PushBack(cachedVal) + // if the cache is too big, pop off the last element from it if validatorCacheList.Len() > 500 { valToRemove := validatorCacheList.Remove(validatorCacheList.Front()).(cachedValidator)