From 92c91d7d8bbcd1e8a80bdefa9c1b95d3a0e17652 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 16 Apr 2021 00:14:14 +0200 Subject: [PATCH] Performance improvement for list pull requests (#15447) (#15500) Co-authored-by: Lunny Xiao --- 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 0ac25fc690e0..87ad318dcf2d 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 15459cd0e6d3..37a5266708b8 100644 --- a/routers/repo/issue.go +++ b/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,16 +257,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 24aaf00ba829..2c5f153d728b 100644 --- a/routers/user/home.go +++ b/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 } // ------------------------------- diff --git a/services/pull/pull.go b/services/pull/pull.go index 331ef46d3d34..153a75094d73 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 1b4f21b4004f..6293bbdd7709 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")}} - + {{svg "octicon-milestone" 14 "mr-2"}}{{.Milestone.Name}} {{end}} {{if .Ref}} - + {{svg "octicon-git-branch" 14 "mr-2"}}{{index $.IssueRefEndNames .ID}} {{end}}