Update merkle cache to be FIFO instead of LRU

x/merkledb/cache.go

@@ -12,32 +12,26 @@ import (
 
 // A cache that calls [onEviction] on the evicted element.
 type onEvictCache[K comparable, V any] struct {
-	lock    sync.Mutex
-	maxSize int
-	// LRU --> MRU from left to right.
-	lru        linkedhashmap.LinkedHashmap[K, V]
+	lock       sync.RWMutex
+	maxSize    int
+	fifo       linkedhashmap.LinkedHashmap[K, V]
 	onEviction func(V) error
 }
 
 func newOnEvictCache[K comparable, V any](maxSize int, onEviction func(V) error) onEvictCache[K, V] {
 	return onEvictCache[K, V]{
 		maxSize:    maxSize,
-		lru:        linkedhashmap.New[K, V](),
+		fifo:       linkedhashmap.New[K, V](),
 		onEviction: onEviction,
 	}
 }
 
 // Get an element from this cache.
 func (c *onEvictCache[K, V]) Get(key K) (V, bool) {
-	c.lock.Lock()
-	defer c.lock.Unlock()
+	c.lock.RLock()
+	defer c.lock.RUnlock()
 
-	val, ok := c.lru.Get(key)
-	if ok {
-		// This key was touched; move it to the MRU position.
-		c.lru.Put(key, val)
-	}
-	return val, ok
+	return c.fifo.Get(key)
 }
 
 // Put an element into this cache. If this causes an element
@@ -47,13 +41,11 @@ func (c *onEvictCache[K, V]) Put(key K, value V) error {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
-	c.lru.Put(key, value) // Mark as MRU
+	c.fifo.Put(key, value) // Mark as MRU
 
-	if c.lru.Len() > c.maxSize {
-		// Note that [c.cache] has already evicted the oldest
-		// element because its max size is [c.maxSize].
-		oldestKey, oldsetVal, _ := c.lru.Oldest()
-		c.lru.Delete(oldestKey)
+	if c.fifo.Len() > c.maxSize {
+		oldestKey, oldsetVal, _ := c.fifo.Oldest()
+		c.fifo.Delete(oldestKey)
 		return c.onEviction(oldsetVal)
 	}
 	return nil
@@ -66,12 +58,12 @@ func (c *onEvictCache[K, V]) Put(key K, value V) error {
 func (c *onEvictCache[K, V]) Flush() error {
 	c.lock.Lock()
 	defer func() {
-		c.lru = linkedhashmap.New[K, V]()
+		c.fifo = linkedhashmap.New[K, V]()
 		c.lock.Unlock()
 	}()
 
 	var errs wrappers.Errs
-	iter := c.lru.NewIterator()
+	iter := c.fifo.NewIterator()
 	for iter.Next() {
 		val := iter.Value()
 		errs.Add(c.onEviction(val))

x/merkledb/cache_test.go

@@ -24,8 +24,8 @@ func TestNewOnEvictCache(t *testing.T) {
 
 	cache := newOnEvictCache[int](maxSize, onEviction)
 	require.Equal(maxSize, cache.maxSize)
-	require.NotNil(cache.lru)
-	require.Equal(0, cache.lru.Len())
+	require.NotNil(cache.fifo)
+	require.Equal(0, cache.fifo.Len())
 	// Can't test function equality directly so do this
 	// to make sure it was assigned correctly
 	err := cache.onEviction(0)
@@ -35,7 +35,7 @@ func TestNewOnEvictCache(t *testing.T) {
 
 // Test the functionality of the cache when the onEviction function
 // never returns an error.
-// Note this test assumes the internal cache is an LRU cache.
+// Note this test assumes the internal cache is a FIFO cache
 func TestOnEvictCacheNoOnEvictionError(t *testing.T) {
 	require := require.New(t)
 
@@ -55,7 +55,7 @@ func TestOnEvictCacheNoOnEvictionError(t *testing.T) {
 	// Put key
 	err := cache.Put(0, 0)
 	require.NoError(err)
-	require.Equal(1, cache.lru.Len())
+	require.Equal(1, cache.fifo.Len())
 
 	// Get key
 	val, ok := cache.Get(0)
@@ -70,21 +70,21 @@ func TestOnEvictCacheNoOnEvictionError(t *testing.T) {
 	for i := 1; i < maxSize; i++ {
 		err := cache.Put(i, i)
 		require.NoError(err)
-		require.Equal(i+1, cache.lru.Len())
+		require.Equal(i+1, cache.fifo.Len())
 	}
 	require.Len(evicted, 0)
 
-	// Cache has [0,1,2] from LRU --> MRU
+	// Cache has [0,1,2]
 
-	// Put another key. This should evict the LRU key (0).
+	// Put another key. This should evict the oldest inserted key (0).
 	err = cache.Put(maxSize, maxSize)
 	require.NoError(err)
-	require.Equal(maxSize, cache.lru.Len())
+	require.Equal(maxSize, cache.fifo.Len())
 	require.Len(evicted, 1)
 	require.Equal(0, evicted[0])
 
-	// Cache has [1,2,3] from LRU --> MRU
-	iter := cache.lru.NewIterator()
+	// Cache has [1,2,3]
+	iter := cache.fifo.NewIterator()
 	require.True(iter.Next())
 	require.Equal(1, iter.Key())
 	require.Equal(1, iter.Value())
@@ -107,57 +107,57 @@ func TestOnEvictCacheNoOnEvictionError(t *testing.T) {
 		require.Equal(i, val)
 	}
 
-	// Cache has [3,2,1] from LRU --> MRU
-	iter = cache.lru.NewIterator()
+	// Cache has [1,2,3]
+	iter = cache.fifo.NewIterator()
 	require.True(iter.Next())
-	require.Equal(3, iter.Key())
-	require.Equal(3, iter.Value())
+	require.Equal(1, iter.Key())
+	require.Equal(1, iter.Value())
 	require.True(iter.Next())
 	require.Equal(2, iter.Key())
 	require.Equal(2, iter.Value())
 	require.True(iter.Next())
-	require.Equal(1, iter.Key())
-	require.Equal(1, iter.Value())
+	require.Equal(3, iter.Key())
+	require.Equal(3, iter.Value())
 	require.False(iter.Next())
 
-	// Put another key to evict the LRU key (3).
+	// Put another key to evict the oldest inserted key (1).
 	err = cache.Put(maxSize+1, maxSize+1)
 	require.NoError(err)
-	require.Equal(maxSize, cache.lru.Len())
+	require.Equal(maxSize, cache.fifo.Len())
 	require.Len(evicted, 2)
-	require.Equal(3, evicted[1])
+	require.Equal(1, evicted[1])
 
-	// Cache has [2,1,4] from LRU --> MRU
-	iter = cache.lru.NewIterator()
+	// Cache has [2,3,4]
+	iter = cache.fifo.NewIterator()
 	require.True(iter.Next())
 	require.Equal(2, iter.Key())
 	require.Equal(2, iter.Value())
 	require.True(iter.Next())
-	require.Equal(1, iter.Key())
-	require.Equal(1, iter.Value())
+	require.Equal(3, iter.Key())
+	require.Equal(3, iter.Value())
 	require.True(iter.Next())
 	require.Equal(4, iter.Key())
 	require.Equal(4, iter.Value())
 	require.False(iter.Next())
 
-	// 3 should no longer be in the cache
-	_, ok = cache.Get(3)
+	// 1 should no longer be in the cache
+	_, ok = cache.Get(1)
 	require.False(ok)
 
 	err = cache.Flush()
 	require.NoError(err)
 
 	// Cache should be empty
-	require.Equal(0, cache.lru.Len())
+	require.Equal(0, cache.fifo.Len())
 	require.Len(evicted, 5)
-	require.Equal(evicted, []int{0, 3, 2, 1, 4})
-	require.Equal(0, cache.lru.Len())
+	require.Equal([]int{0, 1, 2, 3, 4}, evicted)
+	require.Equal(0, cache.fifo.Len())
 	require.Equal(maxSize, cache.maxSize) // Should be unchanged
 }
 
 // Test the functionality of the cache when the onEviction function
 // returns an error.
-// Note this test assumes the internal cache is an LRU cache.
+// Note this test assumes the cache is FIFO.
 func TestOnEvictCacheOnEvictionError(t *testing.T) {
 	var (
 		require    = require.New(t)
@@ -179,17 +179,19 @@ func TestOnEvictCacheOnEvictionError(t *testing.T) {
 	for i := 0; i < maxSize; i++ {
 		err := cache.Put(i, i)
 		require.NoError(err)
-		require.Equal(i+1, cache.lru.Len())
+		require.Equal(i+1, cache.fifo.Len())
 	}
 
-	// Put another key. This should evict the LRU key (0)
+	// Cache has [0,1]
+
+	// Put another key. This should evict the first key (0)
 	// and return an error since 0 is even.
 	err := cache.Put(maxSize, maxSize)
 	require.ErrorIs(err, errTest)
 
-	// Cache should still have correct state [1,2]
+	// Cache has [1,2]
 	require.Equal(evicted, []int{0})
-	require.Equal(maxSize, cache.lru.Len())
+	require.Equal(maxSize, cache.fifo.Len())
 	_, ok := cache.Get(0)
 	require.False(ok)
 	_, ok = cache.Get(1)
@@ -202,7 +204,7 @@ func TestOnEvictCacheOnEvictionError(t *testing.T) {
 	require.ErrorIs(err, errTest)
 
 	// Should still be empty.
-	require.Equal(0, cache.lru.Len())
+	require.Equal(0, cache.fifo.Len())
 	require.Equal(evicted, []int{0, 1, 2})
 	_, ok = cache.Get(0)
 	require.False(ok)