Skip to content

Commit 093e0a3

Browse files
lunnylunny
committed
performance improvements for pull request list API
1 parent cd7d131 commit 093e0a3

File tree

9 files changed

+200
-99
lines changed

9 files changed

+200
-99
lines changed

‎models/issues/assignees.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ func init() {
2727

2828
// LoadAssignees load assignees of this issue.
2929
func (issue *Issue) LoadAssignees(ctx context.Context) (err error) {
30+
if issue.isAssigneeLoaded || len(issue.Assignees) > 0 {
31+
return nil
32+
}
33+
3034
// Reset maybe preexisting assignees
3135
issue.Assignees = []*user_model.User{}
3236
issue.Assignee = nil
@@ -35,6 +39,7 @@ func (issue *Issue) LoadAssignees(ctx context.Context) (err error) {
3539
Join("INNER", "issue_assignees", "assignee_id = `user`.id").
3640
Where("issue_assignees.issue_id = ?", issue.ID).
3741
Find(&issue.Assignees)
42+
issue.isAssigneeLoaded = true
3843
if err != nil {
3944
return err
4045
}

‎models/issues/issue.go

Lines changed: 52 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -98,44 +98,48 @@ var ErrIssueAlreadyChanged = util.NewInvalidArgumentErrorf("the issue is already
9898

9999
// Issue represents an issue or pull request of repository.
100100
type Issue struct {
101-
ID int64 `xorm:"pk autoincr"`
102-
RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"`
103-
Repo *repo_model.Repository `xorm:"-"`
104-
Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository.
105-
PosterID int64 `xorm:"INDEX"`
106-
Poster *user_model.User `xorm:"-"`
107-
OriginalAuthor string
108-
OriginalAuthorID int64 `xorm:"index"`
109-
Title string `xorm:"name"`
110-
Content string `xorm:"LONGTEXT"`
111-
RenderedContent template.HTML `xorm:"-"`
112-
ContentVersion int `xorm:"NOT NULL DEFAULT 0"`
113-
Labels []*Label `xorm:"-"`
114-
MilestoneID int64 `xorm:"INDEX"`
115-
Milestone *Milestone `xorm:"-"`
116-
Project *project_model.Project `xorm:"-"`
117-
Priority int
118-
AssigneeID int64 `xorm:"-"`
119-
Assignee *user_model.User `xorm:"-"`
120-
IsClosed bool `xorm:"INDEX"`
121-
IsRead bool `xorm:"-"`
122-
IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not.
123-
PullRequest *PullRequest `xorm:"-"`
124-
NumComments int
125-
Ref string
126-
PinOrder int `xorm:"DEFAULT 0"`
101+
ID int64 `xorm:"pk autoincr"`
102+
RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"`
103+
Repo *repo_model.Repository `xorm:"-"`
104+
Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository.
105+
PosterID int64 `xorm:"INDEX"`
106+
Poster *user_model.User `xorm:"-"`
107+
OriginalAuthor string
108+
OriginalAuthorID int64 `xorm:"index"`
109+
Title string `xorm:"name"`
110+
Content string `xorm:"LONGTEXT"`
111+
RenderedContent template.HTML `xorm:"-"`
112+
ContentVersion int `xorm:"NOT NULL DEFAULT 0"`
113+
Labels []*Label `xorm:"-"`
114+
isLabelsLoaded bool `xorm:"-"`
115+
MilestoneID int64 `xorm:"INDEX"`
116+
Milestone *Milestone `xorm:"-"`
117+
isMilestoneLoaded bool `xorm:"-"`
118+
Project *project_model.Project `xorm:"-"`
119+
Priority int
120+
AssigneeID int64 `xorm:"-"`
121+
Assignee *user_model.User `xorm:"-"`
122+
isAssigneeLoaded bool `xorm:"-"`
123+
IsClosed bool `xorm:"INDEX"`
124+
IsRead bool `xorm:"-"`
125+
IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not.
126+
PullRequest *PullRequest `xorm:"-"`
127+
NumComments int
128+
Ref string
129+
PinOrder int `xorm:"DEFAULT 0"`
127130

128131
DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"`
129132

130133
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
131134
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
132135
ClosedUnix timeutil.TimeStamp `xorm:"INDEX"`
133136

134-
Attachments []*repo_model.Attachment `xorm:"-"`
135-
Comments CommentList `xorm:"-"`
136-
Reactions ReactionList `xorm:"-"`
137-
TotalTrackedTime int64 `xorm:"-"`
138-
Assignees []*user_model.User `xorm:"-"`
137+
Attachments []*repo_model.Attachment `xorm:"-"`
138+
isAttachmentsLoaded bool `xorm:"-"`
139+
Comments CommentList `xorm:"-"`
140+
Reactions ReactionList `xorm:"-"`
141+
TotalTrackedTime int64 `xorm:"-"`
142+
Assignees []*user_model.User `xorm:"-"`
139143

140144
// IsLocked limits commenting abilities to users on an issue
141145
// with write access
@@ -187,6 +191,19 @@ func (issue *Issue) LoadRepo(ctx context.Context) (err error) {
187191
return nil
188192
}
189193

194+
func (issue *Issue) LoadAttachments(ctx context.Context) (err error) {
195+
if issue.isAttachmentsLoaded || issue.Attachments != nil {
196+
return nil
197+
}
198+
199+
issue.Attachments, err = repo_model.GetAttachmentsByIssueID(ctx, issue.ID)
200+
issue.isAttachmentsLoaded = true
201+
if err != nil {
202+
return fmt.Errorf("getAttachmentsByIssueID [%d]: %w", issue.ID, err)
203+
}
204+
return nil
205+
}
206+
190207
// IsTimetrackerEnabled returns true if the repo enables timetracking
191208
func (issue *Issue) IsTimetrackerEnabled(ctx context.Context) bool {
192209
if err := issue.LoadRepo(ctx); err != nil {
@@ -287,8 +304,9 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) {
287304

288305
// LoadMilestone load milestone of this issue.
289306
func (issue *Issue) LoadMilestone(ctx context.Context) (err error) {
290-
if (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 {
307+
if !issue.isMilestoneLoaded && (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 {
291308
issue.Milestone, err = GetMilestoneByRepoID(ctx, issue.RepoID, issue.MilestoneID)
309+
issue.isMilestoneLoaded = true
292310
if err != nil && !IsErrMilestoneNotExist(err) {
293311
return fmt.Errorf("getMilestoneByRepoID [repo_id: %d, milestone_id: %d]: %w", issue.RepoID, issue.MilestoneID, err)
294312
}
@@ -327,11 +345,8 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
327345
return err
328346
}
329347

330-
if issue.Attachments == nil {
331-
issue.Attachments, err = repo_model.GetAttachmentsByIssueID(ctx, issue.ID)
332-
if err != nil {
333-
return fmt.Errorf("getAttachmentsByIssueID [%d]: %w", issue.ID, err)
334-
}
348+
if err = issue.LoadAttachments(ctx); err != nil {
349+
return err
335350
}
336351

337352
if err = issue.loadComments(ctx); err != nil {

‎models/issues/issue_label.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,9 @@ func FixIssueLabelWithOutsideLabels(ctx context.Context) (int64, error) {
325325

326326
// LoadLabels loads labels
327327
func (issue *Issue) LoadLabels(ctx context.Context) (err error) {
328-
if issue.Labels == nil && issue.ID != 0 {
328+
if !issue.isLabelsLoaded && issue.Labels == nil && issue.ID != 0 {
329329
issue.Labels, err = GetLabelsByIssueID(ctx, issue.ID)
330+
issue.isLabelsLoaded = true
330331
if err != nil {
331332
return fmt.Errorf("getLabelsByIssueID [%d]: %w", issue.ID, err)
332333
}

‎models/issues/issue_list.go

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,11 @@ func (issues IssueList) LoadRepositories(ctx context.Context) (repo_model.Reposi
7474

7575
func (issues IssueList) getPosterIDs() []int64 {
7676
return container.FilterSlice(issues, func(issue *Issue) (int64, bool) {
77-
return issue.PosterID, true
77+
return issue.PosterID, issue.Poster == nil
7878
})
7979
}
8080

81-
func (issues IssueList) loadPosters(ctx context.Context) error {
81+
func (issues IssueList) LoadPosters(ctx context.Context) error {
8282
if len(issues) == 0 {
8383
return nil
8484
}
@@ -136,7 +136,7 @@ func (issues IssueList) getIssueIDs() []int64 {
136136
return ids
137137
}
138138

139-
func (issues IssueList) loadLabels(ctx context.Context) error {
139+
func (issues IssueList) LoadLabels(ctx context.Context) error {
140140
if len(issues) == 0 {
141141
return nil
142142
}
@@ -168,7 +168,7 @@ func (issues IssueList) loadLabels(ctx context.Context) error {
168168
err = rows.Scan(&labelIssue)
169169
if err != nil {
170170
if err1 := rows.Close(); err1 != nil {
171-
return fmt.Errorf("IssueList.loadLabels: Close: %w", err1)
171+
return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1)
172172
}
173173
return err
174174
}
@@ -177,14 +177,15 @@ func (issues IssueList) loadLabels(ctx context.Context) error {
177177
// When there are no rows left and we try to close it.
178178
// Since that is not relevant for us, we can safely ignore it.
179179
if err1 := rows.Close(); err1 != nil {
180-
return fmt.Errorf("IssueList.loadLabels: Close: %w", err1)
180+
return fmt.Errorf("IssueList.LoadLabels: Close: %w", err1)
181181
}
182182
left -= limit
183183
issueIDs = issueIDs[limit:]
184184
}
185185

186186
for _, issue := range issues {
187187
issue.Labels = issueLabels[issue.ID]
188+
issue.isLabelsLoaded = true
188189
}
189190
return nil
190191
}
@@ -195,7 +196,7 @@ func (issues IssueList) getMilestoneIDs() []int64 {
195196
})
196197
}
197198

198-
func (issues IssueList) loadMilestones(ctx context.Context) error {
199+
func (issues IssueList) LoadMilestones(ctx context.Context) error {
199200
milestoneIDs := issues.getMilestoneIDs()
200201
if len(milestoneIDs) == 0 {
201202
return nil
@@ -220,6 +221,7 @@ func (issues IssueList) loadMilestones(ctx context.Context) error {
220221

221222
for _, issue := range issues {
222223
issue.Milestone = milestoneMaps[issue.MilestoneID]
224+
issue.isMilestoneLoaded = true
223225
}
224226
return nil
225227
}
@@ -263,7 +265,7 @@ func (issues IssueList) LoadProjects(ctx context.Context) error {
263265
return nil
264266
}
265267

266-
func (issues IssueList) loadAssignees(ctx context.Context) error {
268+
func (issues IssueList) LoadAssignees(ctx context.Context) error {
267269
if len(issues) == 0 {
268270
return nil
269271
}
@@ -310,6 +312,7 @@ func (issues IssueList) loadAssignees(ctx context.Context) error {
310312

311313
for _, issue := range issues {
312314
issue.Assignees = assignees[issue.ID]
315+
issue.isAssigneeLoaded = true
313316
}
314317
return nil
315318
}
@@ -413,6 +416,7 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) {
413416

414417
for _, issue := range issues {
415418
issue.Attachments = attachments[issue.ID]
419+
issue.isAttachmentsLoaded = true
416420
}
417421
return nil
418422
}
@@ -538,23 +542,23 @@ func (issues IssueList) LoadAttributes(ctx context.Context) error {
538542
return fmt.Errorf("issue.loadAttributes: LoadRepositories: %w", err)
539543
}
540544

541-
if err := issues.loadPosters(ctx); err != nil {
542-
return fmt.Errorf("issue.loadAttributes: loadPosters: %w", err)
545+
if err := issues.LoadPosters(ctx); err != nil {
546+
return fmt.Errorf("issue.loadAttributes: LoadPosters: %w", err)
543547
}
544548

545-
if err := issues.loadLabels(ctx); err != nil {
546-
return fmt.Errorf("issue.loadAttributes: loadLabels: %w", err)
549+
if err := issues.LoadLabels(ctx); err != nil {
550+
return fmt.Errorf("issue.loadAttributes: LoadLabels: %w", err)
547551
}
548552

549-
if err := issues.loadMilestones(ctx); err != nil {
550-
return fmt.Errorf("issue.loadAttributes: loadMilestones: %w", err)
553+
if err := issues.LoadMilestones(ctx); err != nil {
554+
return fmt.Errorf("issue.loadAttributes: LoadMilestones: %w", err)
551555
}
552556

553557
if err := issues.LoadProjects(ctx); err != nil {
554558
return fmt.Errorf("issue.loadAttributes: loadProjects: %w", err)
555559
}
556560

557-
if err := issues.loadAssignees(ctx); err != nil {
561+
if err := issues.LoadAssignees(ctx); err != nil {
558562
return fmt.Errorf("issue.loadAttributes: loadAssignees: %w", err)
559563
}
560564

‎models/issues/pull.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,11 @@ type PullRequest struct {
159159

160160
ChangedProtectedFiles []string `xorm:"TEXT JSON"`
161161

162-
IssueID int64 `xorm:"INDEX"`
163-
Issue *Issue `xorm:"-"`
164-
Index int64
165-
RequestedReviewers []*user_model.User `xorm:"-"`
162+
IssueID int64 `xorm:"INDEX"`
163+
Issue *Issue `xorm:"-"`
164+
Index int64
165+
RequestedReviewers []*user_model.User `xorm:"-"`
166+
isRequestedReviewersLoaded bool `xorm:"-"`
166167

167168
HeadRepoID int64 `xorm:"INDEX"`
168169
HeadRepo *repo_model.Repository `xorm:"-"`
@@ -289,10 +290,11 @@ func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) {
289290

290291
// LoadRequestedReviewers loads the requested reviewers.
291292
func (pr *PullRequest) LoadRequestedReviewers(ctx context.Context) error {
292-
if len(pr.RequestedReviewers) > 0 {
293+
if pr.isRequestedReviewersLoaded || len(pr.RequestedReviewers) > 0 {
293294
return nil
294295
}
295296

297+
pr.isRequestedReviewersLoaded = true
296298
reviews, err := GetReviewsByIssueID(ctx, pr.Issue.ID)
297299
if err != nil {
298300
return err

0 commit comments

Comments
 (0)