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.

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

Generate new-comment-url once instead of n times for PRs #17167

Closed
delvh opened this issue Sep 27, 2021 · 2 comments · Fixed by #17618
Closed

Generate new-comment-url once instead of n times for PRs #17167

delvh opened this issue Sep 27, 2021 · 2 comments · Fixed by #17618
Labels
performance/speed performance issues with slow downs topic/ui Change the appearance of the Gitea UI

Comments

@delvh
Copy link
Member

delvh commented Sep 27, 2021 •
edited
Loading

Gitea Version

All up to 1.16.0+dev-293-ge8574f2f7

Operating System

All

Browser Version

Independent of Browser

Can you reproduce the bug on the Gitea demo site?

Yes

Description

Currently, the data-new-comment-url variable needed for the New review comment (+) button is calculated per button - see the code sample below that shows the code for one such button.

<a class="ui primary button add-code-comment add-code-comment-right" data-path="test2.txt" data-side="right" data-idx="3" data-new-comment-url="https://try.gitea.io/delvh/kanban-test/pulls/10/files/reviews/new_comment">
…
</a>

This, however, is unnecessary, as all these buttons point to exactly the same URL.
So, it would suffice to set this one time globally when rendering the template.
This would speed up diff generation, file sending, and also client-side processing time.
Especially large PRs benefit disproportionally from this. This is simply due to having a higher ratio of buttons compared to the rest of the page. This is even more true when those big PRs change lines instead of adding completely new lines/ deleting old lines, as then the button will be generated twice per line.

Benchmark

I've tested on one PR that had 3200 lines added and almost none changed.
On that PR, there were 4666 instances of this unnecessary declaration.
The size that was unnecessarily generated was about 435KB, or 6.5% of the whole size of the page (6.7MB).
Here, you can see that this simple fix can improve performance by up to 7%, which definitely is worth fixing.

Screenshots

See code sample above.
@wxiaoguang
Copy link
Contributor

Just a guess, rewrite in Vue will decrease the size significantly.

@delvh
Copy link
Member Author

delvh commented Sep 28, 2021

Most likely true as well.

@lafriks lafriks added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI and removed type/enhancement An improvement of existing functionality labels Sep 28, 2021
@delvh delvh added the performance/speed performance issues with slow downs label Sep 28, 2021
Gusted added a commit to Gusted/gitea that referenced this issue Nov 11, 2021
@Gusted
perf: only generate data-comment-url once
dc55f42 
- Resolves go-gitea#17167
- Don't generate the same `data-comment-url` over and over for each
edited LOC.
- Set `data-comment-url` for top-table of diff output.

Data for diff with 2.5K additons 0 edits 0 deletions:
```
No-patch:
Split: 2.82 MB
Unified: 2.37 MB

Patch:
Split: 2.59 MB (-8.2%)
Unified: 2.15 MB (-9.3%)
```
@Gusted Gusted mentioned this issue Nov 11, 2021
Merged
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
performance/speed performance issues with slow downs topic/ui Change the appearance of the Gitea UI
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

perf: only generate data-comment-url once Gusted/gitea
3 participants
@lafriks @wxiaoguang @delvh