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

feat: refactor subtask 2 #626

Merged
merged 4 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
25 changes: 0 additions & 25 deletions basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
package iavl

import (
"bytes"
"encoding/hex"
mrand "math/rand"
"sort"
Expand Down Expand Up @@ -171,26 +170,6 @@ func TestBasic(t *testing.T) {
}

func TestUnit(t *testing.T) {
expectHash := func(tree *ImmutableTree, hashCount int64) {
// ensure number of new hash calculations is as expected.
hash, count, err := tree.root.hashWithCount()
require.NoError(t, err)
if count != hashCount {
t.Fatalf("Expected %v new hashes, got %v", hashCount, count)
}
// nuke hashes and reconstruct hash, ensure it's the same.
tree.root.traverse(tree, true, func(node *Node) bool {
node.hash = nil
return false
})
// ensure that the new hash after nuking is the same as the old.
newHash, _, err := tree.root.hashWithCount()
require.NoError(t, err)
if !bytes.Equal(hash, newHash) {
t.Fatalf("Expected hash %v but got %v after nuking", hash, newHash)
}
}

expectSet := func(tree *MutableTree, i int, repr string, hashCount int64) {
tree.SaveVersion()
updated, err := tree.Set(i2b(i), []byte{})
Expand All @@ -200,8 +179,6 @@ func TestUnit(t *testing.T) {
t.Fatalf("Adding %v to %v:\nExpected %v\nUnexpectedly got %v updated:%v",
i, P(tree.lastSaved.root, tree.lastSaved), repr, P(tree.root, tree.ImmutableTree), updated)
}
// ensure hash calculation requirements
expectHash(tree.ImmutableTree, hashCount)
tree.ImmutableTree = tree.lastSaved.clone()
}

Expand All @@ -214,8 +191,6 @@ func TestUnit(t *testing.T) {
t.Fatalf("Removing %v from %v:\nExpected %v\nUnexpectedly got %v value:%v removed:%v",
i, P(tree.lastSaved.root, tree.lastSaved), repr, P(tree.root, tree.ImmutableTree), value, removed)
}
// ensure hash calculation requirements
expectHash(tree.ImmutableTree, hashCount)
tree.ImmutableTree = tree.lastSaved.clone()
}

Expand Down
75 changes: 33 additions & 42 deletions cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,29 @@ import (
ibytes "github.com/cosmos/iavl/internal/bytes"
)

// Node[T] represents a node eligible for caching.
type Node[T any] interface {
Copy link
Member

Choose a reason for hiding this comment

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

what's the reason for this revert?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously, the global nonce is used as a nodekey, so we didn't need to convert. But now we are using version + nonce, I think we don't need template type here.

GetKey() T
// Node represents a node eligible for caching.
type Node interface {
GetKey() []byte
}

// Cache is an in-memory structure to persist nodes for quick access.
// Please see lruCache for more details about why we need a custom
// cache implementation.
type Cache[T any] interface {
type Cache interface {
// Adds node to cache. If full and had to remove the oldest element,
// returns the oldest, otherwise nil.
// CONTRACT: node can never be nil. Otherwise, cache panics.
Add(node Node[T]) Node[T]
Add(node Node) Node

// Returns Node[T] for the key, if exists. nil otherwise.
Get(key T) Node[T]
// Returns Node for the key, if exists. nil otherwise.
Get(key []byte) Node

// Has returns true if node with key exists in cache, false otherwise.
Has(key T) bool
Has(key []byte) bool

// Remove removes node with key from cache. The removed node is returned.
// if not in cache, return nil.
Remove(key T) Node[T]
Remove(key []byte) Node

// Len returns the cache length.
Len() int
Expand All @@ -44,33 +44,33 @@ type Cache[T any] interface {
// The alternative implementations do not allow for
// customization and the ability to estimate the byte
// size of the cache.
type lruCache[K comparable, T any] struct {
dict map[K]*list.Element // FastNode cache.
maxElementCount int // FastNode the maximum number of nodes in the cache.
ll *list.List // LRU queue of cache elements. Used for deletion.
type lruCache struct {
dict map[string]*list.Element // FastNode cache.
maxElementCount int // FastNode the maximum number of nodes in the cache.
ll *list.List // LRU queue of cache elements. Used for deletion.
}

var _ Cache[int] = (*lruCache[string, int])(nil)
var _ Cache = (*lruCache)(nil)

func New[K comparable, T any](maxElementCount int) Cache[T] {
return &lruCache[K, T]{
dict: make(map[K]*list.Element),
func New(maxElementCount int) Cache {
return &lruCache{
dict: make(map[string]*list.Element),
maxElementCount: maxElementCount,
ll: list.New(),
}
}

func (c *lruCache[K, T]) Add(node Node[T]) Node[T] {
key := c.getKey(node.GetKey())
if e, exists := c.dict[key]; exists {
func (c *lruCache) Add(node Node) Node {
keyStr := ibytes.UnsafeBytesToStr(node.GetKey())
if e, exists := c.dict[keyStr]; exists {
c.ll.MoveToFront(e)
old := e.Value
e.Value = node
return old.(Node[T])
return old.(Node)
}

elem := c.ll.PushFront(node)
c.dict[key] = elem
c.dict[keyStr] = elem

if c.ll.Len() > c.maxElementCount {
oldest := c.ll.Back()
Expand All @@ -79,41 +79,32 @@ func (c *lruCache[K, T]) Add(node Node[T]) Node[T] {
return nil
}

func (c *lruCache[K, T]) Get(key T) Node[T] {
if ele, hit := c.dict[c.getKey(key)]; hit {
func (c *lruCache) Get(key []byte) Node {
if ele, hit := c.dict[ibytes.UnsafeBytesToStr(key)]; hit {
c.ll.MoveToFront(ele)
return ele.Value.(Node[T])
return ele.Value.(Node)
}
return nil
}

func (c *lruCache[K, T]) Has(key T) bool {
_, exists := c.dict[c.getKey(key)]
func (c *lruCache) Has(key []byte) bool {
_, exists := c.dict[ibytes.UnsafeBytesToStr(key)]
return exists
}

func (c *lruCache[K, T]) Len() int {
func (c *lruCache) Len() int {
return c.ll.Len()
}

func (c *lruCache[K, T]) Remove(key T) Node[T] {
if elem, exists := c.dict[c.getKey(key)]; exists {
func (c *lruCache) Remove(key []byte) Node {
if elem, exists := c.dict[ibytes.UnsafeBytesToStr(key)]; exists {
return c.remove(elem)
}
return nil
}

func (c *lruCache[K, T]) remove(e *list.Element) Node[T] {
removed := c.ll.Remove(e).(Node[T])
delete(c.dict, c.getKey(removed.GetKey()))
func (c *lruCache) remove(e *list.Element) Node {
removed := c.ll.Remove(e).(Node)
delete(c.dict, ibytes.UnsafeBytesToStr(removed.GetKey()))
return removed
}

func (c *lruCache[K, T]) getKey(key T) K {
switch any(key).(type) {
case []byte:
return any(ibytes.UnsafeBytesToStr(any(key).([]byte))).(K)
default:
return any(key).(K)
}
}
4 changes: 2 additions & 2 deletions cache/cache_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func BenchmarkAdd(b *testing.B) {
}

for name, tc := range testcases {
cache := cache.New[string, []byte](tc.cacheMax)
cache := cache.New(tc.cacheMax)
b.Run(name, func(b *testing.B) {
for i := 0; i < b.N; i++ {
b.StopTimer()
Expand All @@ -46,7 +46,7 @@ func BenchmarkAdd(b *testing.B) {
func BenchmarkRemove(b *testing.B) {
b.ReportAllocs()

cache := cache.New[string, []byte](1000)
cache := cache.New(1000)
existentKeyMirror := [][]byte{}
// Populate cache
for i := 0; i < 50; i++ {
Expand Down
20 changes: 10 additions & 10 deletions cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type cacheOp struct {
}

type testcase struct {
setup func(cache.Cache[[]byte])
setup func(cache.Cache)
cacheMax int
cacheOps []cacheOp
expectedNodeIndexes []int // contents of the cache once test case completes represent by indexes in testNodes
Expand All @@ -43,9 +43,9 @@ const (
testKey = "key"
)

var _ cache.Node[[]byte] = (*testNode)(nil)
var _ cache.Node = (*testNode)(nil)

var testNodes = []cache.Node[[]byte]{
var testNodes = []cache.Node{
&testNode{
key: []byte(fmt.Sprintf("%s%d", testKey, 1)),
},
Expand Down Expand Up @@ -150,7 +150,7 @@ func Test_Cache_Add(t *testing.T) {

for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
cache := cache.New[string, []byte](tc.cacheMax)
cache := cache.New(tc.cacheMax)

expectedCurSize := 0

Expand Down Expand Up @@ -189,7 +189,7 @@ func Test_Cache_Remove(t *testing.T) {
},
},
"remove non-existent key, cache max 1 - nil returned": {
setup: func(c cache.Cache[[]byte]) {
setup: func(c cache.Cache) {
require.Nil(t, c.Add(testNodes[1]))
require.Equal(t, 1, c.Len())
},
Expand All @@ -203,7 +203,7 @@ func Test_Cache_Remove(t *testing.T) {
expectedNodeIndexes: []int{1},
},
"remove existent key, cache max 1 - removed": {
setup: func(c cache.Cache[[]byte]) {
setup: func(c cache.Cache) {
require.Nil(t, c.Add(testNodes[0]))
require.Equal(t, 1, c.Len())
},
Expand All @@ -216,7 +216,7 @@ func Test_Cache_Remove(t *testing.T) {
},
},
"remove twice, cache max 1 - removed first time, then nil": {
setup: func(c cache.Cache[[]byte]) {
setup: func(c cache.Cache) {
require.Nil(t, c.Add(testNodes[0]))
require.Equal(t, 1, c.Len())
},
Expand All @@ -233,7 +233,7 @@ func Test_Cache_Remove(t *testing.T) {
},
},
"remove all, cache max 3": {
setup: func(c cache.Cache[[]byte]) {
setup: func(c cache.Cache) {
require.Nil(t, c.Add(testNodes[0]))
require.Nil(t, c.Add(testNodes[1]))
require.Nil(t, c.Add(testNodes[2]))
Expand All @@ -259,7 +259,7 @@ func Test_Cache_Remove(t *testing.T) {

for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
cache := cache.New[string, []byte](tc.cacheMax)
cache := cache.New(tc.cacheMax)

if tc.setup != nil {
tc.setup(cache)
Expand Down Expand Up @@ -290,7 +290,7 @@ func Test_Cache_Remove(t *testing.T) {
}
}

func validateCacheContentsAfterTest(t *testing.T, tc testcase, cache cache.Cache[[]byte]) {
func validateCacheContentsAfterTest(t *testing.T, tc testcase, cache cache.Cache) {
require.Equal(t, len(tc.expectedNodeIndexes), cache.Len())
for _, idx := range tc.expectedNodeIndexes {
expectedNode := testNodes[idx]
Expand Down
4 changes: 2 additions & 2 deletions export.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var ErrorExportDone = errors.New("export is complete")
type ExportNode struct {
Key []byte
Value []byte
Version int64
NodeKey *NodeKey
Height int8
}

Expand Down Expand Up @@ -53,7 +53,7 @@ func (e *Exporter) export(ctx context.Context) {
exportNode := &ExportNode{
Key: node.key,
Value: node.value,
Version: node.version,
NodeKey: node.nodeKey,
Height: node.subtreeHeight,
}

Expand Down
18 changes: 9 additions & 9 deletions export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,15 @@ func TestExporter(t *testing.T) {
tree := setupExportTreeBasic(t)

expect := []*ExportNode{
{Key: []byte("a"), Value: []byte{1}, Version: 1, Height: 0},
{Key: []byte("b"), Value: []byte{2}, Version: 3, Height: 0},
{Key: []byte("b"), Value: nil, Version: 3, Height: 1},
{Key: []byte("c"), Value: []byte{3}, Version: 3, Height: 0},
{Key: []byte("c"), Value: nil, Version: 3, Height: 2},
{Key: []byte("d"), Value: []byte{4}, Version: 2, Height: 0},
{Key: []byte("e"), Value: []byte{5}, Version: 3, Height: 0},
{Key: []byte("e"), Value: nil, Version: 3, Height: 1},
{Key: []byte("d"), Value: nil, Version: 3, Height: 3},
{Key: []byte("a"), Value: []byte{1}, NodeKey: &NodeKey{version: 1, nonce: 3}, Height: 0},
{Key: []byte("b"), Value: []byte{2}, NodeKey: &NodeKey{version: 3, nonce: 4}, Height: 0},
{Key: []byte("b"), Value: nil, NodeKey: &NodeKey{version: 3, nonce: 3}, Height: 1},
{Key: []byte("c"), Value: []byte{3}, NodeKey: &NodeKey{version: 3, nonce: 5}, Height: 0},
{Key: []byte("c"), Value: nil, NodeKey: &NodeKey{version: 3, nonce: 2}, Height: 2},
{Key: []byte("d"), Value: []byte{4}, NodeKey: &NodeKey{version: 2, nonce: 5}, Height: 0},
{Key: []byte("e"), Value: []byte{5}, NodeKey: &NodeKey{version: 3, nonce: 7}, Height: 0},
{Key: []byte("e"), Value: nil, NodeKey: &NodeKey{version: 3, nonce: 6}, Height: 1},
{Key: []byte("d"), Value: nil, NodeKey: &NodeKey{version: 3, nonce: 1}, Height: 3},
}

actual := make([]*ExportNode, 0, len(expect))
Expand Down
2 changes: 1 addition & 1 deletion fastnode/fast_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type Node struct {
value []byte
}

var _ cache.Node[[]byte] = (*Node)(nil)
var _ cache.Node = (*Node)(nil)

// NewNode returns a new fast node from a value and version.
func NewNode(key []byte, value []byte, version int64) *Node {
Expand Down
Loading