make caching/lru a real LRU

btree/cache_internal_test.go

@@ -18,15 +18,31 @@ func Test_Get(t *testing.T) {
 			Keys:     []rune{'b'},
 			Pointers: []int{2},
 		}
+		n3 := Node[rune, int, int]{
+			Keys:     []rune{'c'},
+			Pointers: []int{3},
+		}
+		n4 := Node[rune, int, int]{
+			Keys:     []rune{'d'},
+			Pointers: []int{4},
+		}
 		ptr1, err := st.Put(&n1)
 		require.NoError(t, err)
 		ptr2, err := st.Put(&n2)
 		require.NoError(t, err)
-
-		c := cachefor[rune, int, int](&st, 1)
-		n, err := st.Get(ptr1)
+		ptr3, err := st.Put(&n3)
+		require.NoError(t, err)
+		ptr4, err := st.Put(&n4)
 		require.NoError(t, err)
-		require.NoError(t, c.Update(ptr1, n))
+
+		c := cachefor[rune, int, int](&st, 2)
+
+		// warm up the cache
+		for _, ptrn := range []int{ptr1, ptr2, ptr3, ptr4} {
+			n, err := st.Get(ptrn)
+			require.NoError(t, err)
+			require.NoError(t, c.Update(ptrn, n))
+		}
 
 		flushErr := errors.New("Store.Update() failed")
 		st.UpdateFn = func(ptr int, n *Node[rune, int, int]) error { return flushErr }

btree/iter_internal_test.go

@@ -504,6 +504,9 @@ func TestBackwardIter(t *testing.T) {
 			return &st.store[ptr], nil
 		}
 
+		// flush the cache
+		it.b.cache.lru.Close()
+
 		require.False(t, it.Next())
 		require.ErrorIs(t, it.Err(), getErr)
 	})

caching/lru/lru.go

@@ -5,54 +5,68 @@ import (
 	"sync/atomic"
 )
 
