Comments list performance optimization (#5305)

This commit is contained in:
Lunny Xiao 2019-04-18 13:00:03 +08:00 committed by techknowlogick
parent 2262811e40
commit dd1acd7ce4
4 changed files with 555 additions and 42 deletions

View file

@ -272,6 +272,10 @@ func (issue *Issue) loadAttributes(e Engine) (err error) {
if err = issue.loadComments(e); err != nil { if err = issue.loadComments(e); err != nil {
return err return err
} }
if err = CommentList(issue.Comments).loadAttributes(e); err != nil {
return err
}
if issue.isTimetrackerEnabled(e) { if issue.isTimetrackerEnabled(e) {
if err = issue.loadTotalTimes(e); err != nil { if err = issue.loadTotalTimes(e); err != nil {
return err return err

View file

@ -154,6 +154,34 @@ func (c *Comment) LoadIssue() (err error) {
return return
} }
func (c *Comment) loadPoster(e Engine) (err error) {
if c.Poster != nil {
return nil
}
c.Poster, err = getUserByID(e, c.PosterID)
if err != nil {
if IsErrUserNotExist(err) {
c.PosterID = -1
c.Poster = NewGhostUser()
} else {
log.Error("getUserByID[%d]: %v", c.ID, err)
}
}
return err
}
func (c *Comment) loadAttachments(e Engine) (err error) {
if len(c.Attachments) > 0 {
return
}
c.Attachments, err = getAttachmentsByCommentID(e, c.ID)
if err != nil {
log.Error("getAttachmentsByCommentID[%d]: %v", c.ID, err)
}
return err
}
// AfterDelete is invoked from XORM after the object is deleted. // AfterDelete is invoked from XORM after the object is deleted.
func (c *Comment) AfterDelete() { func (c *Comment) AfterDelete() {
if c.ID <= 0 { if c.ID <= 0 {
@ -997,32 +1025,6 @@ func FindComments(opts FindCommentsOptions) ([]*Comment, error) {
return findComments(x, opts) return findComments(x, opts)
} }
// GetCommentsByIssueID returns all comments of an issue.
func GetCommentsByIssueID(issueID int64) ([]*Comment, error) {
return findComments(x, FindCommentsOptions{
IssueID: issueID,
Type: CommentTypeUnknown,
})
}
// GetCommentsByIssueIDSince returns a list of comments of an issue since a given time point.
func GetCommentsByIssueIDSince(issueID, since int64) ([]*Comment, error) {
return findComments(x, FindCommentsOptions{
IssueID: issueID,
Type: CommentTypeUnknown,
Since: since,
})
}
// GetCommentsByRepoIDSince returns a list of comments for all issues in a repo since a given time point.
func GetCommentsByRepoIDSince(repoID, since int64) ([]*Comment, error) {
return findComments(x, FindCommentsOptions{
RepoID: repoID,
Type: CommentTypeUnknown,
Since: since,
})
}
// UpdateComment updates information of comment. // UpdateComment updates information of comment.
func UpdateComment(doer *User, c *Comment, oldContent string) error { func UpdateComment(doer *User, c *Comment, oldContent string) error {
if _, err := x.ID(c.ID).AllCols().Update(c); err != nil { if _, err := x.ID(c.ID).AllCols().Update(c); err != nil {
@ -1039,6 +1041,9 @@ func UpdateComment(doer *User, c *Comment, oldContent string) error {
if err := c.Issue.LoadAttributes(); err != nil { if err := c.Issue.LoadAttributes(); err != nil {
return err return err
} }
if err := c.loadPoster(x); err != nil {
return err
}
mode, _ := AccessLevel(doer, c.Issue.Repo) mode, _ := AccessLevel(doer, c.Issue.Repo)
if err := PrepareWebhooks(c.Issue.Repo, HookEventIssueComment, &api.IssueCommentPayload{ if err := PrepareWebhooks(c.Issue.Repo, HookEventIssueComment, &api.IssueCommentPayload{
@ -1087,6 +1092,7 @@ func DeleteComment(doer *User, comment *Comment) error {
if err := sess.Commit(); err != nil { if err := sess.Commit(); err != nil {
return err return err
} }
sess.Close()
if err := comment.LoadPoster(); err != nil { if err := comment.LoadPoster(); err != nil {
return err return err
@ -1098,6 +1104,9 @@ func DeleteComment(doer *User, comment *Comment) error {
if err := comment.Issue.LoadAttributes(); err != nil { if err := comment.Issue.LoadAttributes(); err != nil {
return err return err
} }
if err := comment.loadPoster(x); err != nil {
return err
}
mode, _ := AccessLevel(doer, comment.Issue.Repo) mode, _ := AccessLevel(doer, comment.Issue.Repo)
@ -1154,6 +1163,11 @@ func fetchCodeCommentsByReview(e Engine, issue *Issue, currentUser *User, review
if err := issue.loadRepo(e); err != nil { if err := issue.loadRepo(e); err != nil {
return nil, err return nil, err
} }
if err := CommentList(comments).loadPosters(e); err != nil {
return nil, err
}
// Find all reviews by ReviewID // Find all reviews by ReviewID
reviews := make(map[int64]*Review) reviews := make(map[int64]*Review)
var ids = make([]int64, 0, len(comments)) var ids = make([]int64, 0, len(comments))

View file

@ -8,18 +8,13 @@ package models
type CommentList []*Comment type CommentList []*Comment
func (comments CommentList) getPosterIDs() []int64 { func (comments CommentList) getPosterIDs() []int64 {
commentIDs := make(map[int64]struct{}, len(comments)) posterIDs := make(map[int64]struct{}, len(comments))
for _, comment := range comments { for _, comment := range comments {
if _, ok := commentIDs[comment.PosterID]; !ok { if _, ok := posterIDs[comment.PosterID]; !ok {
commentIDs[comment.PosterID] = struct{}{} posterIDs[comment.PosterID] = struct{}{}
} }
} }
return keysInt64(commentIDs) return keysInt64(posterIDs)
}
// LoadPosters loads posters from database
func (comments CommentList) LoadPosters() error {
return comments.loadPosters(x)
} }
func (comments CommentList) loadPosters(e Engine) error { func (comments CommentList) loadPosters(e Engine) error {
@ -56,3 +51,488 @@ func (comments CommentList) loadPosters(e Engine) error {
} }
return nil return nil
} }
func (comments CommentList) getCommentIDs() []int64 {
var ids = make([]int64, 0, len(comments))
for _, comment := range comments {
ids = append(ids, comment.ID)
}
return ids
}
func (comments CommentList) getLabelIDs() []int64 {
var ids = make(map[int64]struct{}, len(comments))
for _, comment := range comments {
if _, ok := ids[comment.LabelID]; !ok {
ids[comment.LabelID] = struct{}{}
}
}
return keysInt64(ids)
}
func (comments CommentList) loadLabels(e Engine) error {
if len(comments) == 0 {
return nil
}
var labelIDs = comments.getLabelIDs()
var commentLabels = make(map[int64]*Label, len(labelIDs))
var left = len(labelIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
rows, err := e.
In("id", labelIDs[:limit]).
Rows(new(Label))
if err != nil {
return err
}
for rows.Next() {
var label Label
err = rows.Scan(&label)
if err != nil {
rows.Close()
return err
}
commentLabels[label.ID] = &label
}
rows.Close()
left = left - limit
labelIDs = labelIDs[limit:]
}
for _, comment := range comments {
comment.Label = commentLabels[comment.ID]
}
return nil
}
func (comments CommentList) getMilestoneIDs() []int64 {
var ids = make(map[int64]struct{}, len(comments))
for _, comment := range comments {
if _, ok := ids[comment.MilestoneID]; !ok {
ids[comment.MilestoneID] = struct{}{}
}
}
return keysInt64(ids)
}
func (comments CommentList) loadMilestones(e Engine) error {
if len(comments) == 0 {
return nil
}
milestoneIDs := comments.getMilestoneIDs()
if len(milestoneIDs) == 0 {
return nil
}
milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs))
var left = len(milestoneIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
err := e.
In("id", milestoneIDs[:limit]).
Find(&milestoneMaps)
if err != nil {
return err
}
left = left - limit
milestoneIDs = milestoneIDs[limit:]
}
for _, issue := range comments {
issue.Milestone = milestoneMaps[issue.MilestoneID]
}
return nil
}
func (comments CommentList) getOldMilestoneIDs() []int64 {
var ids = make(map[int64]struct{}, len(comments))
for _, comment := range comments {
if _, ok := ids[comment.OldMilestoneID]; !ok {
ids[comment.OldMilestoneID] = struct{}{}
}
}
return keysInt64(ids)
}
func (comments CommentList) loadOldMilestones(e Engine) error {
if len(comments) == 0 {
return nil
}
milestoneIDs := comments.getOldMilestoneIDs()
if len(milestoneIDs) == 0 {
return nil
}
milestoneMaps := make(map[int64]*Milestone, len(milestoneIDs))
var left = len(milestoneIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
err := e.
In("id", milestoneIDs[:limit]).
Find(&milestoneMaps)
if err != nil {
return err
}
left = left - limit
milestoneIDs = milestoneIDs[limit:]
}
for _, issue := range comments {
issue.OldMilestone = milestoneMaps[issue.MilestoneID]
}
return nil
}
func (comments CommentList) getAssigneeIDs() []int64 {
var ids = make(map[int64]struct{}, len(comments))
for _, comment := range comments {
if _, ok := ids[comment.AssigneeID]; !ok {
ids[comment.AssigneeID] = struct{}{}
}
}
return keysInt64(ids)
}
func (comments CommentList) loadAssignees(e Engine) error {
if len(comments) == 0 {
return nil
}
var assigneeIDs = comments.getAssigneeIDs()
var assignees = make(map[int64]*User, len(assigneeIDs))
var left = len(assigneeIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
rows, err := e.
In("id", assigneeIDs[:limit]).
Rows(new(User))
if err != nil {
return err
}
for rows.Next() {
var user User
err = rows.Scan(&user)
if err != nil {
rows.Close()
return err
}
assignees[user.ID] = &user
}
rows.Close()
left = left - limit
assigneeIDs = assigneeIDs[limit:]
}
for _, comment := range comments {
comment.Assignee = assignees[comment.AssigneeID]
}
return nil
}
// getIssueIDs returns all the issue ids on this comment list which issue hasn't been loaded
func (comments CommentList) getIssueIDs() []int64 {
var ids = make(map[int64]struct{}, len(comments))
for _, comment := range comments {
if comment.Issue != nil {
continue
}
if _, ok := ids[comment.IssueID]; !ok {
ids[comment.IssueID] = struct{}{}
}
}
return keysInt64(ids)
}
// Issues returns all the issues of comments
func (comments CommentList) Issues() IssueList {
var issues = make(map[int64]*Issue, len(comments))
for _, comment := range comments {
if comment.Issue != nil {
if _, ok := issues[comment.Issue.ID]; !ok {
issues[comment.Issue.ID] = comment.Issue
}
}
}
var issueList = make([]*Issue, 0, len(issues))
for _, issue := range issues {
issueList = append(issueList, issue)
}
return issueList
}
func (comments CommentList) loadIssues(e Engine) error {
if len(comments) == 0 {
return nil
}
var issueIDs = comments.getIssueIDs()
var issues = make(map[int64]*Issue, len(issueIDs))
var left = len(issueIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
rows, err := e.
In("id", issueIDs[:limit]).
Rows(new(Issue))
if err != nil {
return err
}
for rows.Next() {
var issue Issue
err = rows.Scan(&issue)
if err != nil {
rows.Close()
return err
}
issues[issue.ID] = &issue
}
rows.Close()
left = left - limit
issueIDs = issueIDs[limit:]
}
for _, comment := range comments {
if comment.Issue == nil {
comment.Issue = issues[comment.IssueID]
}
}
return nil
}
func (comments CommentList) getDependentIssueIDs() []int64 {
var ids = make(map[int64]struct{}, len(comments))
for _, comment := range comments {
if comment.DependentIssue != nil {
continue
}
if _, ok := ids[comment.DependentIssueID]; !ok {
ids[comment.DependentIssueID] = struct{}{}
}
}
return keysInt64(ids)
}
func (comments CommentList) loadDependentIssues(e Engine) error {
if len(comments) == 0 {
return nil
}
var issueIDs = comments.getDependentIssueIDs()
var issues = make(map[int64]*Issue, len(issueIDs))
var left = len(issueIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
rows, err := e.
In("id", issueIDs[:limit]).
Rows(new(Issue))
if err != nil {
return err
}
for rows.Next() {
var issue Issue
err = rows.Scan(&issue)
if err != nil {
rows.Close()
return err
}
issues[issue.ID] = &issue
}
rows.Close()
left = left - limit
issueIDs = issueIDs[limit:]
}
for _, comment := range comments {
if comment.DependentIssue == nil {
comment.DependentIssue = issues[comment.DependentIssueID]
}
}
return nil
}
func (comments CommentList) loadAttachments(e Engine) (err error) {
if len(comments) == 0 {
return nil
}
var attachments = make(map[int64][]*Attachment, len(comments))
var commentsIDs = comments.getCommentIDs()
var left = len(commentsIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
rows, err := e.Table("attachment").
Join("INNER", "comment", "comment.id = attachment.comment_id").
In("comment.id", commentsIDs[:limit]).
Rows(new(Attachment))
if err != nil {
return err
}
for rows.Next() {
var attachment Attachment
err = rows.Scan(&attachment)
if err != nil {
rows.Close()
return err
}
attachments[attachment.CommentID] = append(attachments[attachment.CommentID], &attachment)
}
rows.Close()
left = left - limit
commentsIDs = commentsIDs[limit:]
}
for _, comment := range comments {
comment.Attachments = attachments[comment.ID]
}
return nil
}
func (comments CommentList) getReviewIDs() []int64 {
var ids = make(map[int64]struct{}, len(comments))
for _, comment := range comments {
if _, ok := ids[comment.ReviewID]; !ok {
ids[comment.ReviewID] = struct{}{}
}
}
return keysInt64(ids)
}
func (comments CommentList) loadReviews(e Engine) error {
if len(comments) == 0 {
return nil
}
var reviewIDs = comments.getReviewIDs()
var reviews = make(map[int64]*Review, len(reviewIDs))
var left = len(reviewIDs)
for left > 0 {
var limit = defaultMaxInSize
if left < limit {
limit = left
}
rows, err := e.
In("id", reviewIDs[:limit]).
Rows(new(Review))
if err != nil {
return err
}
for rows.Next() {
var review Review
err = rows.Scan(&review)
if err != nil {
rows.Close()
return err
}
reviews[review.ID] = &review
}
rows.Close()
left = left - limit
reviewIDs = reviewIDs[limit:]
}
for _, comment := range comments {
comment.Review = reviews[comment.ReviewID]
}
return nil
}
// loadAttributes loads all attributes
func (comments CommentList) loadAttributes(e Engine) (err error) {
if err = comments.loadPosters(e); err != nil {
return
}
if err = comments.loadLabels(e); err != nil {
return
}
if err = comments.loadMilestones(e); err != nil {
return
}
if err = comments.loadOldMilestones(e); err != nil {
return
}
if err = comments.loadAssignees(e); err != nil {
return
}
if err = comments.loadAttachments(e); err != nil {
return
}
if err = comments.loadReviews(e); err != nil {
return
}
if err = comments.loadIssues(e); err != nil {
return
}
if err = comments.loadDependentIssues(e); err != nil {
return
}
return nil
}
// LoadAttributes loads attributes of the comments, except for attachments and
// comments
func (comments CommentList) LoadAttributes() error {
return comments.loadAttributes(x)
}
// LoadAttachments loads attachments
func (comments CommentList) LoadAttachments() error {
return comments.loadAttachments(x)
}
// LoadPosters loads posters
func (comments CommentList) LoadPosters() error {
return comments.loadPosters(x)
}
// LoadIssues loads issues of comments
func (comments CommentList) LoadIssues() error {
return comments.loadIssues(x)
}

View file

@ -57,6 +57,7 @@ func ListIssueComments(ctx *context.APIContext) {
ctx.Error(500, "GetRawIssueByIndex", err) ctx.Error(500, "GetRawIssueByIndex", err)
return return
} }
issue.Repo = ctx.Repo.Repository
comments, err := models.FindComments(models.FindCommentsOptions{ comments, err := models.FindComments(models.FindCommentsOptions{
IssueID: issue.ID, IssueID: issue.ID,
@ -64,16 +65,18 @@ func ListIssueComments(ctx *context.APIContext) {
Type: models.CommentTypeComment, Type: models.CommentTypeComment,
}) })
if err != nil { if err != nil {
ctx.Error(500, "GetCommentsByIssueIDSince", err) ctx.Error(500, "FindComments", err)
return
}
if err := models.CommentList(comments).LoadPosters(); err != nil {
ctx.Error(500, "LoadPosters", err)
return return
} }
apiComments := make([]*api.Comment, len(comments)) apiComments := make([]*api.Comment, len(comments))
if err = models.CommentList(comments).LoadPosters(); err != nil { for i, comment := range comments {
ctx.Error(500, "LoadPosters", err) comment.Issue = issue
return
}
for i := range comments {
apiComments[i] = comments[i].APIFormat() apiComments[i] = comments[i].APIFormat()
} }
ctx.JSON(200, &apiComments) ctx.JSON(200, &apiComments)
@ -115,7 +118,7 @@ func ListRepoIssueComments(ctx *context.APIContext) {
Type: models.CommentTypeComment, Type: models.CommentTypeComment,
}) })
if err != nil { if err != nil {
ctx.Error(500, "GetCommentsByRepoIDSince", err) ctx.Error(500, "FindComments", err)
return return
} }
@ -125,6 +128,18 @@ func ListRepoIssueComments(ctx *context.APIContext) {
} }
apiComments := make([]*api.Comment, len(comments)) apiComments := make([]*api.Comment, len(comments))
if err := models.CommentList(comments).LoadIssues(); err != nil {
ctx.Error(500, "LoadIssues", err)
return
}
if err := models.CommentList(comments).LoadPosters(); err != nil {
ctx.Error(500, "LoadPosters", err)
return
}
if _, err := models.CommentList(comments).Issues().LoadRepositories(); err != nil {
ctx.Error(500, "LoadRepositories", err)
return
}
for i := range comments { for i := range comments {
apiComments[i] = comments[i].APIFormat() apiComments[i] = comments[i].APIFormat()
} }