Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve dashboard's repo list performance #18963

Merged
merged 18 commits into from
Apr 26, 2022

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Mar 1, 2022

  • Avoid "a few" database lookups for each repo that's being requested, only return minimal information which doesn't require a database operation for each repo. This is general change for the repo API, so other use-cases of this will also benefit from it.
  • Makes fetching these list faster.
  • Less CPU overhead when a user visits home page.
  • Unintentionally removed some jQuery code.

- Avoid a lot of database lookups for all the repo's, by adding a
undocumented "minimal" mode for this specific task, which returns the
data that's only needed by this list which doesn't require any database
lookups.
- Makes fetching these list faster.
- Less CPU overhead when a user visits home page.
@Gusted Gusted added this to the 1.17.0 milestone Mar 1, 2022
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 1, 2022
@lunny
Copy link
Member

lunny commented Mar 2, 2022

I would like we split api/v1 and UI API to make the things simple. This is a chance to make a new route but not reuse the old one.

@Gusted
Copy link
Contributor Author

Gusted commented Mar 2, 2022

I would like we split api/v1 and UI API to make the things simple. This is a chance to make a new route but not reuse the old one.

Sounds like a good idea to me, which route do you propose for it? I guess api/internal/... or api/ui/...?

Gusted added 2 commits March 2, 2022 12:10
- Use async in the function so we can use `await`.
- Remove `archivedFilter` check for count, as it doesn't make sense to
  show the count of repos when you can't even see them(as they are
  filited away).
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 2, 2022
@6543
Copy link
Member

6543 commented Mar 3, 2022

... -> #16052

@6543
Copy link
Member

6543 commented Mar 14, 2022

I'm for adding a new ui route - we can migrate one func at a time ... just start now

(UI routes dont have the issue to need to be/stay backwards compatible)

@Gusted
Copy link
Contributor Author

Gusted commented Mar 14, 2022

just start now

I don't think that's possible, #16052 adds many new things and as well it's own file for the new API, it will result in conflict files for that PR if I try remotely to migrate it to a new route.

@6543
Copy link
Member

6543 commented Apr 7, 2022

#19318 got merged ... please ajust this pull @Gusted

@Gusted
Copy link
Contributor Author

Gusted commented Apr 11, 2022

@6543 Updated it.

@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2022

Codecov Report

Merging #18963 (7319f8d) into main (deffe9e) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main   #18963   +/-   ##
=======================================
  Coverage   47.37%   47.37%           
=======================================
  Files         949      949           
  Lines      132101   132102    +1     
=======================================
+ Hits        62583    62588    +5     
+ Misses      61987    61982    -5     
- Partials     7531     7532    +1     
Impacted Files Coverage Δ
routers/api/v1/repo/repo.go 66.55% <ø> (ø)
routers/web/repo/repo.go 24.37% <0.00%> (-0.06%) ⬇️
modules/process/manager_exec.go 85.36% <0.00%> (-7.32%) ⬇️
models/unit/unit.go 46.08% <0.00%> (-1.74%) ⬇️
models/repo_list.go 76.15% <0.00%> (+0.48%) ⬆️
modules/queue/workerpool.go 53.62% <0.00%> (+2.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update deffe9e...7319f8d. Read the comment docs.

routers/web/repo/repo.go Outdated Show resolved Hide resolved
routers/web/repo/repo.go Outdated Show resolved Hide resolved
routers/web/repo/repo.go Outdated Show resolved Hide resolved
web_src/js/components/DashboardRepoList.js Outdated Show resolved Hide resolved
web_src/js/components/DashboardRepoList.js Outdated Show resolved Hide resolved
Gusted and others added 2 commits April 14, 2022 16:37
routers/web/repo/repo.go Outdated Show resolved Hide resolved
web_src/js/components/DashboardRepoList.js Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 26, 2022
Co-authored-by: wxiaoguang <[email protected]>
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI seems down .... wait for the pass

@zeripath
Copy link
Contributor

make lgtm work

@techknowlogick techknowlogick merged commit 076eaad into go-gitea:main Apr 26, 2022
@Gusted Gusted deleted the improve-dashboard-list branch April 26, 2022 20:35
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 28, 2022
* giteaofficial/main: (21 commits)
  Prevent intermittent race in attribute reader close (go-gitea#19537)
  Make repository file list useable on mobile (go-gitea#19515)
  Update image URL for Discord webhook (go-gitea#19536)
  [skip ci] Updated translations via Crowdin
  Fix 64-bit atomic operations on 32-bit machines (go-gitea#19531)
  Fix `upgrade.sh` script error with `su -c` (go-gitea#19483)
  When view _Siderbar or _Footer, just display once (go-gitea#19501)
  Fix migrate release from github (go-gitea#19510)
  Prevent dangling archiver goroutine (go-gitea#19516)
  Don't let repo clone URL overflow (go-gitea#19517)
  Add commit status popup to issuelist (go-gitea#19375)
  Disable unnecessary GitHooks elements (go-gitea#18485)
  Disable unnecessary GitHooks elements
  Improve dashboard's repo list performance (go-gitea#18963)
  By default force vertical tabs on mobile (go-gitea#19486)
  Refactor readme file renderer (go-gitea#19502)
  Allow package dump skipping (go-gitea#19506)
  Unset git author/committer variables when running integration tests (go-gitea#19512)
  Allow commit status popup on /pulls page (go-gitea#19507)
  Use router param for filepath in GetRawFile (go-gitea#19499)
  ...
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
* Improve dashboard's repo list performance

- Avoid a lot of database lookups for all the repo's, by adding a
undocumented "minimal" mode for this specific task, which returns the
data that's only needed by this list which doesn't require any database
lookups.
- Makes fetching these list faster.
- Less CPU overhead when a user visits home page.

* Refactor javascript code + fix Fork icon

- Use async in the function so we can use `await`.
- Remove `archivedFilter` check for count, as it doesn't make sense to
  show the count of repos when you can't even see them(as they are
  filited away).

* Add `count_only`

* Remove uncessary code

* Improve comment

Co-authored-by: delvh <[email protected]>

* Update web_src/js/components/DashboardRepoList.js

Co-authored-by: delvh <[email protected]>

* Update web_src/js/components/DashboardRepoList.js

Co-authored-by: delvh <[email protected]>

* By default apply minimal mode

* Remove `minimal` paramater

* Refactor count header

* Simplify init

Co-authored-by: wxiaoguang <[email protected]>

Co-authored-by: delvh <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
Co-authored-by: zeripath <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. performance/cpu
Projects
None yet
Development

Successfully merging this pull request may close these issues.