From ef4fc302468cc8a9fd8f65c4ebdc6f55138450d1 Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 27 Feb 2023 18:46:00 +0000 Subject: [PATCH] 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 Co-authored-by: KN4CK3R Co-authored-by: Lunny Xiao Co-authored-by: delvh --- models/issues/stopwatch.go | 34 +++++++++++++++++++---------- models/issues/stopwatch_test.go | 4 ++-- routers/web/repo/issue.go | 15 +++---------- routers/web/repo/issue_stopwatch.go | 14 +----------- 4 files changed, 28 insertions(+), 39 deletions(-) diff --git a/models/issues/stopwatch.go b/models/issues/stopwatch.go index 6bf936c5d4..c8cd5ad33f 100644 --- a/models/issues/stopwatch.go +++ b/models/issues/stopwatch.go @@ -9,6 +9,7 @@ import ( "time" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -132,12 +133,26 @@ func StopwatchExists(userID, issueID int64) bool { } // HasUserStopwatch returns true if the user has a stopwatch -func HasUserStopwatch(ctx context.Context, userID int64) (exists bool, sw *Stopwatch, err error) { - sw = new(Stopwatch) +func HasUserStopwatch(ctx context.Context, userID int64) (exists bool, sw *Stopwatch, issue *Issue, err error) { + type stopwatchIssueRepo struct { + Stopwatch `xorm:"extends"` + Issue `xorm:"extends"` + repo.Repository `xorm:"extends"` + } + + swIR := new(stopwatchIssueRepo) exists, err = db.GetEngine(ctx). + Table("stopwatch"). Where("user_id = ?", userID). - Get(sw) - return exists, sw, err + Join("INNER", "issue", "issue.id = stopwatch.issue_id"). + Join("INNER", "repository", "repository.id = issue.repo_id"). + Get(swIR) + if exists { + sw = &swIR.Stopwatch + issue = &swIR.Issue + issue.Repo = &swIR.Repository + } + return exists, sw, issue, err } // FinishIssueStopwatchIfPossible if stopwatch exist then finish it otherwise ignore @@ -217,23 +232,18 @@ func CreateIssueStopwatch(ctx context.Context, user *user_model.User, issue *Iss } // if another stopwatch is running: stop it - exists, sw, err := HasUserStopwatch(ctx, user.ID) + exists, _, otherIssue, err := HasUserStopwatch(ctx, user.ID) if err != nil { return err } if exists { - issue, err := GetIssueByID(ctx, sw.IssueID) - if err != nil { - return err - } - - if err := FinishIssueStopwatch(ctx, user, issue); err != nil { + if err := FinishIssueStopwatch(ctx, user, otherIssue); err != nil { return err } } // Create stopwatch - sw = &Stopwatch{ + sw := &Stopwatch{ UserID: user.ID, IssueID: issue.ID, } diff --git a/models/issues/stopwatch_test.go b/models/issues/stopwatch_test.go index ec2778aa81..ea3827a1f6 100644 --- a/models/issues/stopwatch_test.go +++ b/models/issues/stopwatch_test.go @@ -45,12 +45,12 @@ func TestStopwatchExists(t *testing.T) { func TestHasUserStopwatch(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - exists, sw, err := issues_model.HasUserStopwatch(db.DefaultContext, 1) + exists, sw, _, err := issues_model.HasUserStopwatch(db.DefaultContext, 1) assert.NoError(t, err) assert.True(t, exists) assert.Equal(t, int64(1), sw.ID) - exists, _, err = issues_model.HasUserStopwatch(db.DefaultContext, 3) + exists, _, _, err = issues_model.HasUserStopwatch(db.DefaultContext, 3) assert.NoError(t, err) assert.False(t, exists) } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 745d6e70a0..e4f9400615 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1432,25 +1432,16 @@ func ViewIssue(ctx *context.Context) { ctx.Data["IsStopwatchRunning"] = issues_model.StopwatchExists(ctx.Doer.ID, issue.ID) if !ctx.Data["IsStopwatchRunning"].(bool) { var exists bool - var sw *issues_model.Stopwatch - if exists, sw, err = issues_model.HasUserStopwatch(ctx, ctx.Doer.ID); err != nil { + var swIssue *issues_model.Issue + if exists, _, swIssue, err = issues_model.HasUserStopwatch(ctx, ctx.Doer.ID); err != nil { ctx.ServerError("HasUserStopwatch", err) return } ctx.Data["HasUserStopwatch"] = exists if exists { // Add warning if the user has already a stopwatch - var otherIssue *issues_model.Issue - if otherIssue, err = issues_model.GetIssueByID(ctx, sw.IssueID); err != nil { - ctx.ServerError("GetIssueByID", err) - return - } - if err = otherIssue.LoadRepo(ctx); err != nil { - ctx.ServerError("LoadRepo", err) - return - } // Add link to the issue of the already running stopwatch - ctx.Data["OtherStopwatchURL"] = otherIssue.Link() + ctx.Data["OtherStopwatchURL"] = swIssue.Link() } } ctx.Data["CanUseTimetracker"] = ctx.Repo.CanUseTimetracker(issue, ctx.Doer) diff --git a/routers/web/repo/issue_stopwatch.go b/routers/web/repo/issue_stopwatch.go index 3d20b08b49..3e715437e6 100644 --- a/routers/web/repo/issue_stopwatch.go +++ b/routers/web/repo/issue_stopwatch.go @@ -86,7 +86,7 @@ func GetActiveStopwatch(ctx *context.Context) { return } - _, sw, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID) + _, sw, issue, err := issues_model.HasUserStopwatch(ctx, ctx.Doer.ID) if err != nil { ctx.ServerError("HasUserStopwatch", err) return @@ -96,18 +96,6 @@ func GetActiveStopwatch(ctx *context.Context) { return } - issue, err := issues_model.GetIssueByID(ctx, sw.IssueID) - if err != nil || issue == nil { - if !issues_model.IsErrIssueNotExist(err) { - ctx.ServerError("GetIssueByID", err) - } - return - } - if err = issue.LoadRepo(ctx); err != nil { - ctx.ServerError("LoadRepo", err) - return - } - ctx.Data["ActiveStopwatch"] = StopwatchTmplInfo{ issue.Link(), issue.Repo.FullName(),