Skip to content

Commit

Permalink
Merge PR: merge cosmos sdk pr for performance.(6811,10339,8811) (#2100)
Browse files Browse the repository at this point in the history 
* merge cosmos sdk pr 6811 and 10339,fix for removeZeroCoins mutates original slice;merge cosmos sdk pr 8811, use typed types/kv.List instead of container/list.List

* merge cosmos sdk pr 6811 and 10339,fix for removeZeroCoins mutates original slice;

* delete invalid comments

Co-authored-by: KamiD <44460798+KamiD@users.noreply.github.com>
  • Loading branch information
@zjg555543 @KamiD
zjg555543 and KamiD authored May 26, 2022
1 parent dff85df commit 48f7205
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 33 deletions.
13 changes: 6 additions & 7 deletions libs/cosmos-sdk/store/cachekv/memiterator.go
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package cachekv

import (
"container/list"
"errors"

tmkv "github.com/okex/exchain/libs/tendermint/libs/kv"
kv "github.com/okex/exchain/libs/cosmos-sdk/types/kv"
dbm "github.com/okex/exchain/libs/tm-db"
)

Expand All @@ -13,15 +11,16 @@ import (
// Implements Iterator.
type memIterator struct {
start, end []byte
items []*tmkv.Pair
items []*kv.Pair
ascending bool
}

func newMemIterator(start, end []byte, items *list.List, ascending bool) *memIterator {
itemsInDomain := make([]*tmkv.Pair, 0)
func newMemIterator(start, end []byte, items *kv.List, ascending bool) *memIterator {
itemsInDomain := make([]*kv.Pair, 0, items.Len())

var entered bool
for e := items.Front(); e != nil; e = e.Next() {
item := e.Value.(*tmkv.Pair)
item := e.Value
if !dbm.IsKeyInDomain(item.Key, start, end) {
if entered {
break
Expand Down
17 changes: 7 additions & 10 deletions libs/cosmos-sdk/store/cachekv/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,19 @@ package cachekv

import (
"bytes"
"container/list"
"io"
"reflect"
"sort"
"sync"
"unsafe"

"github.com/okex/exchain/libs/iavl"

"github.com/tendermint/go-amino"

tmkv "github.com/okex/exchain/libs/tendermint/libs/kv"
dbm "github.com/okex/exchain/libs/tm-db"
"github.com/tendermint/go-amino"

"github.com/okex/exchain/libs/cosmos-sdk/store/tracekv"
"github.com/okex/exchain/libs/cosmos-sdk/store/types"
kv "github.com/okex/exchain/libs/cosmos-sdk/types/kv"
)

// If value is nil but deleted is false, it means the parent doesn't have the
Expand All @@ -35,7 +32,7 @@ type Store struct {
dirty map[string]cValue
readList map[string][]byte
unsortedCache map[string]struct{}
sortedCache *list.List // always ascending sorted
sortedCache *kv.List // always ascending sorted
parent types.KVStore

preChangesHandler PreChangesHandler
Expand All @@ -48,7 +45,7 @@ func NewStore(parent types.KVStore) *Store {
dirty: make(map[string]cValue),
readList: make(map[string][]byte),
unsortedCache: make(map[string]struct{}),
sortedCache: list.New(),
sortedCache: kv.NewList(),
parent: parent,
}
}
Expand Down Expand Up @@ -309,13 +306,13 @@ func byteSliceToStr(b []byte) string {

// Constructs a slice of dirty items, to use w/ memIterator.
func (store *Store) dirtyItems(start, end []byte) {
unsorted := make([]*tmkv.Pair, 0)
unsorted := make([]*kv.Pair, 0)

n := len(store.unsortedCache)
for key := range store.unsortedCache {
if dbm.IsKeyInDomain(strToByte(key), start, end) {
cacheValue := store.dirty[key]
unsorted = append(unsorted, &tmkv.Pair{Key: []byte(key), Value: cacheValue.value})
unsorted = append(unsorted, &kv.Pair{Key: []byte(key), Value: cacheValue.value})
}
}

Expand All @@ -335,7 +332,7 @@ func (store *Store) dirtyItems(start, end []byte) {

for e := store.sortedCache.Front(); e != nil && len(unsorted) != 0; {
uitem := unsorted[0]
sitem := e.Value.(*tmkv.Pair)
sitem := e.Value
comp := bytes.Compare(uitem.Key, sitem.Key)
switch comp {
case -1:
Expand Down
22 changes: 14 additions & 8 deletions libs/cosmos-sdk/types/coin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,18 +558,24 @@ func (coins Coins) IsAnyGTE(coinsB Coins) bool {

// removeZeroCoins removes all zero coins from the given coin set in-place.
func removeZeroCoins(coins Coins) Coins {
i, l := 0, len(coins)
for i < l {
for i := 0; i < len(coins); i++ {
if coins[i].IsZero() {
// remove coin
coins = append(coins[:i], coins[i+1:]...)
l--
} else {
i++
break
} else if i == len(coins)-1 {
return coins
}
}
var result []Coin
if len(coins) > 0 {
result = make([]Coin, 0, len(coins)-1)
}

return coins[:i]
for _, coin := range coins {
if !coin.IsZero() {
result = append(result, coin)
}
}
return result
}

//-----------------------------------------------------------------------------
Expand Down
52 changes: 52 additions & 0 deletions libs/cosmos-sdk/types/coin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,58 @@ func TestCoinIsZero(t *testing.T) {
require.False(t, res)
}

func TestFilteredZeroCoins(t *testing.T) {
cases := []struct {
name string
input Coins
original string
expected string
}{
{
name: "all greater than zero",
input: Coins{
NewInt64Coin("testa", 1),
NewInt64Coin("testb", 2),
NewInt64Coin("testc", 3),
NewInt64Coin("testd", 4),
NewInt64Coin("teste", 5),
},
original: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
expected: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
},
{
name: "zero coin in middle",
input: Coins{
NewInt64Coin("testa", 1),
NewInt64Coin("testb", 2),
NewInt64Coin("testc", 0),
NewInt64Coin("testd", 4),
NewInt64Coin("teste", 5),
},
original: "1.000000000000000000testa,2.000000000000000000testb,0.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
expected: "1.000000000000000000testa,2.000000000000000000testb,4.000000000000000000testd,5.000000000000000000teste",
},
{
name: "zero coin end (unordered)",
input: Coins{
NewInt64Coin("teste", 5),
NewInt64Coin("testc", 3),
NewInt64Coin("testa", 1),
NewInt64Coin("testd", 4),
NewInt64Coin("testb", 0),
},
original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb",
expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
},
}

for _, tt := range cases {
undertest := NewCoins(tt.input...)
require.Equal(t, tt.expected, undertest.String(), "NewCoins must return expected results")
require.Equal(t, tt.original, tt.input.String(), "input must be unmodified and match original")
}
}

// ----------------------------------------------------------------------------
// Coins tests

Expand Down
22 changes: 14 additions & 8 deletions libs/cosmos-sdk/types/dec_coin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,18 +659,24 @@ func (coins DecCoins) IsAllPositive() bool {
}

func removeZeroDecCoins(coins DecCoins) DecCoins {
i, l := 0, len(coins)
for i < l {
for i := 0; i < len(coins); i++ {
if coins[i].IsZero() {
// remove coin
coins = append(coins[:i], coins[i+1:]...)
l--
} else {
i++
break
} else if i == len(coins)-1 {
return coins
}
}
var result []DecCoin
if len(coins) > 0 {
result = make([]DecCoin, 0, len(coins)-1)
}

return coins[:i]
for _, coin := range coins {
if !coin.IsZero() {
result = append(result, coin)
}
}
return result
}

//-----------------------------------------------------------------------------
Expand Down
51 changes: 51 additions & 0 deletions libs/cosmos-sdk/types/dec_coin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,57 @@ func TestAddDecCoins(t *testing.T) {
}
}

func TestFilteredZeroDecCoins(t *testing.T) {
cases := []struct {
name string
input DecCoins
original string
expected string
}{
{
name: "all greater than zero",
input: DecCoins{
{"testa", NewDec(1)},
{"testb", NewDec(2)},
{"testc", NewDec(3)},
{"testd", NewDec(4)},
{"teste", NewDec(5)},
},
original: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
expected: "1.000000000000000000testa,2.000000000000000000testb,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
},
{
name: "zero coin in middle",
input: DecCoins{
{"testa", NewDec(1)},
{"testb", NewDec(2)},
{"testc", NewDec(0)},
{"testd", NewDec(4)},
{"teste", NewDec(5)},
},
original: "1.000000000000000000testa,2.000000000000000000testb,0.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
expected: "1.000000000000000000testa,2.000000000000000000testb,4.000000000000000000testd,5.000000000000000000teste",
},
{
name: "zero coin end (unordered)",
input: DecCoins{
{"teste", NewDec(5)},
{"testc", NewDec(3)},
{"testa", NewDec(1)},
{"testd", NewDec(4)},
{"testb", NewDec(0)},
},
original: "5.000000000000000000teste,3.000000000000000000testc,1.000000000000000000testa,4.000000000000000000testd,0.000000000000000000testb",
expected: "1.000000000000000000testa,3.000000000000000000testc,4.000000000000000000testd,5.000000000000000000teste",
},
}

for _, tt := range cases {
undertest := NewDecCoins(tt.input...)
require.Equal(t, tt.expected, undertest.String(), "NewDecCoins must return expected results")
require.Equal(t, tt.original, tt.input.String(), "input must be unmodified and match original")
}
}
func TestIsValid(t *testing.T) {
tests := []struct {
coin DecCoin
Expand Down

0 comments on commit 48f7205

Please sign in to comment.