Merge pull request #1215 from jmank88/mem-cache-opt

gps: source cache: memory cache optimization

Author: sdboyer

internal/gps/source.go

@@ -494,8 +494,10 @@ func (sg *sourceGateway) listVersions(ctx context.Context) ([]PairedVersion, err
 	if err != nil {
 		return nil, err
 	}
-
-	return sg.cache.getAllVersions(), nil
+	if pvs, ok := sg.cache.getAllVersions(); ok {
+		return pvs, nil
+	}
+	return nil, nil
 }
 
 func (sg *sourceGateway) revisionPresentIn(ctx context.Context, r Revision) (bool, error) {

internal/gps/source_cache.go

@@ -43,7 +43,7 @@ type singleSourceCache interface {
 	getVersionsFor(Revision) ([]UnpairedVersion, bool)
 
 	// Gets all the version pairs currently known to the cache.
-	getAllVersions() []PairedVersion
+	getAllVersions() ([]PairedVersion, bool)
 
 	// Get the revision corresponding to the given unpaired version.
 	getRevisionFor(UnpairedVersion) (Revision, bool)
@@ -61,9 +61,10 @@ type singleSourceCache interface {
 }
 
 type singleSourceCacheMemory struct {
-	mut    sync.RWMutex // protects all maps
+	mut    sync.RWMutex // protects all fields
 	infos  map[ProjectAnalyzerInfo]map[Revision]projectInfo
 	ptrees map[Revision]pkgtree.PackageTree
+	vList  []PairedVersion // replaced, never modified
 	vMap   map[UnpairedVersion]Revision
 	rMap   map[Revision][]UnpairedVersion
 }
@@ -136,13 +137,14 @@ func (c *singleSourceCacheMemory) getPackageTree(r Revision) (pkgtree.PackageTre
 
 func (c *singleSourceCacheMemory) setVersionMap(versionList []PairedVersion) {
 	c.mut.Lock()
+	c.vList = versionList
 	// TODO(sdboyer) how do we handle cache consistency here - revs that may
 	// be out of date vis-a-vis the ptrees or infos maps?
 	for r := range c.rMap {
 		c.rMap[r] = nil
 	}
 
-	c.vMap = make(map[UnpairedVersion]Revision)
+	c.vMap = make(map[UnpairedVersion]Revision, len(versionList))
 
 	for _, pv := range versionList {
 		u, r := pv.Unpair(), pv.Revision()
@@ -167,15 +169,17 @@ func (c *singleSourceCacheMemory) getVersionsFor(r Revision) ([]UnpairedVersion,
 	return versionList, has
 }
 
-func (c *singleSourceCacheMemory) getAllVersions() []PairedVersion {
-	if len(c.vMap) == 0 {
-		return nil
-	}
-	vlist := make([]PairedVersion, 0, len(c.vMap))
-	for v, r := range c.vMap {
-		vlist = append(vlist, v.Pair(r))
+func (c *singleSourceCacheMemory) getAllVersions() ([]PairedVersion, bool) {
+	c.mut.Lock()
+	vList := c.vList
+	c.mut.Unlock()
+
+	if vList == nil {
+		return nil, false
 	}
-	return vlist
+	cp := make([]PairedVersion, len(vList))
+	copy(cp, vList)
+	return cp, true
 }
 
 func (c *singleSourceCacheMemory) getRevisionFor(uv UnpairedVersion) (Revision, bool) {

internal/gps/source_cache_bolt.go

@@ -351,8 +351,7 @@ func (s *singleSourceCacheBolt) getVersionsFor(rev Revision) (uvs []UnpairedVers
 	return
 }
 
-func (s *singleSourceCacheBolt) getAllVersions() []PairedVersion {
-	var pvs []PairedVersion
+func (s *singleSourceCacheBolt) getAllVersions() (pvs []PairedVersion, ok bool) {
 	err := s.viewSourceBucket(func(src *bolt.Bucket) error {
 		versions := cacheFindLatestValid(src, cacheVersion, s.epoch)
 		if versions == nil {
@@ -369,14 +368,15 @@ func (s *singleSourceCacheBolt) getAllVersions() []PairedVersion {
 				return err
 			}
 			pvs = append(pvs, uv.Pair(Revision(v)))
+			ok = true
 			return nil
 		})
 	})
 	if err != nil {
 		s.logger.Println(errors.Wrap(err, "failed to get all cached versions"))
-		return nil
+		return nil, false
 	}
-	return pvs
+	return
 }
 
 func (s *singleSourceCacheBolt) getRevisionFor(uv UnpairedVersion) (rev Revision, ok bool) {

internal/gps/source_cache_bolt_test.go

@@ -119,8 +119,8 @@ func TestBoltCacheTimeout(t *testing.T) {
 		}
 		comparePackageTree(t, ptree, got)
 
-		gotV := c.getAllVersions()
-		if len(gotV) != len(pvs) {
+		gotV, ok := c.getAllVersions()
+		if !ok || len(gotV) != len(pvs) {
 			t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", gotV, pvs)
 		} else {
 			SortPairedForDowngrade(gotV)
@@ -161,8 +161,8 @@ func TestBoltCacheTimeout(t *testing.T) {
 		}
 		comparePackageTree(t, ptree, gotPtree)
 
-		pvs := c.getAllVersions()
-		if len(pvs) > 0 {
+		pvs, ok := c.getAllVersions()
+		if ok || len(pvs) > 0 {
 			t.Errorf("expected no cached versions, but got:\n\t%#v", pvs)
 		}
 	}
@@ -194,8 +194,8 @@ func TestBoltCacheTimeout(t *testing.T) {
 		}
 		comparePackageTree(t, ptree, got)
 
-		gotV := c.getAllVersions()
-		if len(gotV) != len(pvs) {
+		gotV, ok := c.getAllVersions()
+		if !ok || len(gotV) != len(pvs) {
 			t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", gotV, pvs)
 		} else {
 			SortPairedForDowngrade(gotV)
@@ -282,8 +282,8 @@ func TestBoltCacheTimeout(t *testing.T) {
 		}
 		comparePackageTree(t, newPtree, got)
 
-		gotV := c.getAllVersions()
-		if len(gotV) != len(newPVS) {
+		gotV, ok := c.getAllVersions()
+		if !ok || len(gotV) != len(newPVS) {
 			t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", gotV, newPVS)
 		} else {
 			SortPairedForDowngrade(gotV)

internal/gps/source_cache_multi.go

@@ -77,19 +77,19 @@ func (c *multiCache) getVersionsFor(rev Revision) ([]UnpairedVersion, bool) {
 	return c.disk.getVersionsFor(rev)
 }
 
-func (c *multiCache) getAllVersions() []PairedVersion {
-	pvs := c.mem.getAllVersions()
-	if pvs != nil {
-		return pvs
+func (c *multiCache) getAllVersions() ([]PairedVersion, bool) {
+	pvs, ok := c.mem.getAllVersions()
+	if ok {
+		return pvs, true
 	}
 
-	pvs = c.disk.getAllVersions()
-	if pvs != nil {
+	pvs, ok = c.disk.getAllVersions()
+	if ok {
 		c.mem.setVersionMap(pvs)
-		return pvs
+		return pvs, true
 	}
 
-	return nil
+	return nil, false
 }
 
 func (c *multiCache) getRevisionFor(uv UnpairedVersion) (Revision, bool) {

internal/gps/source_cache_test.go

@@ -305,8 +305,8 @@ func (test singleSourceCacheTest) run(t *testing.T) {
 		}
 
 		t.Run("getAllVersions", func(t *testing.T) {
-			got := c.getAllVersions()
-			if len(got) != len(versions) {
+			got, ok := c.getAllVersions()
+			if !ok || len(got) != len(versions) {
 				t.Errorf("unexpected versions:\n\t(GOT): %#v\n\t(WNT): %#v", got, versions)
 			} else {
 				SortPairedForDowngrade(got)
@@ -566,8 +566,8 @@ func (discardCache) getVersionsFor(Revision) ([]UnpairedVersion, bool) {
 	return nil, false
 }
 
-func (discardCache) getAllVersions() []PairedVersion {
-	return nil
+func (discardCache) getAllVersions() ([]PairedVersion, bool) {
+	return nil, false
 }
 
 func (discardCache) getRevisionFor(UnpairedVersion) (Revision, bool) {

internal/gps/source_test.go

@@ -60,8 +60,8 @@ func testSourceGateway(t *testing.T) {
 				t.Fatalf("error on cloning git repo: %s", err)
 			}
 
-			cvlist := sg.cache.getAllVersions()
-			if len(cvlist) != 4 {
+			cvlist, ok := sg.cache.getAllVersions()
+			if !ok || len(cvlist) != 4 {
 				t.Fatalf("repo setup should've cached four versions, got %v: %s", len(cvlist), cvlist)
 			}