Skip to content

Commit

Permalink
Some performance optimization on dashboard and issues page (#29010)
Browse files Browse the repository at this point in the history 
This PR do some loading speed optimization for feeds user interface
pages.
- Load action users batchly but not one by one.
- Load action repositories batchly but not one by one.
- Load action's Repo Owners batchly but not one by one.
- Load action's possible issues batchly but not one by one.
- Load action's possible comments batchly but not one by one.
  • Loading branch information
@lunny
lunny authored Mar 12, 2024
1 parent 75a9f61 commit aed3b53
Show file tree
Hide file tree
Showing 7 changed files with 228 additions and 75 deletions.
106 changes: 53 additions & 53 deletions models/activities/action.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ type Action struct {
Repo *repo_model.Repository `xorm:"-"`
CommentID int64 `xorm:"INDEX"`
Comment *issues_model.Comment `xorm:"-"`
Issue *issues_model.Issue `xorm:"-"` // get the issue id from content
IsDeleted bool `xorm:"NOT NULL DEFAULT false"`
RefName string
IsPrivate bool `xorm:"NOT NULL DEFAULT false"`
Expand Down Expand Up @@ -290,11 +291,6 @@ func (a *Action) GetRepoAbsoluteLink(ctx context.Context) string {
return setting.AppURL + url.PathEscape(a.GetRepoUserName(ctx)) + "/" + url.PathEscape(a.GetRepoName(ctx))
}

// GetCommentHTMLURL returns link to action comment.
func (a *Action) GetCommentHTMLURL(ctx context.Context) string {
return a.getCommentHTMLURL(ctx)
}

func (a *Action) loadComment(ctx context.Context) (err error) {
if a.CommentID == 0 || a.Comment != nil {
return nil
Expand All @@ -303,69 +299,44 @@ func (a *Action) loadComment(ctx context.Context) (err error) {
return err
}

func (a *Action) getCommentHTMLURL(ctx context.Context) string {
// GetCommentHTMLURL returns link to action comment.
func (a *Action) GetCommentHTMLURL(ctx context.Context) string {
if a == nil {
return "#"
}
_ = a.loadComment(ctx)
if a.Comment != nil {
return a.Comment.HTMLURL(ctx)
}
if len(a.GetIssueInfos()) == 0 {
return "#"
}
// Return link to issue
issueIDString := a.GetIssueInfos()[0]
issueID, err := strconv.ParseInt(issueIDString, 10, 64)
if err != nil {
return "#"
}

issue, err := issues_model.GetIssueByID(ctx, issueID)
if err != nil {
if err := a.LoadIssue(ctx); err != nil || a.Issue == nil {
return "#"
}

if err = issue.LoadRepo(ctx); err != nil {
if err := a.Issue.LoadRepo(ctx); err != nil {
return "#"
}

return issue.HTMLURL()
return a.Issue.HTMLURL()
}

// GetCommentLink returns link to action comment.
func (a *Action) GetCommentLink(ctx context.Context) string {
return a.getCommentLink(ctx)
}

func (a *Action) getCommentLink(ctx context.Context) string {
if a == nil {
return "#"
}
_ = a.loadComment(ctx)
if a.Comment != nil {
return a.Comment.Link(ctx)
}
if len(a.GetIssueInfos()) == 0 {
return "#"
}
// Return link to issue
issueIDString := a.GetIssueInfos()[0]
issueID, err := strconv.ParseInt(issueIDString, 10, 64)
if err != nil {
return "#"
}

issue, err := issues_model.GetIssueByID(ctx, issueID)
if err != nil {
if err := a.LoadIssue(ctx); err != nil || a.Issue == nil {
return "#"
}

if err = issue.LoadRepo(ctx); err != nil {
if err := a.Issue.LoadRepo(ctx); err != nil {
return "#"
}

return issue.Link()
return a.Issue.Link()
}

// GetBranch returns the action's repository branch.
Expand Down Expand Up @@ -393,6 +364,10 @@ func (a *Action) GetCreate() time.Time {
return a.CreatedUnix.AsTime()
}

func (a *Action) IsIssueEvent() bool {
return a.OpType.InActions("comment_issue", "approve_pull_request", "reject_pull_request", "comment_pull", "merge_pull_request")
}

// GetIssueInfos returns a list of associated information with the action.
func (a *Action) GetIssueInfos() []string {
// make sure it always returns 3 elements, because there are some access to the a[1] and a[2] without checking the length
Expand All @@ -403,27 +378,52 @@ func (a *Action) GetIssueInfos() []string {
return ret
}

func (a *Action) getIssueIndex() int64 {
infos := a.GetIssueInfos()
if len(infos) == 0 {
return 0
}
index, _ := strconv.ParseInt(infos[0], 10, 64)
return index
}

func (a *Action) LoadIssue(ctx context.Context) error {
if a.Issue != nil {
return nil
}
if index := a.getIssueIndex(); index > 0 {
issue, err := issues_model.GetIssueByIndex(ctx, a.RepoID, index)
if err != nil {
return err
}
a.Issue = issue
a.Issue.Repo = a.Repo
}
return nil
}

// GetIssueTitle returns the title of first issue associated with the action.
func (a *Action) GetIssueTitle(ctx context.Context) string {
index, _ := strconv.ParseInt(a.GetIssueInfos()[0], 10, 64)
issue, err := issues_model.GetIssueByIndex(ctx, a.RepoID, index)
if err != nil {
log.Error("GetIssueByIndex: %v", err)
return "500 when get issue"
if err := a.LoadIssue(ctx); err != nil {
log.Error("LoadIssue: %v", err)
return "<500 when get issue>"
}
if a.Issue == nil {
return "<Issue not found>"
}
return issue.Title
return a.Issue.Title
}

// GetIssueContent returns the content of first issue associated with
// this action.
// GetIssueContent returns the content of first issue associated with this action.
func (a *Action) GetIssueContent(ctx context.Context) string {
index, _ := strconv.ParseInt(a.GetIssueInfos()[0], 10, 64)
issue, err := issues_model.GetIssueByIndex(ctx, a.RepoID, index)
if err != nil {
log.Error("GetIssueByIndex: %v", err)
return "500 when get issue"
if err := a.LoadIssue(ctx); err != nil {
log.Error("LoadIssue: %v", err)
return "<500 when get issue>"
}
if a.Issue == nil {
return "<Content not found>"
}
return issue.Content
return a.Issue.Content
}

// GetFeedsOptions options for retrieving feeds
Expand Down Expand Up @@ -463,7 +463,7 @@ func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, err
return nil, 0, fmt.Errorf("FindAndCount: %w", err)
}

if err := ActionList(actions).loadAttributes(ctx); err != nil {
if err := ActionList(actions).LoadAttributes(ctx); err != nil {
return nil, 0, fmt.Errorf("LoadAttributes: %w", err)
}

Expand Down
134 changes: 113 additions & 21 deletions models/activities/action_list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@ package activities
import (
"context"
"fmt"
"strconv"

"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
)

// ActionList defines a list of actions
Expand All @@ -24,7 +29,7 @@ func (actions ActionList) getUserIDs() []int64 {
return userIDs.Values()
}

func (actions ActionList) loadUsers(ctx context.Context) (map[int64]*user_model.User, error) {
func (actions ActionList) LoadActUsers(ctx context.Context) (map[int64]*user_model.User, error) {
if len(actions) == 0 {
return nil, nil
}
Expand Down Expand Up @@ -52,7 +57,7 @@ func (actions ActionList) getRepoIDs() []int64 {
return repoIDs.Values()
}

func (actions ActionList) loadRepositories(ctx context.Context) error {
func (actions ActionList) LoadRepositories(ctx context.Context) error {
if len(actions) == 0 {
return nil
}
Expand All @@ -63,49 +68,136 @@ func (actions ActionList) loadRepositories(ctx context.Context) error {
if err != nil {
return fmt.Errorf("find repository: %w", err)
}

for _, action := range actions {
action.Repo = repoMaps[action.RepoID]
}
return nil
repos := repo_model.RepositoryList(util.ValuesOfMap(repoMaps))
return repos.LoadUnits(ctx)
}

func (actions ActionList) loadRepoOwner(ctx context.Context, userMap map[int64]*user_model.User) (err error) {
if userMap == nil {
userMap = make(map[int64]*user_model.User)
}

userSet := make(container.Set[int64], len(actions))
for _, action := range actions {
if action.Repo == nil {
continue
}
repoOwner, ok := userMap[action.Repo.OwnerID]
if !ok {
repoOwner, err = user_model.GetUserByID(ctx, action.Repo.OwnerID)
if err != nil {
if user_model.IsErrUserNotExist(err) {
continue
}
return err
}
userMap[repoOwner.ID] = repoOwner
if _, ok := userMap[action.Repo.OwnerID]; !ok {
userSet.Add(action.Repo.OwnerID)
}
}

if err := db.GetEngine(ctx).
In("id", userSet.Values()).
Find(&userMap); err != nil {
return fmt.Errorf("find user: %w", err)
}

for _, action := range actions {
if action.Repo != nil {
action.Repo.Owner = userMap[action.Repo.OwnerID]
}
action.Repo.Owner = repoOwner
}

return nil
}

// loadAttributes loads all attributes
func (actions ActionList) loadAttributes(ctx context.Context) error {
userMap, err := actions.loadUsers(ctx)
// LoadAttributes loads all attributes
func (actions ActionList) LoadAttributes(ctx context.Context) error {
// the load sequence cannot be changed because of the dependencies
userMap, err := actions.LoadActUsers(ctx)
if err != nil {
return err
}

if err := actions.loadRepositories(ctx); err != nil {
if err := actions.LoadRepositories(ctx); err != nil {
return err
}
if err := actions.loadRepoOwner(ctx, userMap); err != nil {
return err
}
if err := actions.LoadIssues(ctx); err != nil {
return err
}
return actions.LoadComments(ctx)
}

func (actions ActionList) LoadComments(ctx context.Context) error {
if len(actions) == 0 {
return nil
}

commentIDs := make([]int64, 0, len(actions))
for _, action := range actions {
if action.CommentID > 0 {
commentIDs = append(commentIDs, action.CommentID)
}
}

return actions.loadRepoOwner(ctx, userMap)
commentsMap := make(map[int64]*issues_model.Comment, len(commentIDs))
if err := db.GetEngine(ctx).In("id", commentIDs).Find(&commentsMap); err != nil {
return fmt.Errorf("find comment: %w", err)
}

for _, action := range actions {
if action.CommentID > 0 {
action.Comment = commentsMap[action.CommentID]
if action.Comment != nil {
action.Comment.Issue = action.Issue
}
}
}
return nil
}

func (actions ActionList) LoadIssues(ctx context.Context) error {
if len(actions) == 0 {
return nil
}

conditions := builder.NewCond()
issueNum := 0
for _, action := range actions {
if action.IsIssueEvent() {
infos := action.GetIssueInfos()
if len(infos) == 0 {
continue
}
index, _ := strconv.ParseInt(infos[0], 10, 64)
if index > 0 {
conditions = conditions.Or(builder.Eq{
"repo_id": action.RepoID,
"`index`": index,
})
issueNum++
}
}
}
if !conditions.IsValid() {
return nil
}

issuesMap := make(map[string]*issues_model.Issue, issueNum)
issues := make([]*issues_model.Issue, 0, issueNum)
if err := db.GetEngine(ctx).Where(conditions).Find(&issues); err != nil {
return fmt.Errorf("find issue: %w", err)
}
for _, issue := range issues {
issuesMap[fmt.Sprintf("%d-%d", issue.RepoID, issue.Index)] = issue
}

for _, action := range actions {
if !action.IsIssueEvent() {
continue
}
if index := action.getIssueIndex(); index > 0 {
if issue, ok := issuesMap[fmt.Sprintf("%d-%d", action.RepoID, index)]; ok {
action.Issue = issue
action.Issue.Repo = action.Repo
}
}
}
return nil
}
10 changes: 10 additions & 0 deletions models/issues/issue_list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,16 @@ func (issues IssueList) loadTotalTrackedTimes(ctx context.Context) (err error) {
}
trackedTimes := make(map[int64]int64, len(issues))

reposMap := make(map[int64]*repo_model.Repository, len(issues))
for _, issue := range issues {
reposMap[issue.RepoID] = issue.Repo
}
repos := repo_model.RepositoryListOfMap(reposMap)

if err := repos.LoadUnits(ctx); err != nil {
return err
}

ids := make([]int64, 0, len(issues))
for _, issue := range issues {
if issue.Repo.IsTimetrackerEnabled(ctx) {
Expand Down
Loading

0 comments on commit aed3b53

Please sign in to comment.