Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: simplify cachekv store with copy-on-write btree #14350

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/auth/vesting) [#13502](https://github.com/cosmos/cosmos-sdk/pull/13502) Add Amino Msg registration for `MsgCreatePeriodicVestingAccount`.
* (x/auth)[#13780](https://github.com/cosmos/cosmos-sdk/pull/13780) `id` (type of int64) in `AccountAddressByID` grpc query is now deprecated, update to account-id(type of uint64) to use `AccountAddressByID`.
* (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Fix group MinExecutionPeriod that is checked on execution now, instead of voting period end.
* (store) [#]() simplify cachekv store with copy-on-write btree, it's consensus-breaking because iteration is done on isolated view now, while it's not the case before.
yihuang marked this conversation as resolved.
Show resolved Hide resolved

### API Breaking Changes

Expand Down
80 changes: 0 additions & 80 deletions store/cachekv/internal/btree.go

This file was deleted.

93 changes: 93 additions & 0 deletions store/cachekv/internal/memcache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package internal

import (
"bytes"
"errors"

"github.com/tidwall/btree"
)

var errKeyEmpty = errors.New("key cannot be empty")

const (
// The approximate number of items and children per B-tree node. Tuned with benchmarks.
// copied from memdb.
bTreeDegree = 32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the impact of misconfiguring this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it only affect performance

)

// MemCache implements the in-memory cache for cachekv store,
// we don't use MemDB here because cachekv is used extensively in sdk core path,
// we need it to be as fast as possible, while `MemDB` is mainly used as a mocking db in unit tests.
//
// We choose tidwall/btree over google/btree here because it provides API to implement step iterator directly.
type MemCache struct {
tree *btree.BTreeG[item]
}

// NewMemCache creates a wrapper around `btree.BTreeG`.
func NewMemCache() MemCache {
return MemCache{tree: btree.NewBTreeGOptions(byKeys, btree.Options{
Degree: bTreeDegree,
NoLocks: false,
})}
}
yihuang marked this conversation as resolved.
Show resolved Hide resolved

// Set set a cache entry, dirty means it's newly set,
// `nil` value means a deletion.
func (bt MemCache) Set(key, value []byte, dirty bool) {
bt.tree.Set(item{key: key, value: value, dirty: dirty})
}

// Get returns (value, found)
func (bt MemCache) Get(key []byte) ([]byte, bool) {
i, found := bt.tree.Get(item{key: key})
if !found {
return nil, false
}
return i.value, true
}

// ScanDirtyItems iterate over the dirty entries.
func (bt MemCache) ScanDirtyItems(fn func(key, value []byte)) {
bt.tree.Scan(func(item item) bool {
if item.dirty {
fn(item.key, item.value)
}
return true
})
}

// Copy the cache. This is a copy-on-write operation and is very fast because
// it only performs a shadowed copy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain a bit how this work? I understand that the copy is not done until the btree is modified. This means either the original or the copy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/tidwall/btree/blob/master/btreeg.go#L296
I think the writer will check a seq number on nodes and do the copy when needed.

func (bt MemCache) Copy() MemCache {
return MemCache{tree: bt.tree.IsoCopy()}
}

// Iterator iterates on a isolated view of the cache, not affected by future modifications.
func (bt MemCache) Iterator(start, end []byte) *memIterator {
if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) {
panic(errKeyEmpty)
}
return NewMemIterator(start, end, bt.Copy(), true)
}

// ReverseIterator iterates on a isolated view of the cache, not affected by future modifications.
func (bt MemCache) ReverseIterator(start, end []byte) *memIterator {
if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) {
panic(errKeyEmpty)
}
return NewMemIterator(start, end, bt.Copy(), false)
}

// item represents a cached key-value pair and the entry of the cache btree.
// If dirty is true, it indicates the cached value is newly set, maybe different from the underlying value.
type item struct {
key []byte
value []byte
dirty bool
}

// byKeys compares the items by key
func byKeys(a, b item) bool {
return bytes.Compare(a.key, b.key) == -1
}
Comment on lines +84 to +95
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: move these types to the top

Loading