Slightly simplify LastCommitCache (#20444)

The LastCommitCache code is a little complex and there is unnecessary
duplication between the gogit and nogogit variants.

This PR adds the LastCommitCache as a field to the git.Repository and
pre-creates it in the ReferencesGit helpers etc. There has been some
simplification and unification of the variant code.

Signed-off-by: Andrew Thornton <art27@cantab.net>

modules/context/repo.go

@@ -1001,6 +1001,8 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 			return
 		}
 		ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount
+		ctx.Repo.GitRepo.LastCommitCache = git.NewLastCommitCache(ctx.Repo.CommitsCount, ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, cache.GetCache())
+
 		return cancel
 	}
 }

modules/git/commit.go

@@ -80,6 +80,9 @@ func (c *Commit) ParentCount() int {
 
 // GetCommitByPath return the commit of relative path object.
 func (c *Commit) GetCommitByPath(relpath string) (*Commit, error) {
+	if c.repo.LastCommitCache != nil {
+		return c.repo.LastCommitCache.GetCommitByPath(c.ID.String(), relpath)
+	}
 	return c.repo.getCommitByPathWithID(c.ID, relpath)
 }
 

modules/git/commit_info_gogit.go

@@ -17,7 +17,7 @@ import (
 )
 
 // GetCommitsInfo gets information of all commits that are corresponding to these entries
-func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string, cache *LastCommitCache) ([]CommitInfo, *Commit, error) {
+func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
 	entryPaths := make([]string, len(tes)+1)
 	// Get the commit for the treePath itself
 	entryPaths[0] = ""
@@ -35,15 +35,15 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 		return nil, nil, err
 	}
 
