Skip to content

Performance improvements for pull request list API #30490

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
May 31, 2024
Merged

Performance improvements for pull request list API #30490

merged 13 commits into from
May 31, 2024

Conversation

lunny
Copy link
Member

@lunny lunny commented Apr 15, 2024

Fix #30483

@lunny lunny added modifies/api This PR adds API routes or modifies them performance/speed performance issues with slow downs labels Apr 15, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 15, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Apr 15, 2024
@lunny lunny added this to the 1.23.0 milestone Apr 15, 2024
@yp05327
Copy link
Contributor

yp05327 commented Apr 16, 2024

Does web side have same issue?

@lunny
Copy link
Member Author

lunny commented Apr 16, 2024

Does web side have same issue?

I don't think web router of issue/pulls has the same issue.

@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 Apr 29, 2024
yp05327
yp05327 reviewed Apr 30, 2024
View reviewed changes
yp05327
yp05327 reviewed May 8, 2024
View reviewed changes
yp05327
yp05327 reviewed May 8, 2024
View reviewed changes
@yp05327
Copy link
Contributor

yp05327 commented May 8, 2024

image
LoadLabels can be moved to L64-L66?

yp05327
yp05327 reviewed May 8, 2024
View reviewed changes
@lunny lunny force-pushed the lunny/performance_api_pull_request_list branch from b0535ca to f4ad036 Compare May 28, 2024 03:10
@github-actions github-actions bot removed modifies/translation modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/templates This PR modifies the template files modifies/docs modifies/migrations modifies/internal modifies/js modifies/dependencies labels May 28, 2024
@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 May 31, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 31, 2024
@silverwind silverwind enabled auto-merge (squash) May 31, 2024 11:44
@silverwind silverwind merged commit 352a2ca into go-gitea:main May 31, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 31, 2024
@lunny lunny deleted the lunny/performance_api_pull_request_list branch May 31, 2024 13:10
@MaxWipfli MaxWipfli mentioned this pull request Jun 1, 2024
Closed
@lunny lunny mentioned this pull request Jun 2, 2024
Merged
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 3, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
d0022eb 
* giteaofficial/main:
  [skip ci] Updated licenses and gitignores
  Fix overflow in issue card (go-gitea#31203)
  Fix agit checkout command line hint & fix ShowMergeInstructions checking (go-gitea#31219)
  Fix the possible migration failure on 286 with postgres 16 (go-gitea#31209)
  Only update poster in issue/comment list if it has been loaded (go-gitea#31216)
  Return an empty string when a repo has no avatar in the repo API (go-gitea#31187)
  Split sanitizer functions and fine-tune some tests (go-gitea#31192)
  Performance improvements for pull request list API (go-gitea#30490)
  Fix URL In Gitea Actions Badge Docs (go-gitea#31191)
@yp05327 yp05327 mentioned this pull request Jun 13, 2024
Merged
lunny pushed a commit that referenced this pull request Jun 13, 2024
@yp05327
Fix PullRequestList.GetIssueIDs's logic (#31352)
e61e9a3 
fix a bug from #30490

`prs.GetIssueIDs()` will also be used in other places, e.g.
`InvalidateCodeComments`
so we should not add `if pr.Issue == nil` in it, or if `pr.Issue` is
already loaded, you will not get the issueID in the results list and
this is not an expected result.

So this will caused a bug:
before calling `InvalidateCodeComments`, all `pr.Issues` in `prs` are
loaded, so `issueIDs` in this function will always be `[]`.

![image](https://github.com/go-gitea/gitea/assets/18380374/ef94d9d2-0bf9-455a-abd6-4d5e6497db7c)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@silverwind silverwind silverwind approved these changes

@yp05327 yp05327 yp05327 approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code performance/speed performance issues with slow downs
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

Performance issue on pulls list API endpoint with lots of PR's
4 participants
@lunny @yp05327 @silverwind @GiteaBot