Improved large commit handling.
Created by: bladealslayer
Previously, only number of changed files mattered. Now, the number of lines to render in the diff are also taken into account.
A hard limit is set, above which diffs are not rendered and users are not allowed to override that. This prevents high server resource usage with huge commits.
Related to #1745 (closed), #2259 (closed)
In addition, handle large commits for MergeRequests and Compare controllers.
Also fixes a bug where diffs are loaded twice, if user goes directly to merge_requests/:id/diffs URL.
Created by: bladealslayer
The main motivation for this improvement is that:
- commits with small number of files changed, but huge number of lines are rendered (e.g. 50 files with 500 000 lines changed)
- when hiding diffs for large files, the backend still renders the diffs - they are just hidden by default from the page
Both of these cases can be quite CPU intensive on the server.
Some more discussion on the feedback entry: http://feedback.gitlab.com/forums/176466-general/suggestions/3787993-hide-files-with-many-changes-in-a-diff-to-improve-
By Administrator on 2013-09-07T12:49:16 (imported from GitLab project)
Created by: DenisBY
How to disable it or force to show the diff? I see that there should be:
+ If you still want to see the diff + = link_to "click this link", url_for(force_show_diff: true), class: "underlined_link"But I don't have this link.
By Administrator on 2013-09-23T14:49:41 (imported from GitLab project)
Created by: bladealslayer
@DenisBY, the link won't appear if the commit is "really big", which means more than 500 changed files or more than 10 000 lines to render (changed lines + context). This is a hard limit, that can't be overridden by the user (using the front-end), to prevent server load.
Such diffs can still be downloaded as patch or plain diff.
The link to force show the diff appears for "moderately sized" diffs, which means more than 100 changed files or more than 5000 lines of diff to render, but not more than the hard limits above.
So, basically, diffs are split in 3 categories - normal, "moderately big" and "really big".
The limits are set in https://github.com/gitlabhq/gitlabhq/blob/master/app/models/commit.rb#L13
By Administrator on 2013-09-23T17:30:59 (imported from GitLab project)
Created by: bladealslayer
@randx what do you think if implementation would ignore image files when calculating total line count in the diff? Image files would still count towards the number of files changed in the diff. That way, large image files won't prevent the diff from rendering.
So something like:
def self.diff_line_count(diffs) image_types = ['.png', '.jpg', '.jpeg', '.gif'] diffs.select{|d| not image_types.include?(File.extname(d.new_path || d.old_path))}.reduce(0){|sum, d| sum + d.diff.lines.count} endBy Administrator on 2013-09-23T20:33:37 (imported from GitLab project)
Created by: bladealslayer
Hmmm. The correct way to do this would be to ignore all but the text files. Looks like that other parts of the code use something like
Gitlab::Git::Blob.new(project.repository, @commit.id, @ref, diff.new_path).text?to determine if a file is text. However, inside theCommitmodel, I can't access neither the project, nor the repository. What would you suggest as the best way to filter out all non-text files, @randx?My suggestion in the comments above would work for images, but still won't cover when other binary files are in a commit.
By Administrator on 2013-09-29T19:03:54 (imported from GitLab project)
Created by: bladealslayer
@dosire For example here: https://github.com/gitlabhq/gitlabhq/blob/master/app/views/projects/commits/_diffs.html.haml#L40 and later in the same file there is
if file.text?.By Administrator on 2013-09-30T07:41:09 (imported from GitLab project)
Created by: dosire
@bladealslayer Indeed https://github.com/gitlabhq/gitlabhq/blob/master/app/views/projects/commits/_diffs.html.haml#L63 seems to be it. The best thing would be if someone creates a pull request to use .text? instead of the current filter for images.
By Administrator on 2013-09-30T08:43:49 (imported from GitLab project)
Created by: bladealslayer
@dosire as I wrote above, the problem is that the
Commitmodel, where we have the commit size calculation logic, doesn't have access to project/repository, which seems needed to have aGitlab::Git::Blobobject.By Administrator on 2013-09-30T10:29:35 (imported from GitLab project)