-	var revs map[string]*object.Commit
-	if cache != nil {
+	var revs map[string]*Commit
+	if commit.repo.LastCommitCache != nil {
 		var unHitPaths []string
-		revs, unHitPaths, err = getLastCommitForPathsByCache(commit.ID.String(), treePath, entryPaths, cache)
+		revs, unHitPaths, err = getLastCommitForPathsByCache(commit.ID.String(), treePath, entryPaths, commit.repo.LastCommitCache)
 		if err != nil {
 			return nil, nil, err
 		}
 		if len(unHitPaths) > 0 {
-			revs2, err := GetLastCommitForPaths(ctx, cache, c, treePath, unHitPaths)
+			revs2, err := GetLastCommitForPaths(ctx, commit.repo.LastCommitCache, c, treePath, unHitPaths)
 			if err != nil {
 				return nil, nil, err
 			}
@@ -68,8 +68,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 		}
 
 		// Check if we have found a commit for this entry in time
-		if rev, ok := revs[entry.Name()]; ok {
-			entryCommit := convertCommit(rev)
+		if entryCommit, ok := revs[entry.Name()]; ok {
 			commitsInfo[i].Commit = entryCommit
 		}
 
@@ -96,10 +95,10 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 	// get it for free during the tree traversal and it's used for listing
 	// pages to display information about newest commit for a given path.
 	var treeCommit *Commit
+	var ok bool
 	if treePath == "" {
 		treeCommit = commit
-	} else if rev, ok := revs[""]; ok {
-		treeCommit = convertCommit(rev)
+	} else if treeCommit, ok = revs[""]; ok {
 		treeCommit.repo = commit.repo
 	}
 	return commitsInfo, treeCommit, nil
@@ -155,16 +154,16 @@ func getFileHashes(c cgobject.CommitNode, treePath string, paths []string) (map[
 	return hashes, nil
 }
 
-func getLastCommitForPathsByCache(commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*object.Commit, []string, error) {
+func getLastCommitForPathsByCache(commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) {
 	var unHitEntryPaths []string
-	results := make(map[string]*object.Commit)
+	results := make(map[string]*Commit)
 	for _, p := range paths {
 		lastCommit, err := cache.Get(commitID, path.Join(treePath, p))
 		if err != nil {
 			return nil, nil, err
 		}
 		if lastCommit != nil {
-			results[p] = lastCommit.(*object.Commit)
+			results[p] = lastCommit
 			continue
 		}
 
@@ -175,7 +174,7 @@ func getLastCommitForPathsByCache(commitID, treePath string, paths []string, cac
 }
 
 // GetLastCommitForPaths returns last commit information
-func GetLastCommitForPaths(ctx context.Context, cache *LastCommitCache, c cgobject.CommitNode, treePath string, paths []string) (map[string]*object.Commit, error) {
+func GetLastCommitForPaths(ctx context.Context, cache *LastCommitCache, c cgobject.CommitNode, treePath string, paths []string) (map[string]*Commit, error) {
 	refSha := c.ID().String()
 
 	// We do a tree traversal with nodes sorted by commit time
@@ -293,13 +292,13 @@ heaploop:
 	}
 
 	// Post-processing
-	result := make(map[string]*object.Commit)
+	result := make(map[string]*Commit)
 	for path, commitNode := range resultNodes {
-		var err error
-		result[path], err = commitNode.Commit()
+		commit, err := commitNode.Commit()
 		if err != nil {
 			return nil, err
 		}
+		result[path] = convertCommit(commit)
 	}
 
 	return result, nil

modules/git/commit_info_nogogit.go

@@ -17,7 +17,7 @@ import (
 )
 
 // GetCommitsInfo gets information of all commits that are corresponding to these entries
-func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string, cache *LastCommitCache) ([]CommitInfo, *Commit, error) {
+func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) {
 	entryPaths := make([]string, len(tes)+1)
 	// Get the commit for the treePath itself
 	entryPaths[0] = ""
@@ -28,15 +28,15 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 	var err error
 
 	var revs map[string]*Commit
-	if cache != nil {
+	if commit.repo.LastCommitCache != nil {
 		var unHitPaths []string
-		revs, unHitPaths, err = getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryPaths, cache)
+		revs, unHitPaths, err = getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryPaths, commit.repo.LastCommitCache)
 		if err != nil {
 			return nil, nil, err
 		}
 		if len(unHitPaths) > 0 {
 			sort.Strings(unHitPaths)
-			commits, err := GetLastCommitForPaths(ctx, cache, commit, treePath, unHitPaths)
+			commits, err := GetLastCommitForPaths(ctx, commit, treePath, unHitPaths)
 			if err != nil {
 				return nil, nil, err
 			}
@@ -47,7 +47,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 		}
 	} else {
 		sort.Strings(entryPaths)
-		revs, err = GetLastCommitForPaths(ctx, nil, commit, treePath, entryPaths)
+		revs, err = GetLastCommitForPaths(ctx, commit, treePath, entryPaths)
 	}
 	if err != nil {
 		return nil, nil, err
@@ -99,18 +99,15 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
 }
 
 func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) {
-	wr, rd, cancel := cache.repo.CatFileBatch(ctx)
-	defer cancel()
-
 	var unHitEntryPaths []string
 	results := make(map[string]*Commit)
 	for _, p := range paths {
-		lastCommit, err := cache.Get(commitID, path.Join(treePath, p), wr, rd)
+		lastCommit, err := cache.Get(commitID, path.Join(treePath, p))
 		if err != nil {
 			return nil, nil, err
 		}
 		if lastCommit != nil {
-			results[p] = lastCommit.(*Commit)
+			results[p] = lastCommit
 			continue
 		}
 
@@ -121,9 +118,9 @@ func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string
 }
 
 // GetLastCommitForPaths returns last commit information
-func GetLastCommitForPaths(ctx context.Context, cache *LastCommitCache, commit *Commit, treePath string, paths []string) (map[string]*Commit, error) {
+func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string, paths []string) (map[string]*Commit, error) {
 	// We read backwards from the commit to obtain all of the commits
-	revs, err := WalkGitLog(ctx, cache, commit.repo, commit, treePath, paths...)
+	revs, err := WalkGitLog(ctx, commit.repo, commit, treePath, paths...)
 	if err != nil {
 		return nil, err
 	}

modules/git/commit_info_test.go

@@ -91,7 +91,7 @@ func testGetCommitsInfo(t *testing.T, repo1 *Repository) {
 		}
 
 		// FIXME: Context.TODO() - if graceful has started we should use its Shutdown context otherwise use install signals in TestMain.
-		commitsInfo, treeCommit, err := entries.GetCommitsInfo(context.TODO(), commit, testCase.Path, nil)
+		commitsInfo, treeCommit, err := entries.GetCommitsInfo(context.TODO(), commit, testCase.Path)
 		assert.NoError(t, err, "Unable to get commit information for entries of subtree: %s in commit: %s from testcase due to error: %v", testCase.Path, testCase.CommitID, err)
 		if err != nil {
 			t.FailNow()
@@ -170,7 +170,7 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) {
 		b.ResetTimer()
 		b.Run(benchmark.name, func(b *testing.B) {
 			for i := 0; i < b.N; i++ {
-				_, _, err := entries.GetCommitsInfo(context.Background(), commit, "", nil)
+				_, _, err := entries.GetCommitsInfo(context.Background(), commit, "")
 				if err != nil {
 					b.Fatal(err)
 				}

modules/git/last_commit_cache.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 )
 
 // Cache represents a caching interface
@@ -19,16 +20,96 @@ type Cache interface {
 	Get(key string) interface{}
 }
 
-func (c *LastCommitCache) getCacheKey(repoPath, ref, entryPath string) string {
-	hashBytes := sha256.Sum256([]byte(fmt.Sprintf("%s:%s:%s", repoPath, ref, entryPath)))
+func getCacheKey(repoPath, commitID, entryPath string) string {
+	hashBytes := sha256.Sum256([]byte(fmt.Sprintf("%s:%s:%s", repoPath, commitID, entryPath)))
 	return fmt.Sprintf("last_commit:%x", hashBytes)
 }
 
+// LastCommitCache represents a cache to store last commit
+type LastCommitCache struct {
+	repoPath    string
+	ttl         func() int64
+	repo        *Repository
+	commitCache map[string]*Commit
+	cache       Cache
+}
+
+// NewLastCommitCache creates a new last commit cache for repo
+func NewLastCommitCache(count int64, repoPath string, gitRepo *Repository, cache Cache) *LastCommitCache {
+	if cache == nil {
+		return nil
+	}
+	if !setting.CacheService.LastCommit.Enabled || count < setting.CacheService.LastCommit.CommitsCount {
+		return nil
+	}
+
+	return &LastCommitCache{
+		repoPath: repoPath,
+		repo:     gitRepo,
+		ttl:      setting.LastCommitCacheTTLSeconds,
+		cache:    cache,
+	}
+}
+
 // Put put the last commit id with commit and entry path
 func (c *LastCommitCache) Put(ref, entryPath, commitID string) error {
 	if c == nil || c.cache == nil {
 		return nil
 	}
 	log.Debug("LastCommitCache save: [%s:%s:%s]", ref, entryPath, commitID)
-	return c.cache.Put(c.getCacheKey(c.repoPath, ref, entryPath), commitID, c.ttl())
+	return c.cache.Put(getCacheKey(c.repoPath, ref, entryPath), commitID, c.ttl())
+}
+
+// Get gets the last commit information by commit id and entry path
+func (c *LastCommitCache) Get(ref, entryPath string) (*Commit, error) {
+	if c == nil || c.cache == nil {
+		return nil, nil
+	}
+
+	commitID, ok := c.cache.Get(getCacheKey(c.repoPath, ref, entryPath)).(string)
+	if !ok || commitID == "" {
+		return nil, nil
+	}
+
+	log.Debug("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, commitID)
+	if c.commitCache != nil {
+		if commit, ok := c.commitCache[commitID]; ok {
+			log.Debug("LastCommitCache hit level 2: [%s:%s:%s]", ref, entryPath, commitID)
+			return commit, nil
+		}
+	}
+
+	commit, err := c.repo.GetCommit(commitID)
+	if err != nil {
+		return nil, err
+	}
+	if c.commitCache == nil {
+		c.commitCache = make(map[string]*Commit)
+	}
+	c.commitCache[commitID] = commit
+	return commit, nil
+}
+
+// GetCommitByPath gets the last commit for the entry in the provided commit
+func (c *LastCommitCache) GetCommitByPath(commitID, entryPath string) (*Commit, error) {
+	sha1, err := NewIDFromString(commitID)
+	if err != nil {
+		return nil, err
+	}
+
+	lastCommit, err := c.Get(sha1.String(), entryPath)
+	if err != nil || lastCommit != nil {
+		return lastCommit, err
+	}
+
+	lastCommit, err = c.repo.getCommitByPathWithID(sha1, entryPath)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := c.Put(commitID, entryPath, lastCommit.ID.String()); err != nil {
+		log.Error("Unable to cache %s as the last commit for %q in %s %s. Error %v", lastCommit.ID.String(), entryPath, commitID, c.repoPath, err)
+	}
+
+	return lastCommit, nil
 }