-type node[K comparable] struct {
+type node[K comparable, V any] struct {
 	key  K
-	next *node[K]
+	val  V
+	next *node[K, V]
+	prev *node[K, V]
 }
 
+// Cache implements a LRU caching strategy.  All methods beside Close
+// are safe for concurrent use.
 type Cache[K comparable, V any] struct {
-	mtx sync.RWMutex
+	mtx sync.Mutex
 
 	size   int
 	target int
 
 	onevict func(K, V) error
 
-	items map[K]V
-	head  *node[K]
-	tail  *node[K]
+	items map[K]*node[K, V]
+	head  *node[K, V]
+	tail  *node[K, V]
 
 	hits   atomic.Uint64
 	misses atomic.Uint64
 }
 
+// New constructs a cache of maximum size target.  The minimum size is
+// two, and any value minor than that will silently be bumped to at
+// least 2.  onevict is an optional callback that is called when
+// evicting an item from the cache.
 func New[K comparable, V any](target int, onevict func(K, V) error) *Cache[K, V] {
+	// it's simpler to assume that we'll always have something in
+	// the linked list, rather than trying to cope with
+	// patological cases like zero or one.
+	target = max(target, 2)
 	return &Cache[K, V]{
 		target:  target,
 		onevict: onevict,
-		items:   make(map[K]V, target),
+		items:   make(map[K]*node[K, V], target),
 	}
 }
 
 func (c *Cache[K, V]) put(key K, val V) {
 	c.size++
-	c.items[key] = val
-
-	n := &node[K]{key: key}
+	n := &node[K, V]{key: key, val: val}
 	if c.head == nil {
 		c.head = n
 		c.tail = n
 	} else {
-		c.tail.next = n
-		c.tail = n
+		n.next = c.head
+		c.head.prev = n
+		c.head = n
 	}
+
+	c.items[key] = n
 }
 
-// assume that the item was just removed from the linked list
-func (c *Cache[K, V]) flush(key K) error {
-	val := c.items[key]
+// evictItem removes an item from the `items' map, without touching
+// the list.
+func (c *Cache[K, V]) evictItem(key K) error {
+	n := c.items[key]
 	if c.onevict != nil {
-		if err := c.onevict(key, val); err != nil {
+		if err := c.onevict(key, n.val); err != nil {
 			return err
 		}
 	}
@@ -62,55 +76,95 @@ func (c *Cache[K, V]) flush(key K) error {
 	return nil
 }
 
+// Get retrieves a key from the cache, returning the value and true,
+// or the zero value and false if not found.  It also bumps the
+// "recent-ness" of the key.
 func (c *Cache[K, V]) Get(key K) (V, bool) {
-	c.mtx.RLock()
-	val, ok := c.items[key]
-	c.mtx.RUnlock()
+	c.mtx.Lock()
+	n, ok := c.items[key]
+	if ok && n != c.head {
+		// move it as the first item in the list
+		prev := n.prev
+		next := n.next
+		prev.next = next
+		if next != nil {
+			next.prev = prev
+		}
+
+		if c.tail == n {
+			c.tail = n.prev
+			c.tail.next = nil
+		}
+
+		n.next = c.head
+		c.head.prev = n
+		c.head = n
+		n.prev = nil
+	}
+	c.mtx.Unlock()
 
 	if ok {
 		c.hits.Add(1)
+		return n.val, true
 	} else {
 		c.misses.Add(1)
+		var zero V
+		return zero, false
 	}
-
-	return val, ok
 }
 
-// Put adds or overrides an element in the cache.
+// Put adds or overrides an element in the cache, eventually evicting
+// the less recent item in the cache.  It can only fail if the
+// `onevict` callback fails, in which case it aborts the operation.
 func (c *Cache[K, V]) Put(key K, val V) error {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
 	if _, ok := c.items[key]; ok {
-		c.items[key] = val
+		c.items[key].val = val
 		return nil
 	}
 
 	if c.size == c.target {
-		if err := c.flush(c.head.key); err != nil {
+		if err := c.evictItem(c.tail.key); err != nil {
 			return err
 		}
-		c.head = c.head.next
+		c.tail = c.tail.prev
+		if c.tail != nil {
+			c.tail.next.prev = nil
+			c.tail.next = nil
+		}
 	}
 
 	c.put(key, val)
 	return nil
 }
 
+// Close flushes all the items in the cache using the `onevict`
+// callback, if provided.  It is safe to reuse a cache after it has
+// been closed, provided that the size and `onevict` callback are
+// still fine: this method behaves like a reset.  It can only fail if
+// `onevict` fails, in which case it returns the last error.
 func (c *Cache[K, V]) Close() error {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
 
 	var err error
-	for n := c.head; n != nil; n = n.next {
-		if e := c.flush(n.key); e != nil {
-			err = e
+	if c.onevict != nil {
+		for key, n := range c.items {
+			if e := c.onevict(key, n.val); e != nil {
+				err = e
+			}
 		}
 	}
+	clear(c.items)
+	c.size = 0
 	c.head = nil
 	c.tail = nil
 	return err
 }
 
+// Stats return the number of cache hit, misses and the size of the
+// cache.  It does *not* reset them.
 func (c *Cache[K, V]) Stats() (hit, miss, size uint64) {
 	return c.hits.Load(), c.misses.Load(), uint64(c.size)
 }

caching/lru/lru_bench_test.go

@@ -0,0 +1,159 @@
+package lru
+
+import (
+	"fmt"
+	"testing"
+)
+
+func BenchmarkPut(b *testing.B) {
+	for _, target := range []int{16, 256, 4096} {
+		b.Run(fmt.Sprintf("target=%d", target), func(b *testing.B) {
+			cache := New[int, int](target, nil)
+			defer cache.Close()
+
+			b.ReportAllocs()
+			for i := 0; i < b.N; i++ {
+				if err := cache.Put(i, i); err != nil {
+					b.Fatal(err)
+				}
+			}
+		})
+	}
+}
+
+func BenchmarkPutUpdateExisting(b *testing.B) {
+	cache := New[int, int](256, nil)
+	defer cache.Close()
+
+	for i := range 256 {
+		if err := cache.Put(i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ReportAllocs()
+	for i := 0; b.Loop(); i++ {
+		if err := cache.Put(i%256, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkGetHit(b *testing.B) {
+	cache := New[int, int](256, nil)
+	defer cache.Close()
+
+	for i := range 256 {
+		if err := cache.Put(i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ReportAllocs()
+	for i := 0; b.Loop(); i++ {
+		_, _ = cache.Get(i % 256)
+	}
+}
+
+func BenchmarkGetMiss(b *testing.B) {
+	cache := New[int, int](256, nil)
+	defer cache.Close()
+
+	for i := range 256 {
+		if err := cache.Put(i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ReportAllocs()
+	for i := 0; b.Loop(); i++ {
+		_, _ = cache.Get(i + 1_000_000)
+	}
+}
+
+func BenchmarkGetParallel(b *testing.B) {
+	cache := New[int, int](256, nil)
+	defer cache.Close()
+
+	for i := range 256 {
+		if err := cache.Put(i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	b.RunParallel(func(pb *testing.PB) {
+		i := 0
+		for pb.Next() {
+			_, _ = cache.Get(i % 256)
+			i++
+		}
+	})
+}
+
+// BenchmarkPutEviction measures Put cost when every insert forces an
+// eviction, i.e. the cache is kept permanently full.
+func BenchmarkPutEviction(b *testing.B) {
+	const target = 256
+	cache := New[int, int](target, nil)
+	defer cache.Close()
+
+	for i := range target {
+		if err := cache.Put(i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ReportAllocs()
+	for i := 0; b.Loop(); i++ {
+		if err := cache.Put(target+i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkPutEvictionWithCallback(b *testing.B) {
+	const target = 256
+	cache := New(target, func(K int, V int) error { return nil })
+	defer cache.Close()
+
+	for i := range target {
+		if err := cache.Put(i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ReportAllocs()
+	for i := 0; b.Loop(); i++ {
+		if err := cache.Put(target+i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+// BenchmarkMixed simulates a workload with 90% reads and 10% writes,
+// where writes cycle through a fixed key space and steadily trigger
+// evictions once the cache is warm.
+func BenchmarkMixed(b *testing.B) {
+	const target = 512
+	cache := New[int, int](target, nil)
+	defer cache.Close()
+
+	for i := range target {
+		if err := cache.Put(i, i); err != nil {
+			b.Fatal(err)
+		}
+	}
+
+	b.ReportAllocs()
+	for i := 0; b.Loop(); i++ {
+		if i%10 == 0 {
+			if err := cache.Put(target+i, i); err != nil {
+				b.Fatal(err)
+			}
+		} else {
+			_, _ = cache.Get(i % target)
+		}
+	}
+}