This PR do some performance optimzations. - [x] Add `index` for the column `comment_id` of `Attachment` table to accelerate query from the database. - [x] Remove unnecessary database queries when viewing issues. Before some conditions which id = 0 will be sent to the database - [x] Remove duplicated load posters - [x] Batch loading attachements, isread of comments on viewing issue --------- Co-authored-by: Zettat123 <zettat123@gmail.com>tags/v1.22.0-rc0
@@ -673,7 +673,8 @@ func (c *Comment) LoadTime(ctx context.Context) error { | |||
return err | |||
} | |||
func (c *Comment) loadReactions(ctx context.Context, repo *repo_model.Repository) (err error) { | |||
// LoadReactions loads comment reactions | |||
func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository) (err error) { | |||
if c.Reactions != nil { | |||
return nil | |||
} | |||
@@ -691,11 +692,6 @@ func (c *Comment) loadReactions(ctx context.Context, repo *repo_model.Repository | |||
return nil | |||
} | |||
// LoadReactions loads comment reactions | |||
func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository) error { | |||
return c.loadReactions(ctx, repo) | |||
} | |||
func (c *Comment) loadReview(ctx context.Context) (err error) { | |||
if c.ReviewID == 0 { | |||
return nil |
@@ -122,7 +122,7 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu | |||
} | |||
// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number | |||
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) ([]*Comment, error) { | |||
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64, showOutdatedComments bool) (CommentList, error) { | |||
opts := FindCommentsOptions{ | |||
Type: CommentTypeCode, | |||
IssueID: issue.ID, |
@@ -19,7 +19,9 @@ type CommentList []*Comment | |||
func (comments CommentList) getPosterIDs() []int64 { | |||
posterIDs := make(container.Set[int64], len(comments)) | |||
for _, comment := range comments { | |||
posterIDs.Add(comment.PosterID) | |||
if comment.PosterID > 0 { | |||
posterIDs.Add(comment.PosterID) | |||
} | |||
} | |||
return posterIDs.Values() | |||
} | |||
@@ -41,18 +43,12 @@ func (comments CommentList) LoadPosters(ctx context.Context) error { | |||
return nil | |||
} | |||
func (comments CommentList) getCommentIDs() []int64 { | |||
ids := make([]int64, 0, len(comments)) | |||
for _, comment := range comments { | |||
ids = append(ids, comment.ID) | |||
} | |||
return ids | |||
} | |||
func (comments CommentList) getLabelIDs() []int64 { | |||
ids := make(container.Set[int64], len(comments)) | |||
for _, comment := range comments { | |||
ids.Add(comment.LabelID) | |||
if comment.LabelID > 0 { | |||
ids.Add(comment.LabelID) | |||
} | |||
} | |||
return ids.Values() | |||
} | |||
@@ -100,7 +96,9 @@ func (comments CommentList) loadLabels(ctx context.Context) error { | |||
func (comments CommentList) getMilestoneIDs() []int64 { | |||
ids := make(container.Set[int64], len(comments)) | |||
for _, comment := range comments { | |||
ids.Add(comment.MilestoneID) | |||
if comment.MilestoneID > 0 { | |||
ids.Add(comment.MilestoneID) | |||
} | |||
} | |||
return ids.Values() | |||
} | |||
@@ -141,7 +139,9 @@ func (comments CommentList) loadMilestones(ctx context.Context) error { | |||
func (comments CommentList) getOldMilestoneIDs() []int64 { | |||
ids := make(container.Set[int64], len(comments)) | |||
for _, comment := range comments { | |||
ids.Add(comment.OldMilestoneID) | |||
if comment.OldMilestoneID > 0 { | |||
ids.Add(comment.OldMilestoneID) | |||
} | |||
} | |||
return ids.Values() | |||
} | |||
@@ -182,7 +182,9 @@ func (comments CommentList) loadOldMilestones(ctx context.Context) error { | |||
func (comments CommentList) getAssigneeIDs() []int64 { | |||
ids := make(container.Set[int64], len(comments)) | |||
for _, comment := range comments { | |||
ids.Add(comment.AssigneeID) | |||
if comment.AssigneeID > 0 { | |||
ids.Add(comment.AssigneeID) | |||
} | |||
} | |||
return ids.Values() | |||
} | |||
@@ -314,7 +316,9 @@ func (comments CommentList) getDependentIssueIDs() []int64 { | |||
if comment.DependentIssue != nil { | |||
continue | |||
} | |||
ids.Add(comment.DependentIssueID) | |||
if comment.DependentIssueID > 0 { | |||
ids.Add(comment.DependentIssueID) | |||
} | |||
} | |||
return ids.Values() | |||
} | |||
@@ -369,6 +373,41 @@ func (comments CommentList) loadDependentIssues(ctx context.Context) error { | |||
return nil | |||
} | |||
// getAttachmentCommentIDs only return the comment ids which possibly has attachments | |||
func (comments CommentList) getAttachmentCommentIDs() []int64 { | |||
ids := make(container.Set[int64], len(comments)) | |||
for _, comment := range comments { | |||
if comment.Type == CommentTypeComment || | |||
comment.Type == CommentTypeReview || | |||
comment.Type == CommentTypeCode { | |||
ids.Add(comment.ID) | |||
} | |||
} | |||
return ids.Values() | |||
} | |||
// LoadAttachmentsByIssue loads attachments by issue id | |||
func (comments CommentList) LoadAttachmentsByIssue(ctx context.Context) error { | |||
if len(comments) == 0 { | |||
return nil | |||
} | |||
attachments := make([]*repo_model.Attachment, 0, len(comments)/2) | |||
if err := db.GetEngine(ctx).Where("issue_id=? AND comment_id>0", comments[0].IssueID).Find(&attachments); err != nil { | |||
return err | |||
} | |||
commentAttachmentsMap := make(map[int64][]*repo_model.Attachment, len(comments)) | |||
for _, attach := range attachments { | |||
commentAttachmentsMap[attach.CommentID] = append(commentAttachmentsMap[attach.CommentID], attach) | |||
} | |||
for _, comment := range comments { | |||
comment.Attachments = commentAttachmentsMap[comment.ID] | |||
} | |||
return nil | |||
} | |||
// LoadAttachments loads attachments | |||
func (comments CommentList) LoadAttachments(ctx context.Context) (err error) { | |||
if len(comments) == 0 { | |||
@@ -376,16 +415,15 @@ func (comments CommentList) LoadAttachments(ctx context.Context) (err error) { | |||
} | |||
attachments := make(map[int64][]*repo_model.Attachment, len(comments)) | |||
commentsIDs := comments.getCommentIDs() | |||
commentsIDs := comments.getAttachmentCommentIDs() | |||
left := len(commentsIDs) | |||
for left > 0 { | |||
limit := db.DefaultMaxInSize | |||
if left < limit { | |||
limit = left | |||
} | |||
rows, err := db.GetEngine(ctx).Table("attachment"). | |||
Join("INNER", "comment", "comment.id = attachment.comment_id"). | |||
In("comment.id", commentsIDs[:limit]). | |||
rows, err := db.GetEngine(ctx). | |||
In("comment_id", commentsIDs[:limit]). | |||
Rows(new(repo_model.Attachment)) | |||
if err != nil { | |||
return err | |||
@@ -415,7 +453,9 @@ func (comments CommentList) LoadAttachments(ctx context.Context) (err error) { | |||
func (comments CommentList) getReviewIDs() []int64 { | |||
ids := make(container.Set[int64], len(comments)) | |||
for _, comment := range comments { | |||
ids.Add(comment.ReviewID) | |||
if comment.ReviewID > 0 { | |||
ids.Add(comment.ReviewID) | |||
} | |||
} | |||
return ids.Values() | |||
} |
@@ -388,9 +388,8 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) { | |||
if left < limit { | |||
limit = left | |||
} | |||
rows, err := db.GetEngine(ctx).Table("attachment"). | |||
Join("INNER", "issue", "issue.id = attachment.issue_id"). | |||
In("issue.id", issuesIDs[:limit]). | |||
rows, err := db.GetEngine(ctx). | |||
In("issue_id", issuesIDs[:limit]). | |||
Rows(new(repo_model.Attachment)) | |||
if err != nil { | |||
return err | |||
@@ -609,3 +608,23 @@ func (issues IssueList) GetApprovalCounts(ctx context.Context) (map[int64][]*Rev | |||
return approvalCountMap, nil | |||
} | |||
func (issues IssueList) LoadIsRead(ctx context.Context, userID int64) error { | |||
issueIDs := issues.getIssueIDs() | |||
issueUsers := make([]*IssueUser, 0, len(issueIDs)) | |||
if err := db.GetEngine(ctx).Where("uid =?", userID). | |||
In("issue_id"). | |||
Find(&issueUsers); err != nil { | |||
return err | |||
} | |||
for _, issueUser := range issueUsers { | |||
for _, issue := range issues { | |||
if issue.ID == issueUser.IssueID { | |||
issue.IsRead = issueUser.IsRead | |||
} | |||
} | |||
} | |||
return nil | |||
} |
@@ -566,6 +566,8 @@ var migrations = []Migration{ | |||
NewMigration("Add default_wiki_branch to repository table", v1_22.AddDefaultWikiBranch), | |||
// v290 -> v291 | |||
NewMigration("Add PayloadVersion to HookTask", v1_22.AddPayloadVersionToHookTaskTable), | |||
// v291 -> v292 | |||
NewMigration("Add Index to attachment.comment_id", v1_22.AddCommentIDIndexofAttachment), | |||
} | |||
// GetCurrentDBVersion returns the current db version |
@@ -0,0 +1,14 @@ | |||
// Copyright 2024 The Gitea Authors. All rights reserved. | |||
// SPDX-License-Identifier: MIT | |||
package v1_22 //nolint | |||
import "xorm.io/xorm" | |||
func AddCommentIDIndexofAttachment(x *xorm.Engine) error { | |||
type Attachment struct { | |||
CommentID int64 `xorm:"INDEX"` | |||
} | |||
return x.Sync(&Attachment{}) | |||
} |
@@ -24,7 +24,7 @@ type Attachment struct { | |||
IssueID int64 `xorm:"INDEX"` // maybe zero when creating | |||
ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating | |||
UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added | |||
CommentID int64 | |||
CommentID int64 `xorm:"INDEX"` | |||
Name string | |||
DownloadCount int64 `xorm:"DEFAULT 0"` | |||
Size int64 `xorm:"DEFAULT 0"` |
@@ -323,10 +323,6 @@ func ListRepoIssueComments(ctx *context.APIContext) { | |||
ctx.Error(http.StatusInternalServerError, "LoadIssues", err) | |||
return | |||
} | |||
if err := comments.LoadPosters(ctx); err != nil { | |||
ctx.Error(http.StatusInternalServerError, "LoadPosters", err) | |||
return | |||
} | |||
if err := comments.LoadAttachments(ctx); err != nil { | |||
ctx.Error(http.StatusInternalServerError, "LoadAttachments", err) | |||
return |
@@ -324,15 +324,15 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt | |||
return | |||
} | |||
// Get posters. | |||
for i := range issues { | |||
// Check read status | |||
if !ctx.IsSigned { | |||
issues[i].IsRead = true | |||
} else if err = issues[i].GetIsRead(ctx, ctx.Doer.ID); err != nil { | |||
ctx.ServerError("GetIsRead", err) | |||
if ctx.IsSigned { | |||
if err := issues.LoadIsRead(ctx, ctx.Doer.ID); err != nil { | |||
ctx.ServerError("LoadIsRead", err) | |||
return | |||
} | |||
} else { | |||
for i := range issues { | |||
issues[i].IsRead = true | |||
} | |||
} | |||
commitStatuses, lastStatus, err := pull_service.GetIssuesAllCommitStatus(ctx, issues) | |||
@@ -1604,20 +1604,20 @@ func ViewIssue(ctx *context.Context) { | |||
// Render comments and and fetch participants. | |||
participants[0] = issue.Poster | |||
if err := issue.Comments.LoadAttachmentsByIssue(ctx); err != nil { | |||
ctx.ServerError("LoadAttachmentsByIssue", err) | |||
return | |||
} | |||
if err := issue.Comments.LoadPosters(ctx); err != nil { | |||
ctx.ServerError("LoadPosters", err) | |||
return | |||
} | |||
for _, comment = range issue.Comments { | |||
comment.Issue = issue | |||
if err := comment.LoadPoster(ctx); err != nil { | |||
ctx.ServerError("LoadPoster", err) | |||
return | |||
} | |||
if comment.Type == issues_model.CommentTypeComment || comment.Type == issues_model.CommentTypeReview { | |||
if err := comment.LoadAttachments(ctx); err != nil { | |||
ctx.ServerError("LoadAttachments", err) | |||
return | |||
} | |||
comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ | |||
Links: markup.Links{ | |||
Base: ctx.Repo.RepoLink, | |||
@@ -1665,7 +1665,6 @@ func ViewIssue(ctx *context.Context) { | |||
comment.Milestone = ghostMilestone | |||
} | |||
} else if comment.Type == issues_model.CommentTypeProject { | |||
if err = comment.LoadProject(ctx); err != nil { | |||
ctx.ServerError("LoadProject", err) | |||
return | |||
@@ -1731,10 +1730,6 @@ func ViewIssue(ctx *context.Context) { | |||
for _, codeComments := range comment.Review.CodeComments { | |||
for _, lineComments := range codeComments { | |||
for _, c := range lineComments { | |||
if err := c.LoadAttachments(ctx); err != nil { | |||
ctx.ServerError("LoadAttachments", err) | |||
return | |||
} | |||
// Check tag. | |||
role, ok = marked[c.PosterID] | |||
if ok { |
@@ -179,11 +179,9 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment, ori | |||
return | |||
} | |||
for _, c := range comments { | |||
if err := c.LoadAttachments(ctx); err != nil { | |||
ctx.ServerError("LoadAttachments", err) | |||
return | |||
} | |||
if err := comments.LoadAttachments(ctx); err != nil { | |||
ctx.ServerError("LoadAttachments", err) | |||
return | |||
} | |||
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled |