Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement for list pull requests #15447

Merged
merged 5 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions models/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 7 additions & 12 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions routers/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

// -------------------------------
Expand Down
60 changes: 50 additions & 10 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"bufio"
"bytes"
"context"
"errors"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -643,33 +642,74 @@ func GetSquashMergeCommitMessages(pr *models.PullRequest) string {
return stringBuilder.String()
}

// 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) {
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
Expand Down
4 changes: 2 additions & 2 deletions templates/shared/issuelist.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down