Merge pull request #75 from bluele/lfu-remove-empty-entry

lfu: remove an empty `freqEntry` in `freqList`

Author: bluele

lfu.go

@@ -12,6 +12,21 @@ type LFUCache struct {
 	freqList *list.List // list for freqEntry
 }
 
+var _ Cache = (*LFUCache)(nil)
+
+type lfuItem struct {
+	clock       Clock
+	key         interface{}
+	value       interface{}
+	freqElement *list.Element
+	expiration  *time.Time
+}
+
+type freqEntry struct {
+	freq  uint
+	items map[*lfuItem]struct{}
+}
+
 func newLFUCache(cb *CacheBuilder) *LFUCache {
 	c := &LFUCache{}
 	buildCache(&c.baseCache, cb)
@@ -23,7 +38,7 @@ func newLFUCache(cb *CacheBuilder) *LFUCache {
 
 func (c *LFUCache) init() {
 	c.freqList = list.New()
-	c.items = make(map[interface{}]*lfuItem, c.size+1)
+	c.items = make(map[interface{}]*lfuItem, c.size)
 	c.freqList.PushFront(&freqEntry{
 		freq:  0,
 		items: make(map[*lfuItem]struct{}),
@@ -183,12 +198,28 @@ func (c *LFUCache) increment(item *lfuItem) {
 	nextFreq := currentFreqEntry.freq + 1
 	delete(currentFreqEntry.items, item)
 
+	// a boolean whether reuse the empty current entry
+	removable := isRemovableFreqEntry(currentFreqEntry)
+
+	// insert item into a valid entry
 	nextFreqElement := currentFreqElement.Next()
-	if nextFreqElement == nil {
-		nextFreqElement = c.freqList.InsertAfter(&freqEntry{
-			freq:  nextFreq,
-			items: make(map[*lfuItem]struct{}),
-		}, currentFreqElement)
+	switch {
+	case nextFreqElement == nil || nextFreqElement.Value.(*freqEntry).freq > nextFreq:
+		if removable {
+			currentFreqEntry.freq = nextFreq
+			nextFreqElement = currentFreqElement
+		} else {
+			nextFreqElement = c.freqList.InsertAfter(&freqEntry{
+				freq:  nextFreq,
+				items: make(map[*lfuItem]struct{}),
+			}, currentFreqElement)
+		}
+	case nextFreqElement.Value.(*freqEntry).freq == nextFreq:
+		if removable {
+			c.freqList.Remove(currentFreqElement)
+		}
+	default:
+		panic("unreachable")
 	}
 	nextFreqElement.Value.(*freqEntry).items[item] = struct{}{}
 	item.freqElement = nextFreqElement
@@ -201,7 +232,7 @@ func (c *LFUCache) evict(count int) {
 		if entry == nil {
 			return
 		} else {
-			for item, _ := range entry.Value.(*freqEntry).items {
+			for item := range entry.Value.(*freqEntry).items {
 				if i >= count {
 					return
 				}
@@ -247,8 +278,12 @@ func (c *LFUCache) remove(key interface{}) bool {
 
 // removeElement is used to remove a given list element from the cache
 func (c *LFUCache) removeItem(item *lfuItem) {
+	entry := item.freqElement.Value.(*freqEntry)
 	delete(c.items, item.key)
-	delete(item.freqElement.Value.(*freqEntry).items, item)
+	delete(entry.items, item)
+	if isRemovableFreqEntry(entry) {
+		c.freqList.Remove(item.freqElement)
+	}
 	if c.evictedFunc != nil {
 		c.evictedFunc(item.key, item.value)
 	}
@@ -325,19 +360,6 @@ func (c *LFUCache) Purge() {
 	c.init()
 }
 
-type freqEntry struct {
-	freq  uint
-	items map[*lfuItem]struct{}
-}
-
-type lfuItem struct {
-	clock       Clock
-	key         interface{}
-	value       interface{}
-	freqElement *list.Element
-	expiration  *time.Time
-}
-
 // IsExpired returns boolean value whether this item is expired or not.
 func (it *lfuItem) IsExpired(now *time.Time) bool {
 	if it.expiration == nil {
@@ -349,3 +371,7 @@ func (it *lfuItem) IsExpired(now *time.Time) bool {
 	}
 	return it.expiration.Before(*now)
 }
+
+func isRemovableFreqEntry(entry *freqEntry) bool {
+	return entry.freq != 0 && len(entry.items) == 0
+}

lfu_test.go

@@ -83,3 +83,114 @@ func TestLFUHas(t *testing.T) {
 		})
 	}
 }
+
+func TestLFUFreqListOrder(t *testing.T) {
+	gc := buildTestCache(t, TYPE_LFU, 5)
+	for i := 4; i >= 0; i-- {
+		gc.Set(i, i)
+		for j := 0; j <= i; j++ {
+			gc.Get(i)
+		}
+	}
+	if l := gc.(*LFUCache).freqList.Len(); l != 6 {
+		t.Fatalf("%v != 6", l)
+	}
+	var i uint
+	for e := gc.(*LFUCache).freqList.Front(); e != nil; e = e.Next() {
+		if e.Value.(*freqEntry).freq != i {
+			t.Fatalf("%v != %v", e.Value.(*freqEntry).freq, i)
+		}
+		i++
+	}
+	gc.Remove(1)
+
+	if l := gc.(*LFUCache).freqList.Len(); l != 5 {
+		t.Fatalf("%v != 5", l)
+	}
+	gc.Set(1, 1)
+	if l := gc.(*LFUCache).freqList.Len(); l != 5 {
+		t.Fatalf("%v != 5", l)
+	}
+	gc.Get(1)
+	if l := gc.(*LFUCache).freqList.Len(); l != 5 {
+		t.Fatalf("%v != 5", l)
+	}
+	gc.Get(1)
+	if l := gc.(*LFUCache).freqList.Len(); l != 6 {
+		t.Fatalf("%v != 6", l)
+	}
+}
+
+func TestLFUFreqListLength(t *testing.T) {
+	k0, v0 := "k0", "v0"
+	k1, v1 := "k1", "v1"
+
+	{
+		gc := buildTestCache(t, TYPE_LFU, 5)
+		if l := gc.(*LFUCache).freqList.Len(); l != 1 {
+			t.Fatalf("%v != 1", l)
+		}
+	}
+	{
+		gc := buildTestCache(t, TYPE_LFU, 5)
+		gc.Set(k0, v0)
+		for i := 0; i < 5; i++ {
+			gc.Get(k0)
+		}
+		if l := gc.(*LFUCache).freqList.Len(); l != 2 {
+			t.Fatalf("%v != 2", l)
+		}
+	}
+
+	{
+		gc := buildTestCache(t, TYPE_LFU, 5)
+		gc.Set(k0, v0)
+		gc.Set(k1, v1)
+		for i := 0; i < 5; i++ {
+			gc.Get(k0)
+			gc.Get(k1)
+		}
+		if l := gc.(*LFUCache).freqList.Len(); l != 2 {
+			t.Fatalf("%v != 2", l)
+		}
+	}
+
+	{
+		gc := buildTestCache(t, TYPE_LFU, 5)
+		gc.Set(k0, v0)
+		gc.Set(k1, v1)
+		for i := 0; i < 5; i++ {
+			gc.Get(k0)
+		}
+		if l := gc.(*LFUCache).freqList.Len(); l != 2 {
+			t.Fatalf("%v != 2", l)
+		}
+		for i := 0; i < 5; i++ {
+			gc.Get(k1)
+		}
+		if l := gc.(*LFUCache).freqList.Len(); l != 2 {
+			t.Fatalf("%v != 2", l)
+		}
+	}
+
+	{
+		gc := buildTestCache(t, TYPE_LFU, 5)
+		gc.Set(k0, v0)
+		gc.Get(k0)
+		if l := gc.(*LFUCache).freqList.Len(); l != 2 {
+			t.Fatalf("%v != 2", l)
+		}
+		gc.Remove(k0)
+		if l := gc.(*LFUCache).freqList.Len(); l != 1 {
+			t.Fatalf("%v != 1", l)
+		}
+		gc.Set(k0, v0)
+		if l := gc.(*LFUCache).freqList.Len(); l != 1 {
+			t.Fatalf("%v != 1", l)
+		}
+		gc.Get(k0)
+		if l := gc.(*LFUCache).freqList.Len(); l != 2 {
+			t.Fatalf("%v != 2", l)
+		}
+	}
+}