Commit 48ff40a0 authored by Stan Hu's avatar Stan Hu

Improve diff performance by eliminating redundant checks for text blobs

On a merge request with over 1000 changed files, there were redundant
calls to blob_text_viewable?, which incurred about 7% of the time.

Improves #14775
parent 6ad514d0
...@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.11.0 (unreleased) v 8.11.0 (unreleased)
- Fix the title of the toggle dropdown button. !5515 (herminiotorres) - Fix the title of the toggle dropdown button. !5515 (herminiotorres)
- Improve diff performance by eliminating redundant checks for text blobs
- Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell) - Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell)
- Fix CI status icon link underline (ClemMakesApps) - Fix CI status icon link underline (ClemMakesApps)
- Cache the commit author in RequestStore to avoid extra lookups in PostReceive - Cache the commit author in RequestStore to avoid extra lookups in PostReceive
......
...@@ -13,7 +13,7 @@ module BlobHelper ...@@ -13,7 +13,7 @@ module BlobHelper
blob = project.repository.blob_at(ref, path) rescue nil blob = project.repository.blob_at(ref, path) rescue nil
return unless blob && blob_text_viewable?(blob) return unless blob
from_mr = options[:from_merge_request_id] from_mr = options[:from_merge_request_id]
link_opts = {} link_opts = {}
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
- if current_user - if current_user
.btn-group{ role: "group" } .btn-group{ role: "group" }
- if blob_text_viewable?(@blob)
= edit_blob_link = edit_blob_link
= replace_blob_link = replace_blob_link
= delete_blob_link = delete_blob_link
...@@ -12,7 +12,8 @@ ...@@ -12,7 +12,8 @@
- if editable_diff?(diff_file) - if editable_diff?(diff_file)
= edit_blob_link(@merge_request.source_project, = edit_blob_link(@merge_request.source_project,
@merge_request.source_branch, diff_file.new_path, @merge_request.source_branch, diff_file.new_path,
from_merge_request_id: @merge_request.id) from_merge_request_id: @merge_request.id,
skip_visible_check: true)
= view_file_btn(diff_commit.id, diff_file, project) = view_file_btn(diff_commit.id, diff_file, project)
......
require 'spec_helper' require 'spec_helper'
describe BlobHelper do describe BlobHelper do
include TreeHelper
let(:blob_name) { 'test.lisp' } let(:blob_name) { 'test.lisp' }
let(:no_context_content) { ":type \"assem\"))" } let(:no_context_content) { ":type \"assem\"))" }
let(:blob_content) { "(make-pathname :defaults name\n#{no_context_content}" } let(:blob_content) { "(make-pathname :defaults name\n#{no_context_content}" }
...@@ -65,4 +67,20 @@ describe BlobHelper do ...@@ -65,4 +67,20 @@ describe BlobHelper do
expect(sanitize_svg(blob).data).to eq(expected) expect(sanitize_svg(blob).data).to eq(expected)
end end
end end
describe "#edit_blob_link" do
let(:project) { create(:project) }
before do
allow(self).to receive(:current_user).and_return(double)
end
it 'verifies blob is text' do
expect(self).not_to receive(:blob_text_viewable?)
button = edit_blob_link(project, 'refs/heads/master', 'README.md')
expect(button).to start_with('<button')
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment