diff options
| -rw-r--r-- | bug/bug.go | 19 | ||||
| -rw-r--r-- | cache/lru_id_cache.go | 64 | ||||
| -rw-r--r-- | cache/repo_cache.go | 20 | ||||
| -rw-r--r-- | cache/repo_cache_bug.go | 89 | ||||
| -rw-r--r-- | cache/repo_cache_test.go | 76 | ||||
| -rw-r--r-- | go.mod | 2 | ||||
| -rw-r--r-- | go.sum | 1 |
7 files changed, 241 insertions, 30 deletions
@@ -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 +} diff --git a/cache/lru_id_cache.go b/cache/lru_id_cache.go new file mode 100644 index 00000000..e31f2843 --- /dev/null +++ b/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) +} diff --git a/cache/repo_cache.go b/cache/repo_cache.go index 89772455..5fe1b798 100644 --- a/cache/repo_cache.go +++ b/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 } diff --git a/cache/repo_cache_bug.go b/cache/repo_cache_bug.go index bcbfcea3..57397361 100644 --- a/cache/repo_cache_bug.go +++ b/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) +} diff --git a/cache/repo_cache_test.go b/cache/repo_cache_test.go index 0deb155e..ba59a7df 100644 --- a/cache/repo_cache_test.go +++ b/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", "rene@descartes.fr") 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", "rene@descartes.fr") + 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) +} @@ -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 @@ -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= |