Problem: memiavl performance regression when cache miss (#1166)

* Problem: memiavl performance regression when cache miss

Solution:
- make the cache optional

* cleanup

memiavl/benchmark_test.go

@@ -8,8 +8,6 @@ import (
 	"testing"
 
 	iavlcache "github.com/cosmos/iavl/cache"
-	lru "github.com/hashicorp/golang-lru"
-	"github.com/hashicorp/golang-lru/simplelru"
 	"github.com/stretchr/testify/require"
 	"github.com/tidwall/btree"
 )
@@ -39,18 +37,40 @@ func BenchmarkRandomGet(b *testing.B) {
 	snapshot, err := OpenSnapshot(snapshotDir)
 	require.NoError(b, err)
 	defer snapshot.Close()
-	diskTree := NewFromSnapshot(snapshot, true, 0)
 
-	require.Equal(b, targetValue, tree.Get(targetKey))
-	require.Equal(b, targetValue, diskTree.Get(targetKey))
-
-	b.ResetTimer()
 	b.Run("memiavl", func(b *testing.B) {
+		require.Equal(b, targetValue, tree.Get(targetKey))
+
+		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
 			_ = tree.Get(targetKey)
 		}
 	})
 	b.Run("memiavl-disk", func(b *testing.B) {
+		diskTree := NewFromSnapshot(snapshot, true, 0)
+		require.Equal(b, targetValue, diskTree.Get(targetKey))
+
+		b.ResetTimer()
+		for i := 0; i < b.N; i++ {
+			_ = diskTree.Get(targetKey)
+		}
+	})
+	b.Run("memiavl-disk-cache-hit", func(b *testing.B) {
+		diskTree := NewFromSnapshot(snapshot, true, 1)
+		require.Equal(b, targetValue, diskTree.Get(targetKey))
+
+		b.ResetTimer()
+		for i := 0; i < b.N; i++ {
+			_ = diskTree.Get(targetKey)
+		}
+	})
+	b.Run("memiavl-disk-cache-miss", func(b *testing.B) {
+		diskTree := NewFromSnapshot(snapshot, true, 0)
+		// enforce an empty cache to emulate cache miss
+		diskTree.cache = iavlcache.New(0)
+		require.Equal(b, targetValue, diskTree.Get(targetKey))
+
+		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
 			_ = diskTree.Get(targetKey)
 		}
@@ -87,34 +107,6 @@ func BenchmarkRandomGet(b *testing.B) {
 			_, _ = bt32.Get(targetItem)
 		}
 	})
-	b.Run("lru-cache", func(b *testing.B) {
-		cache, err := lru.NewARC(amount)
-		require.NoError(b, err)
-		for _, item := range items {
-			cache.Add(string(item.key), item.value)
-		}
-		v, _ := cache.Get(string(targetItem.key))
-		require.Equal(b, targetValue, v.([]byte))
-
-		b.ResetTimer()
-		for i := 0; i < b.N; i++ {
-			_, _ = cache.Get(string(targetKey))
-		}
-	})
-	b.Run("simplelru", func(b *testing.B) {
-		cache, err := simplelru.NewLRU(amount, nil)
-		require.NoError(b, err)
-		for _, item := range items {
-			cache.Add(string(item.key), item.value)
-		}
-		v, _ := cache.Get(string(targetItem.key))
-		require.Equal(b, targetValue, v.([]byte))
-
-		b.ResetTimer()
-		for i := 0; i < b.N; i++ {
-			_, _ = cache.Get(string(targetKey))
-		}
-	})
 	b.Run("iavl-lru", func(b *testing.B) {
 		cache := iavlcache.New(amount)
 		for _, item := range items {

memiavl/go.mod

@@ -9,7 +9,6 @@ require (
 	github.com/cosmos/gogoproto v1.4.7
 	github.com/cosmos/iavl v0.19.6
 	github.com/gogo/protobuf v1.3.2
-	github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d
 	github.com/ledgerwatch/erigon-lib v0.0.0-20230210071639-db0e7ed11263
 	github.com/stretchr/testify v1.8.3
 	github.com/tendermint/tendermint v0.34.29

memiavl/go.sum

@@ -196,7 +196,6 @@ github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhE
 github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek=
 github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d h1:dg1dEPuWpEqDnvIw251EVy4zlP8gWbsGj4BsUKCRpYs=
-github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
 github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
 github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
 github.com/hdevalence/ed25519consensus v0.0.0-20220222234857-c00d1f31bab3 h1:aSVUgRRRtOrZOC1fYmY9gV0e9z/Iu+xNVSASWjsuyGU=

memiavl/tree.go

@@ -14,6 +14,13 @@ import (
 
 var emptyHash = sha256.New().Sum(nil)
 
+func NewCache(cacheSize int) cache.Cache {
+	if cacheSize == 0 {
+		return nil
+	}
+	return cache.New(cacheSize)
+}
+
 // verify change sets by replay them to rebuild iavl tree and verify the root hashes
 type Tree struct {
 	version uint32
@@ -49,7 +56,7 @@ func NewEmptyTree(version uint64, initialVersion uint32, cacheSize int) *Tree {
 		initialVersion: initialVersion,
 		// no need to copy if the tree is not backed by snapshot
 		zeroCopy: true,
-		cache:    cache.New(cacheSize),
+		cache:    NewCache(cacheSize),
 	}
 }
 
@@ -70,7 +77,7 @@ func NewFromSnapshot(snapshot *Snapshot, zeroCopy bool, cacheSize int) *Tree {
 		version:  snapshot.Version(),
 		snapshot: snapshot,
 		zeroCopy: zeroCopy,
-		cache:    cache.New(cacheSize),
+		cache:    NewCache(cacheSize),
 	}
 
 	if !snapshot.IsEmpty() {
@@ -105,7 +112,7 @@ func (t *Tree) Copy(cacheSize int) *Tree {
 	}
 	newTree := *t
 	// cache is not copied along because it's not thread-safe to access
-	newTree.cache = cache.New(cacheSize)
+	newTree.cache = NewCache(cacheSize)
 	return &newTree
 }
 
@@ -126,12 +133,16 @@ func (t *Tree) set(key, value []byte) {
 		value = []byte{}
 	}
 	t.root, _ = setRecursive(t.root, key, value, t.version+1, t.cowVersion)
-	t.cache.Add(&cacheNode{key, value})
+	if t.cache != nil {
+		t.cache.Add(&cacheNode{key, value})
+	}
 }
 
 func (t *Tree) remove(key []byte) {
 	_, t.root, _ = removeRecursive(t.root, key, t.version+1, t.cowVersion)
-	t.cache.Remove(key)
+	if t.cache != nil {
+		t.cache.Remove(key)
+	}
 }
 
 // SaveVersion increases the version number and optionally updates the hashes
@@ -192,16 +203,20 @@ func (t *Tree) GetByIndex(index int64) ([]byte, []byte) {
 }
 
 func (t *Tree) Get(key []byte) []byte {
-	if node := t.cache.Get(key); node != nil {
-		return node.(*cacheNode).value
+	if t.cache != nil {
+		if node := t.cache.Get(key); node != nil {
+			return node.(*cacheNode).value
+		}
 	}
 
 	_, value := t.GetWithIndex(key)
 	if value == nil {
 		return nil
 	}
 
-	t.cache.Add(&cacheNode{key, value})
+	if t.cache != nil {
+		t.cache.Add(&cacheNode{key, value})
+	}
 	return value
 }