It takes a long time to open a big pull request page #14734
Comments
|
How many files and lines changed on this PR? |
|
An example https://gitea.com/gitea/tea/pulls/316/files I think you should put a limit to the amount of content displayed and if the user requires more by clicking on a button to load more content. |
|
@lunny |
|
I agree that the pull request page can be very hard to use, even more when dealing with many files. One solution would be to show a tree of the modified files and show the diff when clicking on those files. We had this on bitbucket and that was really better than to have everything on the same page. |
|
Same problem here. Setting DISABLE_DIFF_HIGHLIGHT = true helps, but only gets you so far. The HTML generated for a pull request is highly complicated and gets huge quickly. A new, simplified mode for this view would be useful. |
|
In my opinion the ideal solution would be to have a file tree on the pull request page, and you would click to select one file diff at a time. |
|
Hello, to follow up on my last message about the pull request page, I started a project built on AspNet Core (5.0, but might work easily on 3.1 or even 2.1 as the backend is rather simple, no database required, everything is done via API) and VueJS for the front end. It uses the Gitea API to provide :
I'm planning to release it as open source when I think it's less buggy, but a few members of my team already use it (I do everyday). Here is what it loooks like : |
|
Yes this is a known problem. I know how to solve it but I still haven't been able to get round to it. We need to make the diff page in sections and farm parts of it off to Ajax and other requests. |
|
I have seen this issue too. I sampled the stack trace when my Gitea was hanging on showing pull diffs and got the following:
|
|
Looking at https://github.com/alecthomas/chroma/blob/7e282be4957c2cb4edeb4562f56d707427393192/lexers/internal/api.go#L91-L98, I am afraid it's noticeably expensive to search for a lexer for every line of code. See also gitea/services/gitdiff/gitdiff.go Line 551 in f4d3bf7 |
|
Infinite scrolling pagination (https://www.digitalocean.com/community/tutorials/vuejs-implementing-infinite-scroll) might be a relatively simple way to address this if it doesn't require redesigning the UI. |
I guess that shows where we can seriously improve then... The lexer detection should be run on the old file and run on the new file once and then reused. |
|
@zeripath |
|
Yeah |
|
Agh I just read that pr. We need to change it. I don't think we can use arccache - it should be twoqueue I think |
Can you elaborate on their differences in this use case? |
|
the issue is ARCCache is patent encumbered and known to be patent encumbered. We can't/shouldn't use that until the ARCCache patents expire - which it's potentially not until 2024 although someone says it's 2022. |
|
We should use a TwoQueue as the documentation for that library suggests which is equivalent but without the patent problem. |
|
@zeripath I see. I'll submit a pr later. |
|
This (sadly) makes Gitea more or less unusable for managing forks of large projects. We have forked a large open source project with a very active upstream so merging in new versions will easily pull in 15k+ new commits and 50k changed files. Even trying to create a pull request via the web view hangs forever, commenting on it is pretty much impossible too :/ Gitea Version: 1.15.6 |
I'm interested in what's the open source project. |
Firefox, I merged in all changes between version 80 and 84 into our fork and the pull request page loads forever. |
|
As a point of reference, I opened it on the Github Upstream repo. This took like 1s to load compared with the infinite loading time with Gitea. |
|
Yeah, because github will hide the changes. If you cannot view the changes, the PR cannot be reviewed manually. |
I completely agree that it is impossible to review change sets that large in the web UI. If the UI doesn't render at all however, commenting on the pull request becomes impossible too as the comment box is rendered after all commits have loaded. This makes giving feedback based on checking out the PR locally via Gitea kinda impossible :) I'm well aware that this is an edge case, but the option to hide the commits/changes similar to Github, if the change set is too large, would be a super helpful addition to Gitea :) |
|
OKay. So we can add an option to hide the file changes in the web UI but allow to create the PR successfully. |
|
That would be fantastic! :) |
|
Similarly to the idea of having a tree of files that can be clicked to show the diff for one file, it would also be very useful to be able to select only one or more commits of the PR, which would also reduce the amount of diff displayed. This is the "Changes from" menu in Github: |
|
AHA! I bet you have some broken git repositories! I've just discovered that |
…and other fixes (#17991) This PR contains multiple fixes. The most important of which is: * Prevent hang in git cat-file if the repository is not a valid repository Unfortunately it appears that if git cat-file is run in an invalid repository it will hang until stdin is closed. This will result in deadlocked /pulls pages and dangling git cat-file calls if a broken repository is tried to be reviewed or pulls exists for a broken repository. Fix #14734 Fix #9271 Fix #16113 Otherwise there are a few small other fixes included which this PR was initially intending to fix: * Fix panic on partial compares due to missing PullRequestWorkInProgressPrefixes * Fix links on pulls pages due to regression from #17551 - by making most /issues routes match /pulls too - Fix #17983 * Fix links on feeds pages due to another regression from #17551 but also fix issue with syncing tags - Fix #17943 * Add missing locale entries for oauth group claims * Prevent NPEs if ColorFormat is called on nil users, repos or teams.
…and other fixes (go-gitea#17991) This PR contains multiple fixes. The most important of which is: * Prevent hang in git cat-file if the repository is not a valid repository Unfortunately it appears that if git cat-file is run in an invalid repository it will hang until stdin is closed. This will result in deadlocked /pulls pages and dangling git cat-file calls if a broken repository is tried to be reviewed or pulls exists for a broken repository. Fix go-gitea#14734 Fix go-gitea#9271 Fix go-gitea#16113 Otherwise there are a few small other fixes included which this PR was initially intending to fix: * Fix panic on partial compares due to missing PullRequestWorkInProgressPrefixes * Fix links on pulls pages due to regression from go-gitea#17551 - by making most /issues routes match /pulls too - Fix go-gitea#17983 * Fix links on feeds pages due to another regression from go-gitea#17551 but also fix issue with syncing tags - Fix go-gitea#17943 * Add missing locale entries for oauth group claims * Prevent NPEs if ColorFormat is called on nil users, repos or teams.
|
FYI for readers: These settings have the most impact on the performance of the files changed page. play around with them and figure out what works for you. I still think Bitbucket Server always had a really great concept for loading diffs, since it only ever had a single file open and you could just click in the file picker to the next one. Rarely did I want to scroll through multiple files, since when are files vertically aligned to each other, they are always in a file system not a vertical line. Is there lexer problem still relevant? Does it still look into the lexer for each line? I might want to have a look into that if so. |
|
An alternative Idea could be building a Diff Viewer completely separate from the current one and implementing it purely in vuejs. Syntax Highlighting has to be figured out then, but I think overall it could lead to a more customizable experience than the current server-client hybrid. Although I really wish gitea could support vuejs SSR somehow, I'm not familiar enough with vuejs to judge if that's easy enough to implement. |
|
That would be a great hotfix to allow to show only one file at a time. I imagine it would be easy to implement and a very stable fallback mechanism until infinite scrolling etc stabilized. (reviewing large PRs got better recently but it is still very annoying) Can someone outline what would have to be done for that? |
|
I think #16829 has resolve this problem. You can set to limit the files loaded every time. And click |
|
Closed since loading files could be configured and there is a |
|
I tried this with the current deployment on try: https://try.gitea.io/thigg/podqast/pulls/1/files @lunny should this be a separate/new issue? I agree that you should not have PRs this big... but if you get into the situation for some reason, it would be nice if at least gitea would not get in your way too. |
Gitea version 1.14.0+dev-449-ge0c753e77 built with GNU Make 4.3, go1.15.6 : bindata, timetzdata, sqlite, sqlite_unlock_notify
The gitea server is deployed locally. I made a pull request and changed more than 100 files and more than 1,000 lines. It took me nearly two minutes to open this pull request page.
I looked at the log, mainly because these two API return very slowly, both of which are about one minute
Are there any configuration items that can improve the speed here?
The text was updated successfully, but these errors were encountered: