From 8202dd131189102d2531c853665d213e43a5c818 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Fri, 16 Apr 2021 01:34:43 +0800
Subject: [PATCH] Performance improvement for list pull requests (#15447)
---
models/issue_list.go | 8 +++++
routers/repo/issue.go | 19 ++++------
routers/user/home.go | 10 +++---
services/pull/pull.go | 62 +++++++++++++++++++++++++++------
templates/shared/issuelist.tmpl | 4 +--
5 files changed, 73 insertions(+), 30 deletions(-)
diff --git a/models/issue_list.go b/models/issue_list.go
index 0ac25fc690..87ad318dcf 100644
--- a/models/issue_list.go
+++ b/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) {
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index da3772ef5a..7471bb65a4 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -239,14 +239,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
@@ -256,16 +255,12 @@ 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)
- return
- }
-
- var statuses, _ = pull_service.GetLastCommitStatus(issues[i].PullRequest)
- commitStatus[issues[i].PullRequest.ID] = models.CalcCommitStatus(statuses)
- }
+ commitStatus, err := pull_service.GetIssuesLastCommitStatus(issues)
+ if err != nil {
+ ctx.ServerError("GetIssuesLastCommitStatus", err)
+ return
}
ctx.Data["Issues"] = issues
diff --git a/routers/user/home.go b/routers/user/home.go
index 431ffdde8e..584bc019fa 100644
--- a/routers/user/home.go
+++ b/routers/user/home.go
@@ -550,14 +550,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
}
// -------------------------------
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 331ef46d3d..153a75094d 100644
--- a/services/pull/pull.go
+++ b/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
diff --git a/templates/shared/issuelist.tmpl b/templates/shared/issuelist.tmpl
index d6cd60cbd2..d07295aa67 100644
--- a/templates/shared/issuelist.tmpl
+++ b/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}}