Implement the LRU Cache

vince created 5 years ago

Change summary

bug/bug.go               | 19 ++++++++
cache/lru_id_cache.go    | 64 ++++++++++++++++++++++++++++++
cache/repo_cache.go      | 20 ++++++++
cache/repo_cache_bug.go  | 89 +++++++++++++++++++++++++++++++----------
cache/repo_cache_test.go | 76 +++++++++++++++++++++++++++++++++-
go.mod                   |  2 
go.sum                   |  1 
7 files changed, 241 insertions(+), 30 deletions(-)

Detailed changes

bug/bug.go

@@ -749,3 +749,22 @@ func (bug *Bug) Compile() Snapshot {
 
 	return snap
 }
+
+// EquivalentBug returns true if two bugs are equal
+func EquivalentBug(expected, actual *Bug) bool {
+	if len(expected.packs) != len(actual.packs) {
+		return false
+	}
+
+	for i := range expected.packs {
+		for j := range expected.packs[i].Operations {
+			actual.packs[i].Operations[j].base().id = expected.packs[i].Operations[j].base().id
+		}
+	}
+
+	if expected != actual {
+		return false
+	}
+
+	return true
+}

cache/lru_id_cache.go

@@ -0,0 +1,64 @@
+package cache
+
+import (
+	lru "github.com/hashicorp/golang-lru"
+
+	"github.com/MichaelMure/git-bug/entity"
+)
+
+type LRUIdCache struct {
+	parentCache *lru.Cache
+	maxSize     int
+	onEvict     func(id entity.Id)
+}
+
+func NewLRUIdCache(size int, onEvicted func(id entity.Id)) (*LRUIdCache, error) {
+	cache, err := lru.New(size)
+	if err != nil {
+		return nil, err
+	}
+
+	return &LRUIdCache{
+		cache,
+		size,
+		onEvicted,
+	}, nil
+}
+
+func (c *LRUIdCache) Add(id entity.Id) bool {
+	return c.parentCache.Add(id, nil)
+}
+
+func (c *LRUIdCache) Contains(id entity.Id) bool {
+	return c.parentCache.Contains(id)
+}
+
+func (c *LRUIdCache) Get(id entity.Id) bool {
+	_, present := c.parentCache.Get(id)
+	return present
+}
+
+func (c *LRUIdCache) GetOldest() (entity.Id, bool) {
+	id, _, present := c.parentCache.GetOldest()
+	return id.(entity.Id), present
+}
+
+func (c *LRUIdCache) GetAll() (ids []entity.Id) {
+	interfaceKeys := c.parentCache.Keys()
+	for _, id := range interfaceKeys {
+		ids = append(ids, id.(entity.Id))
+	}
+	return
+}
+
+func (c *LRUIdCache) Len() int {
+	return c.parentCache.Len()
+}
+
+func (c *LRUIdCache) Remove(id entity.Id) bool {
+	return c.parentCache.Remove(id)
+}
+
+func (c *LRUIdCache) Resize(size int) int {
+	return c.parentCache.Resize(size)
+}

cache/repo_cache.go

