LRU cache with maximum size

This changes the cache to use a LRU policy with a maximum number of entries. This maximum is for the moment static when instantiating the cache.
It also removes the goroutine that cleans the expired items by a simple check when getting elements as we can simply check the tail expiration.

Fixes #273

cache.go

@@ -4,6 +4,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/dedis/onet/log"
 	uuid "github.com/satori/go.uuid"
 )
 
@@ -12,103 +13,144 @@ import (
 // The implementation is robust against concurrent call
 type cacheTTL struct {
 	entries         map[uuid.UUID]*cacheTTLEntry
-	stopCh          chan (struct{})
-	stopOnce        sync.Once
-	cleanInterval   time.Duration
+	head            *cacheTTLEntry
+	tail            *cacheTTLEntry
 	entryExpiration time.Duration
+	size            int
 	sync.Mutex
 }
 
 type cacheTTLEntry struct {
+	key        uuid.UUID
 	item       interface{}
 	expiration time.Time
+	prev       *cacheTTLEntry
+	next       *cacheTTLEntry
 }
 
 // create a generic cache and start the cleaning routine
-func newCacheTTL(interval, expiration time.Duration) *cacheTTL {
-	c := &cacheTTL{
+func newCacheTTL(expiration time.Duration, size int) *cacheTTL {
+	if size == 0 {
+		log.Error("Cannot instantiate a cache with a size of 0")
+		return nil
+	}
+
+	return &cacheTTL{
 		entries:         make(map[uuid.UUID]*cacheTTLEntry),
-		stopCh:          make(chan (struct{})),
-		cleanInterval:   interval,
 		entryExpiration: expiration,
+		size:            size,
 	}
-	go c.cleaner()
-	return c
-}
-
-// Stop the cleaning routine
-func (c *cacheTTL) stop() {
-	c.stopOnce.Do(func() {
-		close(c.stopCh)
-	})
 }
 
-// Wait for either a clean or a stop order
-func (c *cacheTTL) cleaner() {
-	for {
-		select {
-		case <-time.After(c.cleanInterval):
-			c.clean()
-		case <-c.stopCh:
-			return
+// add the item to the cache with the given key
+func (c *cacheTTL) set(key uuid.UUID, item interface{}) {
+	entry := c.entries[key]
+	if entry != nil {
+		entry.expiration = time.Now().Add(c.entryExpiration)
+		entry.item = item
+	} else {
+		entry = &cacheTTLEntry{
+			key:        key,
+			item:       item,
+			expiration: time.Now().Add(c.entryExpiration),
 		}
-	}
-}
 
-// Check and delete expired items
-func (c *cacheTTL) clean() {
-	c.Lock()
-	now := time.Now()
-	for k, e := range c.entries {
-		if now.After(e.expiration) {
-			delete(c.entries, k)
+		c.clean() // clean before checking the size
+		if len(c.entries) >= c.size && c.tail != nil {
+			// deletes the oldest entry and ignore edge cases with size = 0
+			delete(c.entries, c.tail.key)
+			c.tail = c.tail.next
+			c.tail.prev = nil
 		}
 	}
-	c.Unlock()
-}
 
-// add the item to the cache with the given key
-func (c *cacheTTL) set(key uuid.UUID, item interface{}) {
-	c.Lock()
-	c.entries[key] = &cacheTTLEntry{
-		item:       item,
-		expiration: time.Now().Add(c.entryExpiration),
-	}
-	c.Unlock()
+	c.moveToHead(entry)
+	c.entries[key] = entry
 }
 
 // returns the cached item or nil
 func (c *cacheTTL) get(key uuid.UUID) interface{} {
-	c.Lock()
-	defer c.Unlock()
-
 	entry, ok := c.entries[key]
 	if ok && time.Now().Before(entry.expiration) {
 		entry.expiration = time.Now().Add(c.entryExpiration)
+		c.moveToHead(entry)
 		return entry.item
 	}
 
+	c.clean() // defensive cleaning
 	return nil
 }
 
+func (c *cacheTTL) moveToHead(e *cacheTTLEntry) {
+	if c.head == e {
+		// already at the top of the list
+		return
+	}
+
+	if c.tail == e && e.next != nil {
+		// item was the tail so we need to assign the new tail
+		c.tail = e.next
+	}
+	// remove the list entry from its previous position
+	if e.next != nil {
+		e.next.prev = e.prev
+	}
+	if e.prev != nil {
+		e.prev.next = e.next
+	}
+
+	// assign the entry at the top of the list
+	if c.head == nil {
+		c.head = e
+		if c.tail == nil {
+			c.tail = c.head
+		}
+	} else {
+		c.head.next = e
+		e.prev = c.head
+		e.next = nil
+		c.head = e
+	}
+}
+
+func (c *cacheTTL) clean() {
+	now := time.Now()
+
+	for c.tail != nil && now.After(c.tail.expiration) {
+		delete(c.entries, c.tail.key)
+
+		if c.head == c.tail {
+			c.head = nil
+			c.tail = nil
+		} else {
+			c.tail = c.tail.next
+		}
+	}
+}
+
 type treeCacheTTL struct {
 	*cacheTTL
 }
 
-func newTreeCache(interval, expiration time.Duration) *treeCacheTTL {
+func newTreeCache(expiration time.Duration, size int) *treeCacheTTL {
 	return &treeCacheTTL{
-		cacheTTL: newCacheTTL(interval, expiration),
+		cacheTTL: newCacheTTL(expiration, size),
 	}
 }
 
 // Set stores the given tree in the cache
 func (c *treeCacheTTL) Set(tree *Tree) {
+	c.Lock()
 	c.set(uuid.UUID(tree.ID), tree)
+	c.Unlock()
 }
 
 // Get retrieves the tree with the given ID if it exists
 // or returns nil
 func (c *treeCacheTTL) Get(id TreeID) *Tree {
+	c.Lock()
+	defer c.Unlock()
+
 	tree := c.get(uuid.UUID(id))
 	if tree != nil {
 		return tree.(*Tree)
@@ -126,20 +168,25 @@ type rosterCacheTTL struct {
 	*cacheTTL
 }
 
-func newRosterCache(interval, expiration time.Duration) *rosterCacheTTL {
+func newRosterCache(expiration time.Duration, size int) *rosterCacheTTL {
 	return &rosterCacheTTL{
-		cacheTTL: newCacheTTL(interval, expiration),
+		cacheTTL: newCacheTTL(expiration, size),
 	}
 }
 
 // Set stores the roster in the cache
 func (c *rosterCacheTTL) Set(roster *Roster) {
+	c.Lock()
 	c.set(uuid.UUID(roster.ID), roster)
+	c.Unlock()
 }
 
 // Get retrieves the Roster with the given ID if it exists
 // or it returns nil
 func (c *rosterCacheTTL) Get(id RosterID) *Roster {
+	c.Lock()
+	defer c.Unlock()
+
 	roster := c.get(uuid.UUID(id))
 	if roster != nil {
 		return roster.(*Roster)
@@ -161,22 +208,23 @@ type treeNodeCacheTTL struct {
 	*cacheTTL
 }
 
-func newTreeNodeCache(interval, expiration time.Duration) *treeNodeCacheTTL {
+func newTreeNodeCache(expiration time.Duration, size int) *treeNodeCacheTTL {
 	return &treeNodeCacheTTL{
-		cacheTTL: newCacheTTL(interval, expiration),
+		cacheTTL: newCacheTTL(expiration, size),
 	}
 }
 
 func (c *treeNodeCacheTTL) Set(tree *Tree, treeNode *TreeNode) {
 	c.Lock()
-	ce, ok := c.entries[uuid.UUID(tree.ID)]
-	if !ok {
-		ce = &cacheTTLEntry{
-			item:       make(map[TreeNodeID]*TreeNode),
-			expiration: time.Now().Add(c.entryExpiration),
-		}
+
+	var treeNodeMap map[TreeNodeID]*TreeNode
+	e := c.get(uuid.UUID(tree.ID))
+	if e == nil {
+		treeNodeMap = make(map[TreeNodeID]*TreeNode)
+	} else {
+		treeNodeMap = e.(map[TreeNodeID]*TreeNode)
 	}
-	treeNodeMap := ce.item.(map[TreeNodeID]*TreeNode)
+
 	// add treenode
 	treeNodeMap[treeNode.ID] = treeNode
 	// add parent if not root
@@ -188,24 +236,24 @@ func (c *treeNodeCacheTTL) Set(tree *Tree, treeNode *TreeNode) {
 		treeNodeMap[c.ID] = c
 	}
 	// add cache
-	c.entries[uuid.UUID(tree.ID)] = ce
+	c.set(uuid.UUID(tree.ID), treeNodeMap)
 	c.Unlock()
 }
 
 func (c *treeNodeCacheTTL) GetFromToken(tok *Token) *TreeNode {
 	c.Lock()
 	defer c.Unlock()
+
 	if tok == nil {
 		return nil
 	}
-	ce, ok := c.entries[uuid.UUID(tok.TreeID)]
-	if !ok || time.Now().After(ce.expiration) {
-		// no tree cached for this token
+
+	e := c.get(uuid.UUID(tok.TreeID))
+	if e == nil {
 		return nil
 	}
-	ce.expiration = time.Now().Add(c.entryExpiration)
 
-	treeNodeMap := ce.item.(map[TreeNodeID]*TreeNode)
+	treeNodeMap := e.(map[TreeNodeID]*TreeNode)
 	tn, ok := treeNodeMap[tok.TreeNodeID]
 	if !ok {
 		// no treeNode cached for this token

cache_test.go

@@ -10,6 +10,7 @@ import (
 )
 
 const nbrOfItems = 20
+const size = 20
 
 func initTrees(tt []*Tree) {
 	for i := range tt {
@@ -20,8 +21,7 @@ func initTrees(tt []*Tree) {
 }
 
 func TestTreeCache(t *testing.T) {
-	cache := newTreeCache(25*time.Millisecond, 100*time.Millisecond)
-	defer cache.stop()
+	cache := newTreeCache(100*time.Millisecond, size)
 
 	trees := make([]*Tree, nbrOfItems)
 	initTrees(trees)
@@ -61,11 +61,12 @@ func TestTreeCache(t *testing.T) {
 		token.TreeID = tree.ID
 		require.Nil(t, cache.GetFromToken(token))
 	}
+
+	require.Equal(t, 0, len(cache.entries))
 }
 
 func TestRosterCache(t *testing.T) {
-	cache := newRosterCache(1*time.Minute, 50*time.Millisecond)
-	defer cache.stop()
+	cache := newRosterCache(50*time.Millisecond, size)
 
 	r := &Roster{}
 	id, _ := uuid.NewV1()
@@ -88,8 +89,7 @@ func generateID() uuid.UUID {
 }
 
 func TestTreeNodeCache(t *testing.T) {
-	cache := newTreeNodeCache(1*time.Minute, 50*time.Millisecond)
-	defer cache.stop()
+	cache := newTreeNodeCache(50*time.Millisecond, size)
 
 	tree := &Tree{ID: TreeID(generateID())}
 	tn1 := &TreeNode{ID: TreeNodeID(generateID())}
@@ -113,8 +113,7 @@ func TestTreeNodeCache(t *testing.T) {
 }
 
 func TestExpirationAndCleaning(t *testing.T) {
-	cache := newTreeCache(25*time.Millisecond, 100*time.Millisecond)
-	defer cache.stop()
+	cache := newTreeCache(100*time.Millisecond, size)
 
 	tt := make([]*Tree, 2)
 	initTrees(tt)
@@ -130,3 +129,25 @@ func TestExpirationAndCleaning(t *testing.T) {
 	token.TreeID = tt[0].ID
 	require.Nil(t, cache.GetFromToken(token))
 }
+
+func TestCacheSize(t *testing.T) {
+	size := 5
+	cache := newTreeCache(50*time.Millisecond, size)
+
+	tt := make([]*Tree, 10)
+	initTrees(tt)
+
+	for _, t := range tt {
+		cache.Set(t)
+		cache.Get(tt[1].ID)
+	}
+
+	require.Equal(t, size, len(cache.entries))
+	require.Nil(t, cache.Get(tt[0].ID))
+	require.Equal(t, tt[9], cache.Get(tt[9].ID))
+	require.Equal(t, tt[1], cache.Get(tt[1].ID))
+
+	time.Sleep(60 * time.Millisecond)
+	require.Nil(t, cache.Get(tt[1].ID))
+	require.Equal(t, 0, len(cache.entries))
+}