diff options
-rw-r--r-- | cache/bug_cache.go | 2 | ||||
-rw-r--r-- | cache/lru_id_cache.go | 13 | ||||
-rw-r--r-- | cache/repo_cache.go | 16 | ||||
-rw-r--r-- | cache/repo_cache_bug.go | 92 | ||||
-rw-r--r-- | cache/repo_cache_test.go | 25 |
5 files changed, 73 insertions, 75 deletions
diff --git a/cache/bug_cache.go b/cache/bug_cache.go index 8365b3f9..ca526f7b 100644 --- a/cache/bug_cache.go +++ b/cache/bug_cache.go @@ -37,8 +37,6 @@ func (c *BugCache) Snapshot() *bug.Snapshot { } func (c *BugCache) Id() entity.Id { - c.mu.RLock() - defer c.mu.RUnlock() return c.bug.Id() } diff --git a/cache/lru_id_cache.go b/cache/lru_id_cache.go index e31f2843..a387cb75 100644 --- a/cache/lru_id_cache.go +++ b/cache/lru_id_cache.go @@ -9,20 +9,16 @@ import ( 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 - } +func NewLRUIdCache(size int) *LRUIdCache { + // Ignore error here + cache, _ := lru.New(size) return &LRUIdCache{ cache, size, - onEvicted, - }, nil + } } func (c *LRUIdCache) Add(id entity.Id) bool { @@ -60,5 +56,6 @@ func (c *LRUIdCache) Remove(id entity.Id) bool { } func (c *LRUIdCache) Resize(size int) int { + c.maxSize = size return c.parentCache.Resize(size) } diff --git a/cache/repo_cache.go b/cache/repo_cache.go index 5fe1b798..f6ff1abe 100644 --- a/cache/repo_cache.go +++ b/cache/repo_cache.go @@ -82,6 +82,9 @@ func NewNamedRepoCache(r repository.ClockedRepo, name string) (*RepoCache, error return &RepoCache{}, err } + presentBugs := NewLRUIdCache(lruCacheSize) + c.presentBugs = presentBugs + err = c.load() if err == nil { return c, nil @@ -180,22 +183,11 @@ 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 i := 0; i < lruCacheSize; i++ { - if len(allBugs) == 0 { - break - } - - b := <-allBugs + for b := range allBugs { if b.Err != nil { return b.Err } diff --git a/cache/repo_cache_bug.go b/cache/repo_cache_bug.go index 57397361..40081e6e 100644 --- a/cache/repo_cache_bug.go +++ b/cache/repo_cache_bug.go @@ -3,6 +3,7 @@ package cache import ( "bytes" "encoding/gob" + "errors" "fmt" "os" "path" @@ -17,6 +18,9 @@ import ( const bugCacheFile = "bug-cache" +var errBugNotInCache = errors.New("bug missing from cache") +var errCantEvictBug = errors.New("unable to evict a bug from the cache") + func bugCacheFilePath(repo repository.Repo) string { return path.Join(repo.GetPath(), "git-bug", bugCacheFile) } @@ -24,13 +28,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 { - err := c.ensureBugLoaded(id) - if err != nil { - return err - } - c.muBug.Lock() - b, _ := c.bugs[id] + b, ok := c.bugs[id] + if !ok { + c.muBug.Unlock() + return errBugNotInCache + } + c.presentBugs.Get(id) c.bugExcerpts[id] = NewBugExcerpt(b.bug, b.Snapshot()) c.muBug.Unlock() @@ -105,14 +109,14 @@ func (c *RepoCache) writeBugCache() error { // ResolveBugExcerpt retrieve a BugExcerpt matching the exact given id func (c *RepoCache) ResolveBugExcerpt(id entity.Id) (*BugExcerpt, error) { - err := c.ensureBugLoaded(id) - if err != nil { - return nil, err - } - c.muBug.RLock() defer c.muBug.RUnlock() - return c.bugExcerpts[id], nil + excerpt, ok := c.bugExcerpts[id] + if !ok { + return nil, errBugNotInCache + } + c.presentBugs.Get(id) + return excerpt, nil } // ResolveBug retrieve a bug matching the exact given id @@ -127,6 +131,8 @@ func (c *RepoCache) ResolveBug(id entity.Id) (*BugCache, error) { return c.bugs[id], nil } +// ensureBugLoaded ensures a bug (if it exists) is loaded into the cache +// it will automatically evict a bug if needed func (c *RepoCache) ensureBugLoaded(id entity.Id) error { if c.presentBugs.Get(id) { return nil @@ -139,27 +145,40 @@ func (c *RepoCache) ensureBugLoaded(id entity.Id) error { bugCache := NewBugCache(c, b) - c.muBug.Lock() - 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) - } - } + err = c.evictIfNeeded() + if err != nil { + return err } + c.muBug.Lock() + defer c.muBug.Unlock() 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 } +// evictIfNeeded will evict a bug from the cache if needed +// it also removes references of the bug from the bugs and bugExcerpts +func (c *RepoCache) evictIfNeeded() error { + c.muBug.Lock() + defer c.muBug.Unlock() + if c.presentBugs.Len() < c.presentBugs.maxSize { + return nil + } + + 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 nil + } + } + + return errCantEvictBug +} + // ResolveBugExcerptPrefix retrieve a BugExcerpt matching an id prefix. It fails if multiple // bugs match. func (c *RepoCache) ResolveBugExcerptPrefix(prefix string) (*BugExcerpt, error) { @@ -361,13 +380,15 @@ func (c *RepoCache) NewBugRaw(author *IdentityCache, unixTime int64, title strin return nil, nil, err } - c.muBug.Lock() - if _, has := c.bugs[b.Id()]; has { - c.muBug.Unlock() - return nil, nil, fmt.Errorf("bug %s already exist in the cache", b.Id()) + if other, err := c.ResolveBug(b.Id()); other != nil { + if err != nil { + return nil, nil, err + } } cached := NewBugCache(c, b) + + c.muBug.Lock() c.bugs[b.Id()] = cached c.muBug.Unlock() @@ -394,27 +415,16 @@ func (c *RepoCache) RemoveBug(prefix string) error { 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 ba59a7df..426279e1 100644 --- a/cache/repo_cache_test.go +++ b/cache/repo_cache_test.go @@ -83,7 +83,8 @@ func TestCache(t *testing.T) { require.Empty(t, cache.identitiesExcerpts) // Reload, only excerpt are loaded - require.NoError(t, cache.load()) + cache, err = NewRepoCache(repo) + require.NoError(t, err) require.Empty(t, cache.bugs) require.Empty(t, cache.identities) require.Len(t, cache.bugExcerpts, 2) @@ -175,17 +176,14 @@ func TestRemove(t *testing.T) { repoCache, err := NewRepoCache(repo) require.NoError(t, err) - // generate a bunch of bugs 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.NewBug("title", "message") - require.NoError(t, err) - } + _, _, err = repoCache.NewBug("title", "message") + require.NoError(t, err) // and one more for testing b1, _, err := repoCache.NewBug("title", "message") @@ -205,8 +203,8 @@ func TestRemove(t *testing.T) { err = repoCache.RemoveBug(b1.Id().String()) require.NoError(t, err) - assert.Equal(t, 100, len(repoCache.bugs)) - assert.Equal(t, 100, len(repoCache.bugExcerpts)) + assert.Equal(t, 1, len(repoCache.bugs)) + assert.Equal(t, 1, len(repoCache.bugExcerpts)) _, err = repoCache.ResolveBug(b1.Id()) assert.Error(t, bug.ErrBugNotExist, err) @@ -218,6 +216,7 @@ func TestCacheEviction(t *testing.T) { require.NoError(t, err) repoCache.presentBugs.Resize(2) + require.Equal(t, 2, repoCache.presentBugs.maxSize) require.Equal(t, 0, repoCache.presentBugs.Len()) require.Equal(t, 0, len(repoCache.bugs)) require.Equal(t, 0, len(repoCache.bugExcerpts)) @@ -249,12 +248,12 @@ func TestCacheEviction(t *testing.T) { 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)) + checkBugPresence(t, repoCache, bug1, false) + checkBugPresence(t, repoCache, bug2, true) + checkBugPresence(t, repoCache, bug3, true) // Accessing bug should update position in lruCache and therefore it should not be evicted repoCache.presentBugs.Get(bug2.Id()) @@ -274,7 +273,9 @@ func checkBugPresence(t *testing.T, cache *RepoCache, bug *BugCache, presence bo require.Equal(t, presence, cache.presentBugs.Contains(id)) b, ok := cache.bugs[id] require.Equal(t, presence, ok) - require.Equal(t, bug, b) + if ok { + require.Equal(t, bug, b) + } _, ok = cache.bugExcerpts[id] require.Equal(t, presence, ok) } |