Skip to content

Commit

Permalink
Defer Last Commit Info (go-gitea#16467)
Browse files Browse the repository at this point in the history
One of the biggest reasons for slow repository browsing is that we wait
until last commit information has been generated for all files in the
repository.

This PR proposes deferring this generation to a new POST endpoint that
does the look up outside of the main page request.

Signed-off-by: Andrew Thornton <[email protected]>
  • Loading branch information
zeripath authored Oct 8, 2021
1 parent 88fa9f3 commit 001dbf1
Show file tree
Hide file tree
Showing 13 changed files with 321 additions and 151 deletions.
61 changes: 35 additions & 26 deletions modules/git/commit_info_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,17 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
return nil, nil, err
}
if len(unHitPaths) > 0 {
revs2, err := GetLastCommitForPaths(ctx, c, treePath, unHitPaths)
revs2, err := GetLastCommitForPaths(ctx, cache, c, treePath, unHitPaths)
if err != nil {
return nil, nil, err
}

for k, v := range revs2 {
if err := cache.Put(commit.ID.String(), path.Join(treePath, k), v.ID().String()); err != nil {
return nil, nil, err
}
revs[k] = v
}
}
} else {
revs, err = GetLastCommitForPaths(ctx, c, treePath, entryPaths)
revs, err = GetLastCommitForPaths(ctx, nil, c, treePath, entryPaths)
}
if err != nil {
return nil, nil, err
Expand All @@ -70,25 +67,29 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
commitsInfo[i] = CommitInfo{
Entry: entry,
}

// Check if we have found a commit for this entry in time
if rev, ok := revs[entry.Name()]; ok {
entryCommit := convertCommit(rev)
commitsInfo[i].Commit = entryCommit
if entry.IsSubModule() {
subModuleURL := ""
var fullPath string
if len(treePath) > 0 {
fullPath = treePath + "/" + entry.Name()
} else {
fullPath = entry.Name()
}
if subModule, err := commit.GetSubModule(fullPath); err != nil {
return nil, nil, err
} else if subModule != nil {
subModuleURL = subModule.URL
}
subModuleFile := NewSubModuleFile(entryCommit, subModuleURL, entry.ID.String())
commitsInfo[i].SubModuleFile = subModuleFile
}

// If the entry if a submodule add a submodule file for this
if entry.IsSubModule() {
subModuleURL := ""
var fullPath string
if len(treePath) > 0 {
fullPath = treePath + "/" + entry.Name()
} else {
fullPath = entry.Name()
}
if subModule, err := commit.GetSubModule(fullPath); err != nil {
return nil, nil, err
} else if subModule != nil {
subModuleURL = subModule.URL
}
subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String())
commitsInfo[i].SubModuleFile = subModuleFile
}
}

Expand Down Expand Up @@ -175,7 +176,9 @@ func getLastCommitForPathsByCache(commitID, treePath string, paths []string, cac
}

