Performance improvement for list pull requests (#15447) (#15500)

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

models/issue_list.go

@@ -53,6 +53,9 @@ func (issues IssueList) loadRepositories(e Engine) ([]*Repository, error) {
 
 	for _, issue := range issues {
 		issue.Repo = repoMaps[issue.RepoID]
+		if issue.PullRequest != nil {
+			issue.PullRequest.BaseRepo = issue.Repo
+		}
 	}
 	return valuesRepository(repoMaps), nil
 }
@@ -516,6 +519,11 @@ func (issues IssueList) LoadDiscussComments() error {
 	return issues.loadComments(x, builder.Eq{"comment.type": CommentTypeComment})
 }
 
+// LoadPullRequests loads pull requests
+func (issues IssueList) LoadPullRequests() error {
+	return issues.loadPullRequests(x)
+}
+
 // GetApprovalCounts returns a map of issue ID to slice of approval counts
 // FIXME: only returns official counts due to double counting of non-official approvals
 func (issues IssueList) GetApprovalCounts() (map[int64][]*ReviewCount, error) {

routers/repo/issue.go

@@ -241,14 +241,13 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
 		}
 	}
 
-	approvalCounts, err := models.IssueList(issues).GetApprovalCounts()
+	var issueList = models.IssueList(issues)
+	approvalCounts, err := issueList.GetApprovalCounts()
 	if err != nil {
 		ctx.ServerError("ApprovalCounts", err)
 		return
 	}
 
-	var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
-
 	// Get posters.
 	for i := range issues {
 		// Check read status
@@ -258,18 +257,14 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
 			ctx.ServerError("GetIsRead", err)
 			return
 		}
+	}
 
-		if issues[i].IsPull {
-			if err := issues[i].LoadPullRequest(); err != nil {
-				ctx.ServerError("LoadPullRequest", err)
+	commitStatus, err := pull_service.GetIssuesLastCommitStatus(issues)
+	if err != nil {
+		ctx.ServerError("GetIssuesLastCommitStatus", err)
 		return
 	}
 
-			var statuses, _ = pull_service.GetLastCommitStatus(issues[i].PullRequest)
-			commitStatus[issues[i].PullRequest.ID] = models.CalcCommitStatus(statuses)
-		}
-	}
-
 	ctx.Data["Issues"] = issues
 	ctx.Data["CommitStatus"] = commitStatus
 

routers/user/home.go

@@ -546,14 +546,14 @@ func buildIssueOverview(ctx *context.Context, unitType models.UnitType) {
 	}
 
 	// maps pull request IDs to their CommitStatus. Will be posted to ctx.Data.
-	var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
 	for _, issue := range issues {
 		issue.Repo = showReposMap[issue.RepoID]
-
-		if isPullList {
-			var statuses, _ = pull_service.GetLastCommitStatus(issue.PullRequest)
-			commitStatus[issue.PullRequest.ID] = models.CalcCommitStatus(statuses)
 	}
+
+	commitStatus, err := pull_service.GetIssuesLastCommitStatus(issues)
+	if err != nil {
+		ctx.ServerError("GetIssuesLastCommitStatus", err)
+		return
 	}
 
 	// -------------------------------

services/pull/pull.go

@@ -8,7 +8,6 @@ import (
 	"bufio"
 	"bytes"
 	"context"
-	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -643,33 +642,74 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string {
 	return stringBuilder.String()
 }
 
-// GetLastCommitStatus returns list of commit statuses for latest commit on this pull request.
-func GetLastCommitStatus(pr *models.PullRequest) (status []*models.CommitStatus, err error) {
-	if err = pr.LoadBaseRepo(); err != nil {
+// GetIssuesLastCommitStatus returns a map
+func GetIssuesLastCommitStatus(issues models.IssueList) (map[int64]*models.CommitStatus, error) {
+	if err := issues.LoadPullRequests(); err != nil {
+		return nil, err
+	}
+	if _, err := issues.LoadRepositories(); err != nil {
 		return nil, err
 	}
 
+	var (
+		gitRepos = make(map[int64]*git.Repository)
+		res      = make(map[int64]*models.CommitStatus)
+		err      error
+	)
+	defer func() {
+		for _, gitRepo := range gitRepos {
+			gitRepo.Close()
+		}
+	}()
+
+	for _, issue := range issues {
+		if !issue.IsPull {
+			continue
+		}
+		gitRepo, ok := gitRepos[issue.RepoID]
+		if !ok {
+			gitRepo, err = git.OpenRepository(issue.Repo.RepoPath())
+			if err != nil {
+				return nil, err
+			}
+			gitRepos[issue.RepoID] = gitRepo
+		}
+
+		status, err := getLastCommitStatus(gitRepo, issue.PullRequest)
+		if err != nil {
+			return nil, err
+		}
+		res[issue.PullRequest.ID] = status
+	}
+	return res, nil
+}
+
+// GetLastCommitStatus returns list of commit statuses for latest commit on this pull request.
+func GetLastCommitStatus(pr *models.PullRequest) (status *models.CommitStatus, err error) {
+	if err = pr.LoadBaseRepo(); err != nil {
+		return nil, err
+	}
 	gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
 	if err != nil {
 		return nil, err
 	}
 	defer gitRepo.Close()
 
-	compareInfo, err := gitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.MergeBase, pr.GetGitRefName())
+	return getLastCommitStatus(gitRepo, pr)
+}
+
+// getLastCommitStatus get pr's last commit status. PR's last commit status is the head commit id's last commit status
+func getLastCommitStatus(gitRepo *git.Repository, pr *models.PullRequest) (status *models.CommitStatus, err error) {
+	sha, err := gitRepo.GetRefCommitID(pr.GetGitRefName())
 	if err != nil {
 		return nil, err
 	}
 
-	if compareInfo.Commits.Len() == 0 {
-		return nil, errors.New("pull request has no commits")
-	}
-
-	sha := compareInfo.Commits.Front().Value.(*git.Commit).ID.String()
 	statusList, err := models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, models.ListOptions{})
 	if err != nil {
 		return nil, err
 	}
-	return statusList, nil
+	return models.CalcCommitStatus(statusList), nil
 }
 
 // IsHeadEqualWithBranch returns if the commits of branchName are available in pull request head

templates/shared/issuelist.tmpl

@@ -62,12 +62,12 @@
 						{{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}}
 					{{end}}
 					{{if and .Milestone (ne $.listType "milestone")}}
-						<a class="milestone" {{if $.RepoLink}}href="{{$.RepoLink}}/milestone/{{.Milestone.ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}/milestone/{{.Milestone.ID}}"{{end}}>
+						<a class="milestone" {{if $.RepoLink}}href="{{$.RepoLink}}/milestone/{{.Milestone.ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.OwnerName}}/{{.Repo.Name}}/milestone/{{.Milestone.ID}}"{{end}}>
 							{{svg "octicon-milestone" 14 "mr-2"}}{{.Milestone.Name}}
 						</a>
 					{{end}}
 					{{if .Ref}}
-						<a class="ref" {{if $.RepoLink}}href="{{$.RepoLink}}{{index $.IssueRefURLs .ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}{{index $.IssueRefURLs .ID}}"{{end}}>
+						<a class="ref" {{if $.RepoLink}}href="{{$.RepoLink}}{{index $.IssueRefURLs .ID}}"{{else}}href="{{AppSubUrl}}/{{.Repo.OwnerName}}/{{.Repo.Name}}{{index $.IssueRefURLs .ID}}"{{end}}>
 							{{svg "octicon-git-branch" 14 "mr-2"}}{{index $.IssueRefEndNames .ID}}
 						</a>
 					{{end}}