Skip to content

Commit a2651c1

Browse files
KN4CK3Rwxiaoguang
andauthored
Add cache for common package queries (#22491)
This adds a cache for common package queries in `GetPackageDescriptor`. Code which needs to process a list of packages benefits from this change. This skips 350 queries in the package integration tests for example. --------- Co-authored-by: wxiaoguang <[email protected]>
1 parent 4dca869 commit a2651c1

File tree

4 files changed

+150
-208
lines changed

4 files changed

+150
-208
lines changed

models/packages/descriptor.go

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
repo_model "code.gitea.io/gitea/models/repo"
1313
user_model "code.gitea.io/gitea/models/user"
14+
"code.gitea.io/gitea/modules/cache"
1415
"code.gitea.io/gitea/modules/json"
1516
"code.gitea.io/gitea/modules/packages/alpine"
1617
"code.gitea.io/gitea/modules/packages/arch"
@@ -102,22 +103,26 @@ func (pd *PackageDescriptor) CalculateBlobSize() int64 {
102103

103104
// GetPackageDescriptor gets the package description for a version
104105
func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDescriptor, error) {
105-
p, err := GetPackageByID(ctx, pv.PackageID)
106+
return getPackageDescriptor(ctx, pv, cache.NewEphemeralCache())
107+
}
108+
109+
func getPackageDescriptor(ctx context.Context, pv *PackageVersion, c *cache.EphemeralCache) (*PackageDescriptor, error) {
110+
p, err := cache.GetWithEphemeralCache(ctx, c, "package", pv.PackageID, GetPackageByID)
106111
if err != nil {
107112
return nil, err
108113
}
109-
o, err := user_model.GetUserByID(ctx, p.OwnerID)
114+
o, err := cache.GetWithEphemeralCache(ctx, c, "user", p.OwnerID, user_model.GetUserByID)
110115
if err != nil {
111116
return nil, err
112117
}
113118
var repository *repo_model.Repository
114119
if p.RepoID > 0 {
115-
repository, err = repo_model.GetRepositoryByID(ctx, p.RepoID)
120+
repository, err = cache.GetWithEphemeralCache(ctx, c, "repo", p.RepoID, repo_model.GetRepositoryByID)
116121
if err != nil && !repo_model.IsErrRepoNotExist(err) {
117122
return nil, err
118123
}
119124
}
120-
creator, err := user_model.GetUserByID(ctx, pv.CreatorID)
125+
creator, err := cache.GetWithEphemeralCache(ctx, c, "user", pv.CreatorID, user_model.GetUserByID)
121126
if err != nil {
122127
if errors.Is(err, util.ErrNotExist) {
123128
creator = user_model.NewGhostUser()
@@ -145,9 +150,13 @@ func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDesc
145150
return nil, err
146151
}
147152

148-
pfds, err := GetPackageFileDescriptors(ctx, pfs)
149-
if err != nil {
150-
return nil, err
153+
pfds := make([]*PackageFileDescriptor, 0, len(pfs))
154+
for _, pf := range pfs {
155+
pfd, err := getPackageFileDescriptor(ctx, pf, c)
156+
if err != nil {
157+
return nil, err
158+
}
159+
pfds = append(pfds, pfd)
151160
}
152161

153162
var metadata any
@@ -221,7 +230,11 @@ func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDesc
221230

222231
// GetPackageFileDescriptor gets a package file descriptor for a package file
223232
func GetPackageFileDescriptor(ctx context.Context, pf *PackageFile) (*PackageFileDescriptor, error) {
224-
pb, err := GetBlobByID(ctx, pf.BlobID)
233+
return getPackageFileDescriptor(ctx, pf, cache.NewEphemeralCache())
234+
}
235+
236+
func getPackageFileDescriptor(ctx context.Context, pf *PackageFile, c *cache.EphemeralCache) (*PackageFileDescriptor, error) {
237+
pb, err := cache.GetWithEphemeralCache(ctx, c, "package_file_blob", pf.BlobID, GetBlobByID)
225238
if err != nil {
226239
return nil, err
227240
}
@@ -251,9 +264,13 @@ func GetPackageFileDescriptors(ctx context.Context, pfs []*PackageFile) ([]*Pack
251264

252265
// GetPackageDescriptors gets the package descriptions for the versions
253266
func GetPackageDescriptors(ctx context.Context, pvs []*PackageVersion) ([]*PackageDescriptor, error) {
267+
return getPackageDescriptors(ctx, pvs, cache.NewEphemeralCache())
268+
}
269+
270+
func getPackageDescriptors(ctx context.Context, pvs []*PackageVersion, c *cache.EphemeralCache) ([]*PackageDescriptor, error) {
254271
pds := make([]*PackageDescriptor, 0, len(pvs))
255272
for _, pv := range pvs {
256-
pd, err := GetPackageDescriptor(ctx, pv)
273+
pd, err := getPackageDescriptor(ctx, pv, c)
257274
if err != nil {
258275
return nil, err
259276
}

modules/cache/context.go

Lines changed: 19 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -5,176 +5,39 @@ package cache
55

66
import (
77
"context"
8-
"sync"
98
"time"
10-
11-
"code.gitea.io/gitea/modules/log"
129
)
1310

14-
// cacheContext is a context that can be used to cache data in a request level context
15-
// This is useful for caching data that is expensive to calculate and is likely to be
16-
// used multiple times in a request.
17-
type cacheContext struct {
18-
data map[any]map[any]any
19-
lock sync.RWMutex
20-
created time.Time
21-
discard bool
22-
}
23-
24-
func (cc *cacheContext) Get(tp, key any) any {
25-
cc.lock.RLock()
26-
defer cc.lock.RUnlock()
27-
return cc.data[tp][key]
28-
}
29-
30-
func (cc *cacheContext) Put(tp, key, value any) {
31-
cc.lock.Lock()
32-
defer cc.lock.Unlock()
33-
34-
if cc.discard {
35-
return
36-
}
37-
38-
d := cc.data[tp]
39-
if d == nil {
40-
d = make(map[any]any)
41-
cc.data[tp] = d
42-
}
43-
d[key] = value
44-
}
45-
46-
func (cc *cacheContext) Delete(tp, key any) {
47-
cc.lock.Lock()
48-
defer cc.lock.Unlock()
49-
delete(cc.data[tp], key)
50-
}
11+
type cacheContextKeyType struct{}
5112

52-
func (cc *cacheContext) Discard() {
53-
cc.lock.Lock()
54-
defer cc.lock.Unlock()
55-
cc.data = nil
56-
cc.discard = true
57-
}
13+
var cacheContextKey = cacheContextKeyType{}
5814

59-
func (cc *cacheContext) isDiscard() bool {
60-
cc.lock.RLock()
61-
defer cc.lock.RUnlock()
62-
return cc.discard
63-
}
64-
65-
// cacheContextLifetime is the max lifetime of cacheContext.
66-
// Since cacheContext is used to cache data in a request level context, 5 minutes is enough.
67-
// If a cacheContext is used more than 5 minutes, it's probably misuse.
68-
const cacheContextLifetime = 5 * time.Minute
69-
70-
var timeNow = time.Now
71-
72-
func (cc *cacheContext) Expired() bool {
73-
return timeNow().Sub(cc.created) > cacheContextLifetime
74-
}
75-
76-
var cacheContextKey = struct{}{}
77-
78-
/*
79-
Since there are both WithCacheContext and WithNoCacheContext,
80-
it may be confusing when there is nesting.
81-
82-
Some cases to explain the design:
83-
84-
When:
85-
- A, B or C means a cache context.
86-
- A', B' or C' means a discard cache context.
87-
- ctx means context.Backgrand().
88-
- A(ctx) means a cache context with ctx as the parent context.
89-
- B(A(ctx)) means a cache context with A(ctx) as the parent context.
90-
- With is alias of WithCacheContext.
91-
- WithNo is alias of WithNoCacheContext.
92-
93-
So:
94-
- With(ctx) -> A(ctx)
95-
- With(With(ctx)) -> A(ctx), not B(A(ctx)), always reuse parent cache context if possible.
96-
- With(With(With(ctx))) -> A(ctx), not C(B(A(ctx))), ditto.
97-
- WithNo(ctx) -> ctx, not A'(ctx), don't create new cache context if we don't have to.
98-
- WithNo(With(ctx)) -> A'(ctx)
99-
- WithNo(WithNo(With(ctx))) -> A'(ctx), not B'(A'(ctx)), don't create new cache context if we don't have to.
100-
- With(WithNo(With(ctx))) -> B(A'(ctx)), not A(ctx), never reuse a discard cache context.
101-
- WithNo(With(WithNo(With(ctx)))) -> B'(A'(ctx))
102-
- With(WithNo(With(WithNo(With(ctx))))) -> C(B'(A'(ctx))), so there's always only one not-discard cache context.
103-
*/
15+
// contextCacheLifetime is the max lifetime of context cache.
16+
// Since context cache is used to cache data in a request level context, 5 minutes is enough.
17+
// If a context cache is used more than 5 minutes, it's probably abused.
18+
const contextCacheLifetime = 5 * time.Minute
10419

10520
func WithCacheContext(ctx context.Context) context.Context {
106-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
107-
if !c.isDiscard() {
108-
// reuse parent context
109-
return ctx
110-
}
111-
}
112-
// FIXME: review the use of this nolint directive
113-
return context.WithValue(ctx, cacheContextKey, &cacheContext{ //nolint:staticcheck
114-
data: make(map[any]map[any]any),
115-
created: timeNow(),
116-
})
117-
}
118-
119-
func WithNoCacheContext(ctx context.Context) context.Context {
120-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
121-
// The caller want to run long-life tasks, but the parent context is a cache context.
122-
// So we should disable and clean the cache data, or it will be kept in memory for a long time.
123-
c.Discard()
21+
if c := GetContextCache(ctx); c != nil {
12422
return ctx
12523
}
126-
127-
return ctx
24+
return context.WithValue(ctx, cacheContextKey, NewEphemeralCache(contextCacheLifetime))
12825
}
12926

130-
func GetContextData(ctx context.Context, tp, key any) any {
131-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
132-
if c.Expired() {
133-
// The warning means that the cache context is misused for long-life task,
134-
// it can be resolved with WithNoCacheContext(ctx).
135-
log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c)
136-
return nil
137-
}
138-
return c.Get(tp, key)
139-
}
140-
return nil
141-
}
142-
143-
func SetContextData(ctx context.Context, tp, key, value any) {
144-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
145-
if c.Expired() {
146-
// The warning means that the cache context is misused for long-life task,
147-
// it can be resolved with WithNoCacheContext(ctx).
148-
log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c)
149-
return
150-
}
151-
c.Put(tp, key, value)
152-
return
153-
}
154-
}
155-
156-
func RemoveContextData(ctx context.Context, tp, key any) {
157-
if c, ok := ctx.Value(cacheContextKey).(*cacheContext); ok {
158-
if c.Expired() {
159-
// The warning means that the cache context is misused for long-life task,
160-
// it can be resolved with WithNoCacheContext(ctx).
161-
log.Warn("cache context is expired, is highly likely to be misused for long-life tasks: %v", c)
162-
return
163-
}
164-
c.Delete(tp, key)
165-
}
27+
func GetContextCache(ctx context.Context) *EphemeralCache {
28+
c, _ := ctx.Value(cacheContextKey).(*EphemeralCache)
29+
return c
16630
}
16731

16832
// GetWithContextCache returns the cache value of the given key in the given context.
33+
// FIXME: in some cases, the "context cache" should not be used, because it has uncontrollable behaviors
34+
// For example, these calls:
35+
// * GetWithContextCache(TargetID) -> OtherCodeCreateModel(TargetID) -> GetWithContextCache(TargetID)
36+
// Will cause the second call is not able to get the correct created target.
37+
// UNLESS it is certain that the target won't be changed during the request, DO NOT use it.
16938
func GetWithContextCache[T, K any](ctx context.Context, groupKey string, targetKey K, f func(context.Context, K) (T, error)) (T, error) {
170-
v := GetContextData(ctx, groupKey, targetKey)
171-
if vv, ok := v.(T); ok {
172-
return vv, nil
173-
}
174-
t, err := f(ctx, targetKey)
175-
if err != nil {
176-
return t, err
39+
if c := GetContextCache(ctx); c != nil {
40+
return GetWithEphemeralCache(ctx, c, groupKey, targetKey, f)
17741
}
178-
SetContextData(ctx, groupKey, targetKey, t)
179-
return t, nil
42+
return f(ctx, targetKey)
18043
}

modules/cache/context_test.go

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,29 @@ import (
88
"testing"
99
"time"
1010

11+
"code.gitea.io/gitea/modules/test"
12+
1113
"github.com/stretchr/testify/assert"
1214
)
1315

1416
func TestWithCacheContext(t *testing.T) {
1517
ctx := WithCacheContext(t.Context())
16-
17-
v := GetContextData(ctx, "empty_field", "my_config1")
18+
c := GetContextCache(ctx)
19+
v, _ := c.Get("empty_field", "my_config1")
1820
assert.Nil(t, v)
1921

2022
const field = "system_setting"
21-
v = GetContextData(ctx, field, "my_config1")
23+
v, _ = c.Get(field, "my_config1")
2224
assert.Nil(t, v)
23-
SetContextData(ctx, field, "my_config1", 1)
24-
v = GetContextData(ctx, field, "my_config1")
25+
c.Put(field, "my_config1", 1)
26+
v, _ = c.Get(field, "my_config1")
2527
assert.NotNil(t, v)
2628
assert.Equal(t, 1, v.(int))
2729

28-
RemoveContextData(ctx, field, "my_config1")
29-
RemoveContextData(ctx, field, "my_config2") // remove a non-exist key
30+
c.Delete(field, "my_config1")
31+
c.Delete(field, "my_config2") // remove a non-exist key
3032

31-
v = GetContextData(ctx, field, "my_config1")
33+
v, _ = c.Get(field, "my_config1")
3234
assert.Nil(t, v)
3335

3436
vInt, err := GetWithContextCache(ctx, field, "my_config1", func(context.Context, string) (int, error) {
@@ -37,42 +39,12 @@ func TestWithCacheContext(t *testing.T) {
3739
assert.NoError(t, err)
3840
assert.Equal(t, 1, vInt)
3941

40-
v = GetContextData(ctx, field, "my_config1")
42+
v, _ = c.Get(field, "my_config1")
4143
assert.EqualValues(t, 1, v)
4244

43-
now := timeNow
44-
defer func() {
45-
timeNow = now
46-
}()
47-
timeNow = func() time.Time {
48-
return now().Add(5 * time.Minute)
49-
}
50-
v = GetContextData(ctx, field, "my_config1")
51-
assert.Nil(t, v)
52-
}
53-
54-
func TestWithNoCacheContext(t *testing.T) {
55-
ctx := t.Context()
56-
57-
const field = "system_setting"
58-
59-
v := GetContextData(ctx, field, "my_config1")
60-
assert.Nil(t, v)
61-
SetContextData(ctx, field, "my_config1", 1)
62-
v = GetContextData(ctx, field, "my_config1")
63-
assert.Nil(t, v) // still no cache
64-
65-
ctx = WithCacheContext(ctx)
66-
v = GetContextData(ctx, field, "my_config1")
67-
assert.Nil(t, v)
68-
SetContextData(ctx, field, "my_config1", 1)
69-
v = GetContextData(ctx, field, "my_config1")
70-
assert.NotNil(t, v)
71-
72-
ctx = WithNoCacheContext(ctx)
73-
v = GetContextData(ctx, field, "my_config1")
45+
defer test.MockVariableValue(&timeNow, func() time.Time {
46+
return time.Now().Add(5 * time.Minute)
47+
})()
48+
v, _ = c.Get(field, "my_config1")
7449
assert.Nil(t, v)
75-
SetContextData(ctx, field, "my_config1", 1)
76-
v = GetContextData(ctx, field, "my_config1")
77-
assert.Nil(t, v) // still no cache
7850
}

0 commit comments

Comments
 (0)