Skip to content

Commit ef4fc30

Browse files
zeripathKN4CK3Rlunnydelvh
authored
Speed up HasUserStopwatch & GetActiveStopwatch (#23051)
GetActiveStopwatch & HasUserStopwatch is a hot piece of code that is repeatedly called and on examination of the cpu profile for TestGit it represents 0.44 seconds of CPU time. This PR reduces this time to 80ms. --------- Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: KN4CK3R <[email protected]> Co-authored-by: Lunny Xiao <[email protected]> Co-authored-by: delvh <[email protected]>
1 parent 0e7bec1 commit ef4fc30

File tree

4 files changed

+28
-39
lines changed

4 files changed

+28
-39
lines changed

models/issues/stopwatch.go

+22-12
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"code.gitea.io/gitea/models/db"
12+
"code.gitea.io/gitea/models/repo"
1213
user_model "code.gitea.io/gitea/models/user"
1314
"code.gitea.io/gitea/modules/timeutil"
1415
"code.gitea.io/gitea/modules/util"
@@ -132,12 +133,26 @@ func StopwatchExists(userID, issueID int64) bool {
132133
}
133134

134135
// HasUserStopwatch returns true if the user has a stopwatch
135-
func HasUserStopwatch(ctx context.Context, userID int64) (exists bool, sw *Stopwatch, err error) {
136-
sw = new(Stopwatch)
136+
func HasUserStopwatch(ctx context.Context, userID int64) (exists bool, sw *Stopwatch, issue *Issue, err error) {
137+
type stopwatchIssueRepo struct {
138+
Stopwatch `xorm:"extends"`
139+
Issue `xorm:"extends"`
140+
repo.Repository `xorm:"extends"`
141+
}
142+
143+
swIR := new(stopwatchIssueRepo)
137144
exists, err = db.GetEngine(ctx).
145+
Table("stopwatch").
138146
Where("user_id = ?", userID).
139-
Get(sw)
140-
return exists, sw, err
147+
Join("INNER", "issue", "issue.id = stopwatch.issue_id").
148+
Join("INNER", "repository", "repository.id = issue.repo_id").
149+
Get(swIR)
150+
if exists {
151+
sw = &swIR.Stopwatch
152+
issue = &swIR.Issue
153+
issue.Repo = &swIR.Repository
154+
}
155+
return exists, sw, issue, err
141156
}
142157

143158
// FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore
@@ -217,23 +232,18 @@ func CreateIssueStopwatch(ctx context.Context, user *user_model.User, issue *Iss
217232
}
218233

219234
// if another stopwatch is running: stop it
220-
exists, sw, err := HasUserStopwatch(ctx, user.ID)
235+
exists, _, otherIssue, err := HasUserStopwatch(ctx, user.ID)
221236
if err != nil {
222237
return err
223238
}
224239
if exists {
225-
issue, err := GetIssueByID(ctx, sw.IssueID)
226-
if err != nil {
227-
return err
228-
}
229-
230-
if err := FinishIssueStopwatch(ctx, user, issue); err != nil {
240+
if err := FinishIssueStopwatch(ctx, user, otherIssue); err != nil {
231241
return err
232242
}
233243
}
234244

235245
// Create stopwatch
236-
sw = &Stopwatch{
246+
sw := &Stopwatch{
237247
UserID: user.ID,
238248
IssueID: issue.ID,
239249
}

models/issues/stopwatch_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ func TestStopwatchExists(t *testing.T) {
4545
func TestHasUserStopwatch(t *testing.T) {
4646
assert.NoError(t, unittest.PrepareTestDatabase())
4747

48-
exists, sw, err := issues_model.HasUserStopwatch(db.DefaultContext, 1)
48+
exists, sw, _, err := issues_model.HasUserStopwatch(db.DefaultContext, 1)
4949
assert.NoError(t, err)
5050
assert.True(t, exists)
5151
assert.Equal(t, int64(1), sw.ID)
5252

53-
exists, _, err = issues_model.HasUserStopwatch(db.DefaultContext, 3)
53+
exists, _, _, err = issues_model.HasUserStopwatch(db.DefaultContext, 3)
5454
assert.NoError(t, err)
5555
assert.False(t, exists)
5656
}

routers/web/repo/issue.go

+3-12
Original file line numberDiff line numberDiff line change
@@ -1432,25 +1432,16 @@ func ViewIssue(ctx *context.Context) {
14321432
ctx.Data["IsStopwatchRunning"] = issues_model.StopwatchExists(ctx.Doer.ID, issue.ID)
14331433
if !ctx.Data["IsStopwatchRunning"].(bool) {
14341434
var exists bool
1435-
var sw *issues_model.Stopwatch
1436-
if exists, sw, err = issues_model.HasUserStopwatch(ctx, ctx.Doer.ID); err != nil {
1435+
var swIssue *issues_model.Issue
1436+
if exists, _, swIssue, err = issues_model.HasUserStopwatch(ctx, ctx.Doer.ID); err != nil {
14371437
ctx.ServerError("HasUserStopwatch", err)
14381438
return
14391439
}
14401440
ctx.Data["HasUserStopwatch"] = exists
14411441
if exists {
14421442
// Add warning if the user has already a stopwatch
1443-
var otherIssue *issues_model.Issue
1444-
if otherIssue, err = issues_model.GetIssueByID(ctx, sw.IssueID); err != nil {
1445-
ctx.ServerError("GetIssueByID", err)
1446-
return
1447-
}
1448-
if err = otherIssue.LoadRepo(ctx); err != nil {
1449-
ctx.ServerError("LoadRepo", err)
1450-
return
1451-
}
14521443
// Add link to the issue of the already running stopwatch
1453-
ctx.Data["OtherStopwatchURL"] = otherIssue.Link()
1444+
ctx.Data["OtherStopwatchURL"] = swIssue.Link()
14541445
}
14551446
}
14561447
ctx.Data["CanUseTimetracker"] = ctx.Repo.CanUseTimetracker(issue, ctx.Doer)

routers/web/repo/issue_stopwatch.go

+1-13
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func GetActiveStopwatch(ctx *context.Context) {
8686
return
8787
}
8888

89-
_, sw, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID)
89+
_, sw, issue, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID)
9090
if err != nil {
9191
ctx.ServerError("HasUserStopwatch", err)
9292
return
@@ -96,18 +96,6 @@ func GetActiveStopwatch(ctx *context.Context) {
9696
return
9797
}
9898

99-
issue, err := issues_model.GetIssueByID(ctx, sw.IssueID)
100-
if err != nil || issue == nil {
101-
if !issues_model.IsErrIssueNotExist(err) {
102-
ctx.ServerError("GetIssueByID", err)
103-
}
104-
return
105-
}
106-
if err = issue.LoadRepo(ctx); err != nil {
107-
ctx.ServerError("LoadRepo", err)
108-
return
109-
}
110-
11199
ctx.Data["ActiveStopwatch"] = StopwatchTmplInfo{
112100
issue.Link(),
113101
issue.Repo.FullName(),

0 commit comments

Comments
 (0)