Speed up HasUserStopwatch & GetActiveStopwatch

[zeripath]

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.

[lunny]

Just compared CI spent time with other PRs. One time is 27 minutes, another time is 35 minutes.
At the moment, the last two time spent is 24 minutes and 24 minutes. So I don't think there is an obvious time reduction from this PR.

[wxiaoguang]

Just compared CI spent time with other PRs. One time is 27 minutes, another time is 35 minutes. At the moment, the last two time spent is 24 minutes and 24 minutes. So I don't think there is an obvious time reduction from this PR.

I think those are different/independent problems.

For this PR, it could improve the all Gitea servers performance during daily usage.

For CI speed, it's not related to this PR.

• The times collected from CI jobs are inaccurate from one to another, and these times are heavily affected by other factors, for example, the server load when the CI runs, or the network, etc.
• If there is a plan to optimize the CI speed, IMO the first step is to have a well-measured benchmark. The second step is to find the bottle necks (the server load, the slowest tests, etc). Making changes is the last step.

[lunny]

Maybe I'm wrong to mention CI because you mentioned TestGit. If it's not for CI speed up, which page/feature is the purpose for?

[zeripath]

Maybe I'm wrong to mention CI because you mentioned TestGit. If it's not for CI speed up, which page/feature is the purpose for?

There should be a small CI improvement but most of my current PRs are going to be improvements in clock-time of the range of less than a minute. Our variance in CI is more than that.

My opening comment mentions CPU time - this is slightly different from clock-time - it's possible that there is no clock-time improvement as the bulk of time is IO time - however, reducing CPU time is still useful. If go is measuring something as CPU time on the cpuprofile then that may be cpu time the goroutine runner could be using elsewhere.

[zeripath]

• If there is a plan to optimize the CI speed, IMO the first step is to have a well-measured benchmark. The second step is to find the bottle necks (the server load, the slowest tests, etc). Making changes is the last step.

You've just mentioned the problems with coming up with a well-measured benchmark - there's too many interrelated factors and noise here that I don't think it's fundamentally possible. That's why I'm mostly stabbing in the dark here looking at things that are common based on clocktime and cputime. HasUserStopwatch repeatedly stood out on the cpuprofile for TestGit and given that's really hot code I think it's a good case for optimisation in any case.

Interpretting the FGProf clock time profile for TestGit is difficult but it appears heavily IO bound (unsurprising) so I'm just not certain what else we can do for that apart from just drop the big commit and/or not include the big commit in the merges etc.

[zeripath]

@lunny I just want to note that there is a slight change in this code - the use of INNER JOIN for repository and issue means that if there is an invalid stopwatch i.e. one that points to a non-existing issue or repository then this code won't see it.

My opinion is that this is fine for HasUserStopwatch but it is a change.

[lunny]

@lunny I just want to note that there is a slight change in this code - the use of INNER JOIN for repository and issue means that if there is an invalid stopwatch i.e. one that points to a non-existing issue or repository then this code won't see it.

My opinion is that this is fine for HasUserStopwatch but it is a change.

I think that's not the scope of this PR.

[lunny]

CI failure is related.

[zeripath]

CI failure is related.

Sorry, mistake when dealing with comments - the change induced overriding of the provided issue - have changed to use the name otherIssue instead.