@@ -21,6 +21,8 @@ import (
 // 2: added cache for identities with a reference in the bug cache
 const formatVersion = 2
 
+const lruCacheSize = 100
+
 var _ repository.RepoCommon = &RepoCache{}
 
 // RepoCache is a cache for a Repository. This cache has multiple functions:
@@ -50,6 +52,9 @@ type RepoCache struct {
 	// bug loaded in memory
 	bugs map[entity.Id]*BugCache
 
+	// presentBugs is an LRU cache that records which bugs the cache has loaded in
+	presentBugs *LRUIdCache
+
 	muIdentity sync.RWMutex
 	// excerpt of identities data for all identities
 	identitiesExcerpts map[entity.Id]*IdentityExcerpt
@@ -144,7 +149,7 @@ func (c *RepoCache) Close() error {
 
 	c.identities = make(map[entity.Id]*IdentityCache)
 	c.identitiesExcerpts = nil
-	c.bugs = make(map[entity.Id]*BugCache)
+	c.bugs = nil
 	c.bugExcerpts = nil
 
 	lockPath := repoLockFilePath(c.repo)
@@ -175,11 +180,22 @@ func (c *RepoCache) buildCache() error {
 
 	_, _ = fmt.Fprintf(os.Stderr, "Building bug cache... ")
 
+	presentBugs, err := NewLRUIdCache(lruCacheSize, c.onEvict)
+	if err != nil {
+		return err
+	}
+	c.presentBugs = presentBugs
+
 	c.bugExcerpts = make(map[entity.Id]*BugExcerpt)
 
 	allBugs := bug.ReadAllLocalBugs(c.repo)
 
-	for b := range allBugs {
+	for i := 0; i < lruCacheSize; i++ {
+		if len(allBugs) == 0 {
+			break
+		}
+
+		b := <-allBugs
 		if b.Err != nil {
 			return b.Err
 		}

cache/repo_cache_bug.go

@@ -24,14 +24,13 @@ func bugCacheFilePath(repo repository.Repo) string {
 // bugUpdated is a callback to trigger when the excerpt of a bug changed,
 // that is each time a bug is updated
 func (c *RepoCache) bugUpdated(id entity.Id) error {
-	c.muBug.Lock()
-
-	b, ok := c.bugs[id]
-	if !ok {
-		c.muBug.Unlock()
-		panic("missing bug in the cache")
+	err := c.ensureBugLoaded(id)
+	if err != nil {
+		return err
 	}
 
+	c.muBug.Lock()
+	b, _ := c.bugs[id]
 	c.bugExcerpts[id] = NewBugExcerpt(b.bug, b.Snapshot())
 	c.muBug.Unlock()
 
@@ -106,38 +105,59 @@ func (c *RepoCache) writeBugCache() error {
 
 // ResolveBugExcerpt retrieve a BugExcerpt matching the exact given id
 func (c *RepoCache) ResolveBugExcerpt(id entity.Id) (*BugExcerpt, error) {
-	c.muBug.RLock()
-	defer c.muBug.RUnlock()
-
-	e, ok := c.bugExcerpts[id]
-	if !ok {
-		return nil, bug.ErrBugNotExist
+	err := c.ensureBugLoaded(id)
+	if err != nil {
+		return nil, err
 	}
 
-	return e, nil
+	c.muBug.RLock()
+	defer c.muBug.RUnlock()
+	return c.bugExcerpts[id], nil
 }
 
 // ResolveBug retrieve a bug matching the exact given id
 func (c *RepoCache) ResolveBug(id entity.Id) (*BugCache, error) {
+	err := c.ensureBugLoaded(id)
+	if err != nil {
+		return nil, err
+	}
+
 	c.muBug.RLock()
-	cached, ok := c.bugs[id]
-	c.muBug.RUnlock()
-	if ok {
-		return cached, nil
+	defer c.muBug.RUnlock()
+	return c.bugs[id], nil
+}
+
+func (c *RepoCache) ensureBugLoaded(id entity.Id) error {
+	if c.presentBugs.Get(id) {
+		return nil
 	}
 
 	b, err := bug.ReadLocalBug(c.repo, id)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	cached = NewBugCache(c, b)
+	bugCache := NewBugCache(c, b)
 
 	c.muBug.Lock()
-	c.bugs[id] = cached
-	c.muBug.Unlock()
+	if c.presentBugs.Len() == c.presentBugs.maxSize {
+		for _, id := range c.presentBugs.GetAll() {
+			if b := c.bugs[id]; !b.NeedCommit() {
+				b.mu.Lock()
+				c.presentBugs.Remove(id)
+				delete(c.bugExcerpts, id)
+				delete(c.bugs, id)
+			}
+		}
+	}
 
-	return cached, nil
+	c.presentBugs.Add(id)
+	c.bugs[id] = bugCache
+	excerpt := NewBugExcerpt(b, bugCache.Snapshot()) // TODO: Is this needed?
+	c.bugExcerpts[id] = excerpt
+
+	c.muBug.Unlock()
+	return nil
 }
 
 // ResolveBugExcerptPrefix retrieve a BugExcerpt matching an id prefix. It fails if multiple
@@ -363,15 +383,38 @@ func (c *RepoCache) NewBugRaw(author *IdentityCache, unixTime int64, title strin
 // RemoveBug removes a bug from the cache and repo given a bug id prefix
 func (c *RepoCache) RemoveBug(prefix string) error {
 	b, err := c.ResolveBugPrefix(prefix)
+	if err != nil {
+		return err
+	}
 
+	err = c.ensureBugLoaded(b.Id())
 	if err != nil {
 		return err
 	}
 
+	c.muBug.Lock()
+	b.mu.Lock()
+	fmt.Println("got lock")
 	err = bug.RemoveBug(c.repo, b.Id())
-
+	if err != nil {
+		c.muBug.Unlock()
+		b.mu.Unlock()
+		return err
+	}
+	fmt.Println("noerr")
+	c.presentBugs.Remove(b.Id())
+	fmt.Println("removing1")
 	delete(c.bugs, b.Id())
+	fmt.Println("removed2")
 	delete(c.bugExcerpts, b.Id())
+	fmt.Println("unlocking")
 
+	c.muBug.Unlock()
 	return c.writeBugCache()
 }
+
+// onEvict will update the bugs and bugExcerpts when a bug is evicted from the cache
+func (c *RepoCache) onEvict(id entity.Id) { // TODO: Do we need this?
+	delete(c.bugs, id)
+	delete(c.bugExcerpts, id)
+}

cache/repo_cache_test.go

@@ -1,9 +1,7 @@
 package cache
 
 import (
-	"fmt"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -181,13 +179,16 @@ func TestRemove(t *testing.T) {
 	rene, err := repoCache.NewIdentity("René Descartes", "[e-mail]")
 	require.NoError(t, err)
 
+	err = repoCache.SetUserIdentity(rene)
+	require.NoError(t, err)
+
 	for i := 0; i < 100; i++ {
-		_, _, err := repoCache.NewBugRaw(rene, time.Now().Unix(), "title", fmt.Sprintf("message%v", i), nil, nil)
+		_, _, err := repoCache.NewBug("title", "message")
 		require.NoError(t, err)
 	}
 
 	// and one more for testing
-	b1, _, err := repoCache.NewBugRaw(rene, time.Now().Unix(), "title", "message", nil, nil)
+	b1, _, err := repoCache.NewBug("title", "message")
 	require.NoError(t, err)
 
 	_, err = repoCache.Push("remoteA")
@@ -210,3 +211,70 @@ func TestRemove(t *testing.T) {
 	_, err = repoCache.ResolveBug(b1.Id())
 	assert.Error(t, bug.ErrBugNotExist, err)
 }
+
+func TestCacheEviction(t *testing.T) {
+	repo := repository.CreateTestRepo(false)
+	repoCache, err := NewRepoCache(repo)
+	require.NoError(t, err)
+	repoCache.presentBugs.Resize(2)
+
+	require.Equal(t, 0, repoCache.presentBugs.Len())
+	require.Equal(t, 0, len(repoCache.bugs))
+	require.Equal(t, 0, len(repoCache.bugExcerpts))
+
+	// Generating some bugs
+	rene, err := repoCache.NewIdentity("René Descartes", "[e-mail]")
+	require.NoError(t, err)
+	err = repoCache.SetUserIdentity(rene)
+	require.NoError(t, err)
+
+	bug1, _, err := repoCache.NewBug("title", "message")
+	require.NoError(t, err)
+
+	checkBugPresence(t, repoCache, bug1, true)
+	require.Equal(t, 1, repoCache.presentBugs.Len())
+	require.Equal(t, 1, len(repoCache.bugs))
+	require.Equal(t, 1, len(repoCache.bugExcerpts))
+
+	bug2, _, err := repoCache.NewBug("title", "message")
+	require.NoError(t, err)
+
+	checkBugPresence(t, repoCache, bug1, true)
+	checkBugPresence(t, repoCache, bug2, true)
+	require.Equal(t, 2, repoCache.presentBugs.Len())
+	require.Equal(t, 2, len(repoCache.bugs))
+	require.Equal(t, 2, len(repoCache.bugExcerpts))
+
+	// Number of bugs should not exceed max size of lruCache, oldest one should be evicted
+	bug3, _, err := repoCache.NewBug("title", "message")
+	require.NoError(t, err)
+
+	checkBugPresence(t, repoCache, bug1, false)
+	checkBugPresence(t, repoCache, bug2, true)
+	checkBugPresence(t, repoCache, bug3, true)
+	require.Equal(t, 2, repoCache.presentBugs.Len())
+	require.Equal(t, 2, len(repoCache.bugs))
+	require.Equal(t, 2, len(repoCache.bugExcerpts))
+
+	// Accessing bug should update position in lruCache and therefore it should not be evicted
+	repoCache.presentBugs.Get(bug2.Id())
+	oldestId, _ := repoCache.presentBugs.GetOldest()
+	require.Equal(t, bug3.Id(), oldestId)
+
+	checkBugPresence(t, repoCache, bug1, false)
+	checkBugPresence(t, repoCache, bug2, true)
+	checkBugPresence(t, repoCache, bug3, true)
+	require.Equal(t, 2, repoCache.presentBugs.Len())
+	require.Equal(t, 2, len(repoCache.bugs))
+	require.Equal(t, 2, len(repoCache.bugExcerpts))
+}
+
+func checkBugPresence(t *testing.T, cache *RepoCache, bug *BugCache, presence bool) {
+	id := bug.Id()
+	require.Equal(t, presence, cache.presentBugs.Contains(id))
+	b, ok := cache.bugs[id]
+	require.Equal(t, presence, ok)
+	require.Equal(t, bug, b)
+	_, ok = cache.bugExcerpts[id]
+	require.Equal(t, presence, ok)
+}

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/dustin/go-humanize v1.0.0
 	github.com/fatih/color v1.9.0
 	github.com/gorilla/mux v1.7.4
-	github.com/hashicorp/golang-lru v0.5.4 // indirect
+	github.com/hashicorp/golang-lru v0.5.4
 	github.com/icrowley/fake v0.0.0-20180203215853-4178557ae428
 	github.com/mattn/go-isatty v0.0.12
 	github.com/phayes/freeport v0.0.0-20171002181615-b8543db493a5

go.sum

@@ -109,6 +109,7 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf
 github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
 github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
+github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI=
 github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
 github.com/hashicorp/go-retryablehttp v0.6.4 h1:BbgctKO892xEyOXnGiaAwIoSq1QZ/SS4AhjoAh9DnfY=
 github.com/hashicorp/go-retryablehttp v0.6.4/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=