Improve lru code and add tests

lru/lru.go

@@ -1,111 +1,140 @@
-// A simple LRU cache for storing documents ([]byte). When the size maximum is reached, items are evicted
-// starting with the least recently used. This data structure is goroutine-safe (it has a lock around all
-// operations).
+// Package lru provides a simple LRU cache that keys []byte values by strings.
 package lru
 
 import (
-	"container/list"
 	"sync"
 )
 
 type cacheValue struct {
-	key   string
-	bytes []byte
+	key        string
+	val        []byte
+	next, prev *cacheValue
 }
 
-// Just an estimate
-func (v *cacheValue) size() uint64 {
-	return uint64(len([]byte(v.key)) + len(v.bytes))
+func (v *cacheValue) size() int64 {
+	return int64(len([]byte(v.key)) + len(v.val))
 }
 
-type Cache struct {
-	sync.Mutex
+type cacheValueList struct {
+	front *cacheValue
+	back  *cacheValue
+}
 
-	// The approximate size of the structure (doesn't include the overhead of the data structures; just the
-	// sum of the size of the stored documents).
-	Size uint64
+func (l *cacheValueList) pushFront(v *cacheValue) {
+	v.next = l.front
+	v.prev = nil
+	if l.front == nil {
+		l.back = v
+	} else {
+		l.front.prev = v
+	}
+	l.front = v
+}
 
-	capacity uint64
-	list     *list.List
-	table    map[string]*list.Element
+func (l *cacheValueList) moveToFront(v *cacheValue) {
+	if v.prev == nil {
+		return
+	}
+	v.prev.next = v.next
+	if v.next == nil {
+		l.back = v.prev
+	} else {
+		v.next.prev = v.prev
+	}
+	v.prev = nil
+	v.next = l.front
+	if l.front != nil {
+		l.front.prev = v
+	}
+	l.front = v
 }
 
-// Create a new Cache with a maximum size of capacity bytes.
-func New(capacity uint64) *Cache {
-	return &Cache{
-		capacity: capacity,
-		list:     list.New(),
-		table:    make(map[string]*list.Element),
+func (l *cacheValueList) delete(v *cacheValue) {
+	if v.prev == nil {
+		l.front = v.next
+	} else {
+		v.prev.next = v.next
+	}
+	if v.next == nil {
+		l.back = v.prev
+	} else {
+		v.next.prev = v.prev
 	}
 }
 
-// Insert some {key, document} into the cache. Doesn't do anything if the key is already present.
-func (c *Cache) Insert(key string, document []byte) {
-	c.Lock()
-	defer c.Unlock()
+// A Cache is a size-bounded LRU cache which associates string keys
+// with []byte values.
+// All methods are safe for concurrent use by multiple goroutines.
+type Cache struct {
+	mu sync.Mutex
 
-	_, ok := c.table[key]
-	if ok {
-		return
+	size     int64
+	capacity int64
+	list     cacheValueList
+	table    map[string]*cacheValue
+}
+
+// New creates a new Cache with a maximum size of capacity bytes.
+func New(capacity int64) *Cache {
+	if capacity < 0 {
+		panic("lru: bad capacity")
+	}
+	return &Cache{
+		capacity: capacity,
+		table:    make(map[string]*cacheValue),
 	}
-	v := &cacheValue{key, document}
-	elt := c.list.PushFront(v)
-	c.table[key] = elt
-	c.Size += v.size()
-	c.trim()
 }
 
-// Get retrieves a value from the cache and returns the value and an indicator boolean to show whether it was
-// present.
-func (c *Cache) Get(key string) (document []byte, ok bool) {
-	c.Lock()
-	defer c.Unlock()
+// Insert adds val to the cache indexed by the given key after evicting enough
+// existing items (least recently used first) to keep the total size beneath
+// this cache's capacity.
+// It does not do anything if the key is already present in the cache or if the
+// size of this single item is greater than the cache's capacity.
+func (c *Cache) Insert(key string, val []byte) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
 
-	elt, ok := c.table[key]
-	if !ok {
-		return nil, false
+	if _, ok := c.table[key]; ok {
+		return
+	}
+	v := &cacheValue{key: key, val: val}
+	if v.size() > c.capacity {
+		return
+	}
+	for c.size+v.size() > c.capacity {
+		c.size -= c.list.back.size()
+		delete(c.table, c.list.back.key)
+		c.list.delete(c.list.back)
 	}
-	c.list.MoveToFront(elt)
-	return elt.Value.(*cacheValue).bytes, true
+	c.list.pushFront(v)
+	c.table[key] = v
+	c.size += v.size()
 }
 
-// If the key is present, move that document to the front of the list to show that it was most recently used.
-func (c *Cache) Update(key string) {
-	c.Lock()
-	defer c.Unlock()
+// Get retrieves a value from the cache by key and indicates
+// whether an item was found.
+func (c *Cache) Get(key string) (val []byte, ok bool) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
 
-	elt, ok := c.table[key]
+	v, ok := c.table[key]
 	if !ok {
-		return
+		return nil, false
 	}
-	c.list.MoveToFront(elt)
+	c.list.moveToFront(v)
+	return v.val, true
 }
 
-// Delete the document indicated by the key, if it is present.
+// Delete removes the item indicated by the key, if it is present.
 func (c *Cache) Delete(key string) {
-	c.Lock()
-	defer c.Unlock()
+	c.mu.Lock()
+	defer c.mu.Unlock()
 
-	elt, ok := c.table[key]
+	v, ok := c.table[key]
 	if !ok {
 		return
 	}
 	delete(c.table, key)
-	v := c.list.Remove(elt).(*cacheValue)
-	c.Size -= v.size()
-}
-
-// If the cache is over capacity, clear elements (starting at the end of the list) until it is back under
-// capacity. Note that this method is not threadsafe (it should only be called from other methods which
-// already hold the lock).
-func (c *Cache) trim() {
-	for c.Size > c.capacity {
-		elt := c.list.Back()
-		if elt == nil {
-			return
-		}
-		v := c.list.Remove(elt).(*cacheValue)
-		delete(c.table, v.key)
-		c.Size -= v.size()
-	}
+	c.list.delete(v)
+	c.size -= v.size()
 }

lru/lru_test.go

@@ -0,0 +1,131 @@
+package lru
+
+import (
+	"bytes"
+	"reflect"
+	"testing"
+)
+
+func TestCacheValueList(t *testing.T) {
+	var l cacheValueList
+
+	check := func(want ...string) {
+		t.Helper()
+		var got []string
+		var prev *cacheValue
+		for v := l.front; v != nil; v = v.next {
+			if v.prev != prev {
+				t.Fatalf("bad prev pointer: got %p; want %p", v.prev, prev)
+			}
+			if string(v.val) != v.key+v.key {
+				t.Fatalf("mismatched value: key=%s; bytes=%s", v.key, v.val)
+			}
+			got = append(got, v.key)
+			prev = v
+		}
+		if l.back != prev {
+			t.Fatalf("bad back pointer: got %p; want %p", l.back, prev)
+		}
+		if !reflect.DeepEqual(got, want) {
+			t.Errorf("wrong list contents: got %q; want %q", got, want)
+		}
+	}
+
+	a := &cacheValue{key: "a", val: []byte("aa")}
+	b := &cacheValue{key: "b", val: []byte("bb")}
+	c := &cacheValue{key: "c", val: []byte("cc")}
+
+	check()
+	l.pushFront(a)
+	check("a")
+	l.moveToFront(a)
+	check("a")
+	l.pushFront(b)
+	check("b", "a")
+	l.moveToFront(b)
+	check("b", "a")
+	l.moveToFront(a)
+	check("a", "b")
+	l.pushFront(c)
+	check("c", "a", "b")
+	l.moveToFront(c)
+	check("c", "a", "b")
+	l.moveToFront(b)
+	check("b", "c", "a")
+	l.moveToFront(c)
+	check("c", "b", "a")
+
+	l.delete(c)
+	check("b", "a")
+	l.pushFront(c)
+	check("c", "b", "a")
+	l.delete(b)
+	check("c", "a")
+	l.pushFront(b)
+	check("b", "c", "a")
+	l.delete(a)
+	check("b", "c")
+	l.delete(b)
+	check("c")
+	l.pushFront(b)
+	check("b", "c")
+	l.delete(c)
+	check("b")
+	l.delete(b)
+	check()
+}
+
+func TestCache(t *testing.T) {
+	c := New(10)
+
+	exists := func(key string, val []byte) {
+		t.Helper()
+		got, ok := c.Get(key)
+		if !ok || !bytes.Equal(got, val) {
+			t.Errorf("Get(%q) gave %q, %t; want %q, true", key, got, ok, val)
+		}
+	}
+	notExists := func(key string) {
+		t.Helper()
+		if _, ok := c.Get(key); ok {
+			t.Errorf("Get(%q) gave ok=true; want ok=false", key)
+		}
+	}
+
+	key4, val4 := "4", []byte("4__")
+	key5, val5 := "5", []byte("5___")
+	key6, val6 := "6", []byte("6____")
+
+	notExists(key4)
+
+	c.Insert(key4, val4)
+	exists(key4, val4)
+
+	c.Insert(key4, nil)
+	exists(key4, val4)
+
+	c.Insert(key5, make([]byte, 10))
+	notExists(key5)
+
+	c.Insert(key5, val5)
+	exists(key4, val4)
+	exists(key5, val5)
+
+	c.Insert(key6, val6)
+	exists(key6, val6)
+	notExists(key4)
+	notExists(key5)
+
+	c.Insert(key4, val4)
+	exists(key6, val6)
+	exists(key4, val4)
+
+	c.Insert(key5, val5)
+	notExists(key6)
+	exists(key4, val4)
+	exists(key5, val5)
+
+	c.Delete(key4)
+	notExists(key4)
+	c.Delete(key4)
+}

pastedown.go

@@ -193,7 +193,8 @@ func pastieHandler(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "No such file.", http.StatusNotFound)
 		return
 	}
-	renderCache.Update(filename)
+	// Fetch the value to mark it as recently used.
+	_, _ = renderCache.Get(filename)
 	w.Write(contents)
 }