// GetLastCommitForPaths returns last commit information
func GetLastCommitForPaths(ctx context.Context, 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]*object.Commit, error) {
refSha := c.ID().String()

// We do a tree traversal with nodes sorted by commit time
heap := binaryheap.NewWith(func(a, b interface{}) int {
if a.(*commitAndPaths).commit.CommitTime().Before(b.(*commitAndPaths).commit.CommitTime()) {
Expand All @@ -192,10 +195,13 @@ func GetLastCommitForPaths(ctx context.Context, c cgobject.CommitNode, treePath

// Start search from the root commit and with full set of paths
heap.Push(&commitAndPaths{c, paths, initialHashes})

heaploop:
for {
select {
case <-ctx.Done():
if ctx.Err() == context.DeadlineExceeded {
break heaploop
}
return nil, ctx.Err()
default:
}
Expand Down Expand Up @@ -233,14 +239,14 @@ func GetLastCommitForPaths(ctx context.Context, c cgobject.CommitNode, treePath
}

var remainingPaths []string
for i, path := range current.paths {
for i, pth := range current.paths {
// The results could already contain some newer change for the same path,
// so don't override that and bail out on the file early.
if resultNodes[path] == nil {
if resultNodes[pth] == nil {
if pathUnchanged[i] {
// The path existed with the same hash in at least one parent so it could
// not have been changed in this commit directly.
remainingPaths = append(remainingPaths, path)
remainingPaths = append(remainingPaths, pth)
} else {
// There are few possible cases how can we get here:
// - The path didn't exist in any parent, so it must have been created by
Expand All @@ -250,7 +256,10 @@ func GetLastCommitForPaths(ctx context.Context, c cgobject.CommitNode, treePath
// - We are looking at a merge commit and the hash of the file doesn't
// match any of the hashes being merged. This is more common for directories,
// but it can also happen if a file is changed through conflict resolution.
resultNodes[path] = current.commit
resultNodes[pth] = current.commit
if err := cache.Put(refSha, path.Join(treePath, pth), current.commit.ID().String()); err != nil {
return nil, err
}
}
}
}
Expand Down
47 changes: 24 additions & 23 deletions modules/git/commit_info_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,18 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
}
if len(unHitPaths) > 0 {
sort.Strings(unHitPaths)
commits, err := GetLastCommitForPaths(ctx, commit, treePath, unHitPaths)
commits, err := GetLastCommitForPaths(ctx, cache, commit, treePath, unHitPaths)
if err != nil {
return nil, nil, err
}

for pth, found := range commits {
if err := cache.Put(commit.ID.String(), path.Join(treePath, pth), found.ID.String()); err != nil {
return nil, nil, err
}
revs[pth] = found
}
}
} else {
sort.Strings(entryPaths)
revs, err = GetLastCommitForPaths(ctx, commit, treePath, entryPaths)
revs, err = GetLastCommitForPaths(ctx, nil, commit, treePath, entryPaths)
}
if err != nil {
return nil, nil, err
Expand All @@ -62,27 +59,31 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath
commitsInfo[i] = CommitInfo{
Entry: entry,
}

// Check if we have found a commit for this entry in time
if entryCommit, ok := revs[entry.Name()]; ok {
commitsInfo[i].Commit = entryCommit
if entry.IsSubModule() {
subModuleURL := ""
var fullPath string
if len(treePath) > 0 {
fullPath = treePath + "/" + entry.Name()
} else {
fullPath = entry.Name()
}
if subModule, err := commit.GetSubModule(fullPath); err != nil {
return nil, nil, err
} else if subModule != nil {
subModuleURL = subModule.URL
}
subModuleFile := NewSubModuleFile(entryCommit, subModuleURL, entry.ID.String())
commitsInfo[i].SubModuleFile = subModuleFile
}
} else {
log.Debug("missing commit for %s", entry.Name())
}

// If the entry if a submodule add a submodule file for this
if entry.IsSubModule() {
subModuleURL := ""
var fullPath string
if len(treePath) > 0 {
fullPath = treePath + "/" + entry.Name()
} else {
fullPath = entry.Name()
}
if subModule, err := commit.GetSubModule(fullPath); err != nil {
return nil, nil, err
} else if subModule != nil {
subModuleURL = subModule.URL
}
subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String())
commitsInfo[i].SubModuleFile = subModuleFile
}
}

// Retrieve the commit for the treePath itself (see above). We basically
Expand Down Expand Up @@ -121,9 +122,9 @@ func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string
}

// GetLastCommitForPaths returns last commit information
func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string, paths []string) (map[string]*Commit, error) {
func GetLastCommitForPaths(ctx context.Context, cache *LastCommitCache, 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, commit.repo, commit, treePath, paths...)
revs, err := WalkGitLog(ctx, cache, commit.repo, commit, treePath, paths...)
if err != nil {
return nil, err
}
Expand Down
3 changes: 3 additions & 0 deletions modules/git/last_commit_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ func (c *LastCommitCache) getCacheKey(repoPath, ref, entryPath string) string {

// 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())
}
8 changes: 2 additions & 6 deletions modules/git/last_commit_cache_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ package git

import (
"context"
"path"

"code.gitea.io/gitea/modules/log"

Expand Down Expand Up @@ -93,15 +92,12 @@ func (c *LastCommitCache) recursiveCache(ctx context.Context, index cgobject.Com
entryMap[entry.Name()] = entry
}

commits, err := GetLastCommitForPaths(ctx, index, treePath, entryPaths)
commits, err := GetLastCommitForPaths(ctx, c, index, treePath, entryPaths)
if err != nil {
return err
}

for entry, cm := range commits {
if err := c.Put(index.ID().String(), path.Join(treePath, entry), cm.ID().String()); err != nil {
return err
}
for entry := range commits {
if entryMap[entry].IsDir() {
subTree, err := tree.SubTree(entry)
if err != nil {
Expand Down
16 changes: 5 additions & 11 deletions modules/git/last_commit_cache_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ package git
import (
"bufio"
"context"
"path"

"code.gitea.io/gitea/modules/log"
)
Expand Down Expand Up @@ -80,28 +79,23 @@ func (c *LastCommitCache) recursiveCache(ctx context.Context, commit *Commit, tr
}

entryPaths := make([]string, len(entries))
entryMap := make(map[string]*TreeEntry)
for i, entry := range entries {
entryPaths[i] = entry.Name()
entryMap[entry.Name()] = entry
}

commits, err := GetLastCommitForPaths(ctx, commit, treePath, entryPaths)
_, err = WalkGitLog(ctx, c, commit.repo, commit, treePath, entryPaths...)
if err != nil {
return err
}

for entry, entryCommit := range commits {
if err := c.Put(commit.ID.String(), path.Join(treePath, entry), entryCommit.ID.String()); err != nil {
return err
}
for _, treeEntry := range entries {
// entryMap won't contain "" therefore skip this.
if treeEntry := entryMap[entry]; treeEntry != nil && treeEntry.IsDir() {
subTree, err := tree.SubTree(entry)
if treeEntry.IsDir() {
subTree, err := tree.SubTree(treeEntry.Name())
if err != nil {
return err
}
if err := c.recursiveCache(ctx, commit, subTree, entry, level-1); err != nil {
if err := c.recursiveCache(ctx, commit, subTree, treeEntry.Name(), level-1); err != nil {
return err
}
}
Expand Down
13 changes: 12 additions & 1 deletion modules/git/log_name_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,9 @@ func (g *LogNameStatusRepoParser) Close() {
}

// WalkGitLog walks the git log --name-status for the head commit in the provided treepath and files
func WalkGitLog(ctx context.Context, repo *Repository, head *Commit, treepath string, paths ...string) (map[string]string, error) {
func WalkGitLog(ctx context.Context, cache *LastCommitCache, repo *Repository, head *Commit, treepath string, paths ...string) (map[string]string, error) {
headRef := head.ID.String()

tree, err := head.SubTree(treepath)
if err != nil {
return nil, err
Expand Down Expand Up @@ -339,6 +341,9 @@ heaploop:
for {
select {
case <-ctx.Done():
if ctx.Err() == context.DeadlineExceeded {
break heaploop
}
g.Close()
return nil, ctx.Err()
default:
Expand All @@ -360,10 +365,16 @@ heaploop:
changed[i] = false
if results[i] == "" {
results[i] = current.CommitID
if err := cache.Put(headRef, path.Join(treepath, paths[i]), current.CommitID); err != nil {
return nil, err
}
delete(path2idx, paths[i])
remaining--
if results[0] == "" {
results[0] = current.CommitID
if err := cache.Put(headRef, treepath, current.CommitID); err != nil {
return nil, err
}
delete(path2idx, "")
remaining--
}
Expand Down
3 changes: 2 additions & 1 deletion modules/git/notes_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
)

// GetNote retrieves the git-notes data for a given commit.
// FIXME: Add LastCommitCache support
func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note) error {
log.Trace("Searching for git note corresponding to the commit %q in the repository %q", commitID, repo.Path)
notes, err := repo.GetCommit(NotesRef)
Expand Down Expand Up @@ -75,7 +76,7 @@ func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note)
return err
}

lastCommits, err := GetLastCommitForPaths(ctx, commitNode, "", []string{path})
lastCommits, err := GetLastCommitForPaths(ctx, nil, commitNode, "", []string{path})
if err != nil {
log.Error("Unable to get the commit for the path %q. Error: %v", path, err)
return err
Expand Down
3 changes: 2 additions & 1 deletion modules/git/notes_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
)

// GetNote retrieves the git-notes data for a given commit.
// FIXME: Add LastCommitCache support
func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note) error {
log.Trace("Searching for git note corresponding to the commit %q in the repository %q", commitID, repo.Path)
notes, err := repo.GetCommit(NotesRef)
Expand Down Expand Up @@ -75,7 +76,7 @@ func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note)
path = path[idx+1:]
}

lastCommits, err := GetLastCommitForPaths(ctx, notes, treePath, []string{path})
lastCommits, err := GetLastCommitForPaths(ctx, nil, notes, treePath, []string{path})
if err != nil {
log.Error("Unable to get the commit for the path %q. Error: %v", treePath, err)
return err
Expand Down
Loading

0 comments on commit 001dbf1

Please sign in to comment.