mirror of
https://github.com/go-gitea/gitea.git
synced 2026-06-23 04:20:41 +00:00
perf: Various performance regression fixes (#38078)
Fixes five N+1 / O(n) query patterns found across common user paths.
Each uses a bulk query that already existed elsewhere in the codebase.
| Location | Problem | Introduced in |
| -------------------------------- |
-------------------------------------------------------------------------------------------------------------------------------
| ------------- |
| `IssueList.LoadIsRead` | `.In("issue_id")` missing its arg — xorm
generates `WHERE 0=1`, so `IsRead` is **never** set; every issue always
appears unread | #29515 |
| `ParseCommitsWithStatus` | `GetLatestCommitStatus` called once per
commit (O(n) queries on commit list / PR commits tab) | #33605 |
| `getReleaseInfos` (release list) | `GetLatestCommitStatus` called once
per release for CI badges | #29149 |
| User milestone dashboard | O(n×m) nested loop matching milestones to
repos | #26300 |
| `findCodeComments` (PR diff) | `LoadResolveDoer` + `LoadReactions`
called per inline comment — up to ~150 queries on a PR with 50 comments
| #20821 |
---------
Co-authored-by: Lauris B <lauris@nix.lv>
This commit is contained in:
@@ -79,6 +79,14 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := comments.loadResolveDoers(ctx); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := comments.loadReactions(ctx, issue.Repo); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Find all reviews by ReviewID
|
||||
reviews := make(map[int64]*Review)
|
||||
ids := make([]int64, 0, len(comments))
|
||||
@@ -107,14 +115,6 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
|
||||
comments[n] = comment
|
||||
n++
|
||||
|
||||
if err := comment.LoadResolveDoer(ctx); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := comment.LoadReactions(ctx, issue.Repo); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var err error
|
||||
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{
|
||||
FootnoteContextID: strconv.FormatInt(comment.ID, 10),
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
user_model "gitea.dev/models/user"
|
||||
"gitea.dev/modules/container"
|
||||
"gitea.dev/modules/log"
|
||||
"gitea.dev/modules/setting"
|
||||
)
|
||||
|
||||
// CommentList defines a list of comments
|
||||
@@ -444,6 +445,73 @@ func (comments CommentList) loadReviews(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// loadResolveDoers bulk-loads the resolve doer for all code comments that have one.
|
||||
func (comments CommentList) loadResolveDoers(ctx context.Context) error {
|
||||
resolveDoerIDs := container.FilterSlice(comments, func(c *Comment) (int64, bool) {
|
||||
return c.ResolveDoerID, c.ResolveDoerID != 0 && c.Type == CommentTypeCode
|
||||
})
|
||||
if len(resolveDoerIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
userMaps, err := user_model.GetUsersMapByIDs(ctx, resolveDoerIDs)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
for _, comment := range comments {
|
||||
if comment.ResolveDoerID == 0 || comment.Type != CommentTypeCode {
|
||||
continue
|
||||
}
|
||||
if u, ok := userMaps[comment.ResolveDoerID]; ok {
|
||||
comment.ResolveDoer = u
|
||||
} else {
|
||||
comment.ResolveDoer = user_model.NewGhostUser()
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// loadReactions bulk-loads reactions for all comments in the list.
|
||||
func (comments CommentList) loadReactions(ctx context.Context, repo *repo_model.Repository) error {
|
||||
if len(comments) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
commentIDs := container.FilterSlice(comments, func(c *Comment) (int64, bool) {
|
||||
return c.ID, c.Reactions == nil
|
||||
})
|
||||
if len(commentIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
var allReactions ReactionList
|
||||
if err := db.GetEngine(ctx).
|
||||
Where("`comment_id` > 0").
|
||||
In("comment_id", commentIDs).
|
||||
In("`type`", setting.UI.Reactions).
|
||||
Asc("issue_id", "comment_id", "created_unix", "id").
|
||||
Find(&allReactions); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if _, err := allReactions.LoadUsers(ctx, repo); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
reactByComment := make(map[int64]ReactionList, len(commentIDs))
|
||||
for _, r := range allReactions {
|
||||
reactByComment[r.CommentID] = append(reactByComment[r.CommentID], r)
|
||||
}
|
||||
|
||||
for _, comment := range comments {
|
||||
if comment.Reactions == nil {
|
||||
comment.Reactions = reactByComment[comment.ID]
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// LoadAttributes loads attributes of the comments, except for attachments and
|
||||
// comments
|
||||
func (comments CommentList) LoadAttributes(ctx context.Context) (err error) {
|
||||
|
||||
@@ -591,9 +591,12 @@ func (issues IssueList) GetApprovalCounts(ctx context.Context) (map[int64][]*Rev
|
||||
|
||||
func (issues IssueList) LoadIsRead(ctx context.Context, userID int64) error {
|
||||
issueIDs := issues.getIssueIDs()
|
||||
if len(issueIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
issueUsers := make([]*IssueUser, 0, len(issueIDs))
|
||||
if err := db.GetEngine(ctx).Where("uid =?", userID).
|
||||
In("issue_id").
|
||||
In("issue_id", issueIDs).
|
||||
Find(&issueUsers); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"gitea.dev/modules/setting"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestIssueList_LoadRepositories(t *testing.T) {
|
||||
@@ -30,6 +31,22 @@ func TestIssueList_LoadRepositories(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIssueList_LoadIsRead(t *testing.T) {
|
||||
// Regression: In("issue_id") was missing the issueIDs argument, causing
|
||||
// xorm to generate "0=1" and never mark any issue as read.
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
|
||||
issue2 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
|
||||
|
||||
// Fixture: uid=1 has is_read=true on issue 1 only.
|
||||
issueList := issues_model.IssueList{issue1, issue2}
|
||||
require.NoError(t, issueList.LoadIsRead(t.Context(), 1))
|
||||
|
||||
assert.True(t, issue1.IsRead, "issue 1 should be marked read for user 1")
|
||||
assert.False(t, issue2.IsRead, "issue 2 should not be marked read for user 1")
|
||||
}
|
||||
|
||||
func TestIssueList_LoadAttributes(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
setting.Service.EnableTimetracking = true
|
||||
|
||||
@@ -101,6 +101,19 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions)
|
||||
|
||||
canReadActions := ctx.Repo.Permission.CanRead(unit.TypeActions)
|
||||
|
||||
// Bulk-load commit statuses for all releases in one query.
|
||||
var commitStatusMap map[string][]*git_model.CommitStatus
|
||||
if canReadActions && len(releases) > 0 {
|
||||
shas := make([]string, 0, len(releases))
|
||||
for _, r := range releases {
|
||||
shas = append(shas, r.Sha1)
|
||||
}
|
||||
commitStatusMap, err = git_model.GetLatestCommitStatusForRepoCommitIDs(ctx, ctx.Repo.Repository.ID, shas)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
releaseInfos := make([]*ReleaseInfo, 0, len(releases))
|
||||
for _, r := range releases {
|
||||
if r.Publisher, ok = cacheUsers[r.PublisherID]; !ok {
|
||||
@@ -130,11 +143,7 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions)
|
||||
}
|
||||
|
||||
if canReadActions {
|
||||
statuses, err := git_model.GetLatestCommitStatus(ctx, r.Repo.ID, r.Sha1, db.ListOptionsAll)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
statuses := commitStatusMap[r.Sha1]
|
||||
info.CommitStatus = git_model.CalcCommitStatus(statuses)
|
||||
info.CommitStatuses = statuses
|
||||
}
|
||||
|
||||
@@ -242,13 +242,13 @@ func Milestones(ctx *context.Context) {
|
||||
}
|
||||
sort.Sort(showRepos)
|
||||
|
||||
repoByID := make(map[int64]*repo_model.Repository, len(showRepos))
|
||||
for _, repo := range showRepos {
|
||||
repoByID[repo.ID] = repo
|
||||
}
|
||||
|
||||
for i := 0; i < len(milestones); {
|
||||
for _, repo := range showRepos {
|
||||
if milestones[i].RepoID == repo.ID {
|
||||
milestones[i].Repo = repo
|
||||
break
|
||||
}
|
||||
}
|
||||
milestones[i].Repo = repoByID[milestones[i].RepoID]
|
||||
if milestones[i].Repo == nil {
|
||||
log.Warn("Cannot find milestone %d 's repository %d", milestones[i].ID, milestones[i].RepoID)
|
||||
milestones = append(milestones[:i], milestones[i+1:]...)
|
||||
|
||||
+18
-12
@@ -7,7 +7,6 @@ import (
|
||||
"context"
|
||||
|
||||
asymkey_model "gitea.dev/models/asymkey"
|
||||
"gitea.dev/models/db"
|
||||
git_model "gitea.dev/models/git"
|
||||
"gitea.dev/models/gituser"
|
||||
repo_model "gitea.dev/models/repo"
|
||||
@@ -72,20 +71,27 @@ func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo
|
||||
|
||||
// ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state
|
||||
func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) ([]*git_model.SignCommitWithStatuses, error) {
|
||||
newCommits := make([]*git_model.SignCommitWithStatuses, 0, len(oldCommits))
|
||||
if len(oldCommits) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
commitIDs := make([]string, 0, len(oldCommits))
|
||||
for _, c := range oldCommits {
|
||||
commit := &git_model.SignCommitWithStatuses{
|
||||
SignCommit: c,
|
||||
}
|
||||
statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.GitCommit.ID.String(), db.ListOptionsAll)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
commitIDs = append(commitIDs, c.GitCommit.ID.String())
|
||||
}
|
||||
statusMap, err := git_model.GetLatestCommitStatusForRepoCommitIDs(ctx, repo.ID, commitIDs)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
commit.Statuses = statuses
|
||||
commit.Status = git_model.CalcCommitStatus(statuses)
|
||||
newCommits = append(newCommits, commit)
|
||||
newCommits := make([]*git_model.SignCommitWithStatuses, 0, len(oldCommits))
|
||||
for _, c := range oldCommits {
|
||||
statuses := statusMap[c.GitCommit.ID.String()]
|
||||
newCommits = append(newCommits, &git_model.SignCommitWithStatuses{
|
||||
SignCommit: c,
|
||||
Statuses: statuses,
|
||||
Status: git_model.CalcCommitStatus(statuses),
|
||||
})
|
||||
}
|
||||
return newCommits, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user