Some performance optimization on dashboard and issues page (#29010)

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.
2024-03-12 12:57:19 +08:00 · 2024-03-12 12:57:19 +08:00 · aed3b53abd
commit aed3b53abd
parent 75a9f61f89
7 changed files with 234 additions and 81 deletions
--- a/models/activities/action.go
+++ b/models/activities/action.go
@ -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"`
@ -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
@ -303,7 +299,8 @@ 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 "#"
 	}
@ -311,34 +308,19 @@ func (a *Action) getCommentHTMLURL(ctx context.Context) string {
 	if a.Comment != nil {
 		return a.Comment.HTMLURL(ctx)
 	}
-	if len(a.GetIssueInfos()) == 0 {
+
+	if err := a.LoadIssue(ctx); err != nil || a.Issue == nil {
 		return "#"
 	}
-	// Return link to issue
-	issueIDString := a.GetIssueInfos()[0]
-	issueID, err := strconv.ParseInt(issueIDString, 10, 64)
-	if err != nil {
+	if err := a.Issue.LoadRepo(ctx); err != nil {
 		return "#"
 	}

-	issue, err := issues_model.GetIssueByID(ctx, issueID)
-	if err != nil {
-		return "#"
-	}
-
-	if err = 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 "#"
 	}
@ -346,26 +328,15 @@ func (a *Action) getCommentLink(ctx context.Context) string {
 	if a.Comment != nil {
 		return a.Comment.Link(ctx)
 	}
-	if len(a.GetIssueInfos()) == 0 {
+
+	if err := a.LoadIssue(ctx); err != nil || a.Issue == nil {
 		return "#"
 	}
-	// Return link to issue
-	issueIDString := a.GetIssueInfos()[0]
-	issueID, err := strconv.ParseInt(issueIDString, 10, 64)
-	if err != nil {
+	if err := a.Issue.LoadRepo(ctx); err != nil {
 		return "#"
 	}

-	issue, err := issues_model.GetIssueByID(ctx, issueID)
-	if err != nil {
-		return "#"
-	}
-
-	if err = issue.LoadRepo(ctx); err != nil {
-		return "#"
-	}
-
-	return issue.Link()
+	return a.Issue.Link()
 }

 // GetBranch returns the action's repository branch.
@ -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
@ -403,27 +378,52 @@ func (a *Action) GetIssueInfos() []string {
 	return ret
 }

-// 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"
+func (a *Action) getIssueIndex() int64 {
+	infos := a.GetIssueInfos()
+	if len(infos) == 0 {
+		return 0
 	}
-	return issue.Title
+	index, _ := strconv.ParseInt(infos[0], 10, 64)
+	return index
 }

-// 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"
+func (a *Action) LoadIssue(ctx context.Context) error {
+	if a.Issue != nil {
+		return nil
 	}
-	return issue.Content
+	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 {
+	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 a.Issue.Title
+}
+
+// GetIssueContent returns the content of first issue associated with this action.
+func (a *Action) GetIssueContent(ctx context.Context) string {
+	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 a.Issue.Content
 }

 // GetFeedsOptions options for retrieving feeds
@ -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)
 	}

--- a/models/activities/action_list.go
+++ b/models/activities/action_list.go
@ -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
@ -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
 	}
@ -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
 	}
@ -63,11 +68,11 @@ 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) {
@ -75,37 +80,124 @@ func (actions ActionList) loadRepoOwner(ctx context.Context, userMap map[int64]*
 		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
 	}
-
-	return actions.loadRepoOwner(ctx, userMap)
+	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)
+		}
+	}
+
+	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
 }
--- a/models/issues/issue_list.go
+++ b/models/issues/issue_list.go
@ -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) {
--- a/models/repo/repo.go
+++ b/models/repo/repo.go
@ -531,6 +531,9 @@ func (repo *Repository) GetBaseRepo(ctx context.Context) (err error) {
 		return nil
 	}

+	if repo.BaseRepo != nil {
+		return nil
+	}
 	repo.BaseRepo, err = GetRepositoryByID(ctx, repo.ForkID)
 	return err
 }
--- a/models/repo/repo_list.go
+++ b/models/repo/repo_list.go
@ -63,6 +63,41 @@ func RepositoryListOfMap(repoMap map[int64]*Repository) RepositoryList {
 	return RepositoryList(ValuesRepository(repoMap))
 }

+func (repos RepositoryList) LoadUnits(ctx context.Context) error {
+	if len(repos) == 0 {
+		return nil
+	}
+
+	// Load units.
+	units := make([]*RepoUnit, 0, len(repos)*6)
+	if err := db.GetEngine(ctx).
+		In("repo_id", repos.IDs()).
+		Find(&units); err != nil {
+		return fmt.Errorf("find units: %w", err)
+	}
+
+	unitsMap := make(map[int64][]*RepoUnit, len(repos))
+	for _, unit := range units {
+		if !unit.Type.UnitGlobalDisabled() {
+			unitsMap[unit.RepoID] = append(unitsMap[unit.RepoID], unit)
+		}
+	}
+
+	for _, repo := range repos {
+		repo.Units = unitsMap[repo.ID]
+	}
+
+	return nil
+}
+
+func (repos RepositoryList) IDs() []int64 {
+	repoIDs := make([]int64, len(repos))
+	for i := range repos {
+		repoIDs[i] = repos[i].ID
+	}
+	return repoIDs
+}
+
 // LoadAttributes loads the attributes for the given RepositoryList
 func (repos RepositoryList) LoadAttributes(ctx context.Context) error {
 	if len(repos) == 0 {
--- a/modules/util/slice.go
+++ b/modules/util/slice.go
@ -53,3 +53,12 @@ func Sorted[S ~[]E, E cmp.Ordered](values S) S {
 	slices.Sort(values)
 	return values
 }
+
+// TODO: Replace with "maps.Values" once available
+func ValuesOfMap[K comparable, V any](m map[K]V) []V {
+	values := make([]V, 0, len(m))
+	for _, v := range m {
+		values = append(values, v)
+	}
+	return values
+}
--- a/routers/web/repo/repo.go
+++ b/routers/web/repo/repo.go
@ -547,9 +547,13 @@ func InitiateDownload(ctx *context.Context) {

 // SearchRepo repositories via options
 func SearchRepo(ctx *context.Context) {
+	page := ctx.FormInt("page")
+	if page <= 0 {
+		page = 1
+	}
 	opts := &repo_model.SearchRepoOptions{
 		ListOptions: db.ListOptions{
-			Page:     ctx.FormInt("page"),
+			Page:     page,
 			PageSize: convert.ToCorrectPageSize(ctx.FormInt("limit")),
 		},
 		Actor:              ctx.Doer,