perf(pull): load the repository once for all commit statuses (#39930)

Pages showing commit statuses loaded the repository of every status
separately, so a PR with ~75 statuses ran ~75 identical repository
queries per merge box render (about 3.4s in a production trace).
`statusRepoCanReadActions` only loaded the repository inside the
permission cache loader, which never runs once `RepoAssignment` has
published the permission.

- `CommitStatus.loadRepository` resolves through the request context
cache
- `statusRepoCanReadActions` loads the repository before the permission
lookup
- `RepoAssignment` publishes its repository to the context cache next to
the permission
- `GetCommitActionsStatusMap` drops its own repository map

Queries per request with 50 statuses (45 Actions, 5 external) on the
head commit, SQLite:

| Page | Repository queries | Total queries |
|---|---|---|
| PR view | 61 → 11 | 157 → 107 |
| Commit | 6 → 5 | 24 → 23 |
| Branches | 6 → 5 | 33 → 32 |
| Commits | 6 → 5 | 23 → 22 |
| Repo home | 8 → 7 | 35 → 34 |
| `/pulls` | 15 → 15 | 37 → 37 |

---------

Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
Lunny Xiaoandsilverwind authored and GitHub committed 2026-10-31 17:32:03 +01:00
1 parent bbb8ef8244
commit 40507491f7
5 files changed
+17 -22

No files matched your search

+9 -10
View File
@@ -182,7 +182,7 @@ func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (in
func (status *CommitStatus) loadRepository(ctx context.Context) (err error) {
if status.Repo == nil {
status.Repo, err = repo_model.GetRepositoryByID(ctx, status.RepoID)
status.Repo, err = cache.GetWithContextCache(ctx, cachegroup.Repository, status.RepoID, repo_model.GetRepositoryByID)
if err != nil {
return fmt.Errorf("getRepositoryByID [%d]: %w", status.RepoID, err)
}
@@ -230,11 +230,9 @@ func (status *CommitStatus) cutTargetURLGiteaActionsPrefix(ctx context.Context)
return "", false
}
if status.Repo == nil {
if err := status.loadRepository(ctx); err != nil {
log.Error("loadRepository: %v", err)
return "", false
}
if err := status.loadRepository(ctx); err != nil {
log.Error("loadRepository: %v", err)
return "", false
}
prefix := status.Repo.Link() + "/actions"
@@ -572,11 +570,12 @@ func SignCommitsApplyDoerPermission(ctx context.Context, doer *user_model.User,
}
func statusRepoCanReadActions(ctx context.Context, doer *user_model.User, status *CommitStatus) bool {
if err := status.loadRepository(ctx); err != nil {
log.Error("loadRepository: %v", err)
return false
}
perm, err := cache.GetWithContextCache(ctx, cachegroup.RepoUserPermission, access_model.RepoUserPermissionCacheKey(status.RepoID, doer),
func(ctx context.Context, _ string) (access_model.Permission, error) { // only runs on a cache miss
if err := status.loadRepository(ctx); err != nil {
return access_model.Permission{}, err
}
func(ctx context.Context, _key string) (access_model.Permission, error) {
return access_model.GetDoerRepoPermission(ctx, status.Repo, doer)
},
)
+4 -1
View File
@@ -13,6 +13,7 @@ import (
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
"gitea.dev/modules/cache"
"gitea.dev/modules/commitstatus"
"gitea.dev/modules/git"
@@ -200,8 +201,10 @@ func TestCommitStatusesApplyDoerPermission(t *testing.T) {
},
}
git_model.CommitStatusesApplyDoerPermission(t.Context(), doer, statuses)
git_model.CommitStatusesApplyDoerPermission(cache.WithCacheContext(t.Context()), doer, statuses)
assert.Equal(t, visibleURL, statuses[0].TargetURL)
assert.Empty(t, statuses[1].TargetURL)
assert.Equal(t, "https://mycicd.org/1", statuses[2].TargetURL)
assert.NotNil(t, statuses[0].Repo)
assert.Same(t, statuses[0].Repo, statuses[2].Repo)
}
+1 -10
View File
@@ -11,7 +11,6 @@ import (
actions_model "gitea.dev/models/actions"
"gitea.dev/models/db"
git_model "gitea.dev/models/git"
repo_model "gitea.dev/models/repo"
"gitea.dev/modules/log"
)
@@ -36,19 +35,11 @@ func GetCommitActionsStatusMap(ctx context.Context, statuses []*git_model.Commit
return nil
}
statusByJobID := make(map[int64]*git_model.CommitStatus)
repoByID := make(map[int64]*repo_model.Repository)
for _, status := range statuses {
if status == nil || status.TargetURL == "" {
continue
}
if status.Repo == nil {
status.Repo = repoByID[status.RepoID]
}
// ParseGiteaActionsTargetURL lazy-loads status.Repo on miss; cache the
// outcome so later entries with the same RepoID skip that load.
_, jobID, ok := status.ParseGiteaActionsTargetURL(ctx)
repoByID[status.RepoID] = status.Repo
if ok {
if _, jobID, ok := status.ParseGiteaActionsTargetURL(ctx); ok {
statusByJobID[jobID] = status
}
}
+1
View File
@@ -8,5 +8,6 @@ const (
EmailAvatarLink = "email_avatar_link"
UserEmailAddresses = "user_email_addresses"
GPGKeyWithSubKeys = "gpg_key_with_subkeys"
Repository = "repository"
RepoUserPermission = "repo_user_permission"
)
+2 -1
View File
@@ -455,8 +455,9 @@ func repoAssignmentLegacy(ctx *Context, data *repoAssignmentPrepareDataStruct) {
return
}
}
// publish it so code resolving the same permission later in this request reuses it
// publish them so code resolving the same repository or permission later in this request reuses them
if c := cache.GetContextCache(ctx); c != nil {
c.Put(cachegroup.Repository, repo.ID, repo)
c.Put(cachegroup.RepoUserPermission, access_model.RepoUserPermissionCacheKey(repo.ID, ctx.Doer), ctx.Repo.Permission)
}