Improve performance of tree rendering in repositories with lots of items
Rails is slow to generate paths dynamically especially when called hundreds/thousands of times. Also, rendering many partials hundreds of times can be slower. This change reduces the number of partials rendered and introduces two fast path methods to speed up path generation.
This commit is contained in:
parent
43463869c8
commit
409f2f4dd2
|
|
@ -109,6 +109,8 @@ module IconsHelper
|
|||
def file_type_icon_class(type, mode, name)
|
||||
if type == 'folder'
|
||||
icon_class = 'folder'
|
||||
elsif type == 'archive'
|
||||
icon_class = 'archive'
|
||||
elsif mode == '120000'
|
||||
icon_class = 'share'
|
||||
else
|
||||
|
|
|
|||
|
|
@ -31,11 +31,21 @@ module TreeHelper
|
|||
# mode - File unix mode
|
||||
# name - File name
|
||||
def tree_icon(type, mode, name)
|
||||
icon("#{file_type_icon_class(type, mode, name)} fw")
|
||||
icon([file_type_icon_class(type, mode, name), 'fw'])
|
||||
end
|
||||
|
||||
def tree_hex_class(content)
|
||||
"file_#{hexdigest(content.name)}"
|
||||
# Using Rails `*_path` methods can be slow, especially when generating
|
||||
# many paths, as with a repository tree that has thousands of items.
|
||||
def fast_project_blob_path(project, blob_path)
|
||||
Addressable::URI.escape(
|
||||
File.join(relative_url_root, project.path_with_namespace, 'blob', blob_path)
|
||||
)
|
||||
end
|
||||
|
||||
def fast_project_tree_path(project, tree_path)
|
||||
Addressable::URI.escape(
|
||||
File.join(relative_url_root, project.path_with_namespace, 'tree', tree_path)
|
||||
)
|
||||
end
|
||||
|
||||
# Simple shortcut to File.join
|
||||
|
|
@ -142,4 +152,8 @@ module TreeHelper
|
|||
def selected_branch
|
||||
@branch_name || tree_edit_branch
|
||||
end
|
||||
|
||||
def relative_url_root
|
||||
Gitlab.config.gitlab.relative_url_root.presence || '/'
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -1,12 +0,0 @@
|
|||
- is_lfs_blob = @lfs_blob_ids.include?(blob_item.id)
|
||||
%tr{ class: "tree-item #{tree_hex_class(blob_item)}" }
|
||||
%td.tree-item-file-name
|
||||
= tree_icon(type, blob_item.mode, blob_item.name)
|
||||
- file_name = blob_item.name
|
||||
= link_to project_blob_path(@project, tree_join(@id || @commit.id, blob_item.name)), class: 'str-truncated', title: file_name do
|
||||
%span= file_name
|
||||
- if is_lfs_blob
|
||||
%span.badge.label-lfs.prepend-left-5 LFS
|
||||
%td.d-none.d-sm-table-cell.tree-commit
|
||||
%td.tree-time-ago.cgray.text-right
|
||||
= render 'projects/tree/spinner'
|
||||
|
|
@ -1,3 +0,0 @@
|
|||
%span.log_loading.hide
|
||||
%i.fa.fa-spinner.fa-spin
|
||||
Loading commit data...
|
||||
|
|
@ -1,6 +0,0 @@
|
|||
%tr.tree-item
|
||||
%td.tree-item-file-name
|
||||
%i.fa.fa-archive.fa-fw
|
||||
= submodule_link(submodule_item, @ref)
|
||||
%td
|
||||
%td.d-none.d-sm-table-cell
|
||||
|
|
@ -1,9 +0,0 @@
|
|||
%tr{ class: "tree-item #{tree_hex_class(tree_item)}" }
|
||||
%td.tree-item-file-name
|
||||
= tree_icon(type, tree_item.mode, tree_item.name)
|
||||
- path = flatten_tree(@path, tree_item)
|
||||
= link_to project_tree_path(@project, tree_join(@id || @commit.id, path)), class: 'str-truncated', title: path do
|
||||
%span= path
|
||||
%td.d-none.d-sm-table-cell.tree-commit
|
||||
%td.tree-time-ago.text-right
|
||||
= render 'projects/tree/spinner'
|
||||
|
|
@ -1,6 +1,27 @@
|
|||
- if tree_row.type == :tree
|
||||
= render partial: 'projects/tree/tree_item', object: tree_row, as: 'tree_item', locals: { type: 'folder' }
|
||||
- elsif tree_row.type == :blob
|
||||
= render partial: 'projects/tree/blob_item', object: tree_row, as: 'blob_item', locals: { type: 'file' }
|
||||
- elsif tree_row.type == :commit
|
||||
= render partial: 'projects/tree/submodule_item', object: tree_row, as: 'submodule_item'
|
||||
- tree_row_name = tree_row.name
|
||||
- tree_row_type = tree_row.type
|
||||
|
||||
%tr{ class: "tree-item file_#{hexdigest(tree_row_name)}" }
|
||||
%td.tree-item-file-name
|
||||
- if tree_row_type == :tree
|
||||
= tree_icon('folder', tree_row.mode, tree_row.name)
|
||||
- path = flatten_tree(@path, tree_row)
|
||||
%a.str-truncated{ href: fast_project_tree_path(@project, tree_join(@id || @commit.id, path)), title: path }
|
||||
%span= path
|
||||
|
||||
- elsif tree_row_type == :blob
|
||||
= tree_icon('file', tree_row.mode, tree_row_name)
|
||||
%a.str-truncated{ href: fast_project_blob_path(@project, tree_join(@id || @commit.id, tree_row_name)), title: tree_row_name }
|
||||
%span= tree_row_name
|
||||
- if @lfs_blob_ids.include?(tree_row.id)
|
||||
%span.badge.label-lfs.prepend-left-5 LFS
|
||||
|
||||
- elsif tree_row_type == :commit
|
||||
= tree_icon('archive', tree_row.mode, tree_row.name)
|
||||
= submodule_link(tree_row, @ref)
|
||||
|
||||
%td.d-none.d-sm-table-cell.tree-commit
|
||||
%td.tree-time-ago.text-right
|
||||
%span.log_loading.hide
|
||||
%i.fa.fa-spinner.fa-spin
|
||||
Loading commit data...
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Improve performance of tree rendering in repositories with lots of items
|
||||
merge_request:
|
||||
author:
|
||||
type: performance
|
||||
|
|
@ -3,7 +3,7 @@ require 'spec_helper'
|
|||
describe TreeHelper do
|
||||
let(:project) { create(:project, :repository) }
|
||||
let(:repository) { project.repository }
|
||||
let(:sha) { 'ce369011c189f62c815f5971d096b26759bab0d1' }
|
||||
let(:sha) { 'c1c67abbaf91f624347bb3ae96eabe3a1b742478' }
|
||||
|
||||
describe '.render_tree' do
|
||||
before do
|
||||
|
|
@ -32,6 +32,49 @@ describe TreeHelper do
|
|||
end
|
||||
end
|
||||
|
||||
describe '.fast_project_blob_path' do
|
||||
it 'generates the same path as project_blob_path' do
|
||||
blob_path = repository.tree(sha, 'with space').entries.first.path
|
||||
fast_path = fast_project_blob_path(project, blob_path)
|
||||
std_path = project_blob_path(project, blob_path)
|
||||
|
||||
expect(fast_path).to eq(std_path)
|
||||
end
|
||||
|
||||
it 'generates the same path with encoded file names' do
|
||||
tree = repository.tree(sha, 'encoding')
|
||||
blob_path = tree.entries.find { |entry| entry.path == 'encoding/テスト.txt' }.path
|
||||
fast_path = fast_project_blob_path(project, blob_path)
|
||||
std_path = project_blob_path(project, blob_path)
|
||||
|
||||
expect(fast_path).to eq(std_path)
|
||||
end
|
||||
|
||||
it 'respects a configured relative URL' do
|
||||
allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
|
||||
blob_path = repository.tree(sha, '').entries.first.path
|
||||
fast_path = fast_project_blob_path(project, blob_path)
|
||||
|
||||
expect(fast_path).to start_with('/gitlab/root')
|
||||
end
|
||||
end
|
||||
|
||||
describe '.fast_project_tree_path' do
|
||||
let(:tree_path) { repository.tree(sha, 'with space').path }
|
||||
let(:fast_path) { fast_project_tree_path(project, tree_path) }
|
||||
let(:std_path) { project_tree_path(project, tree_path) }
|
||||
|
||||
it 'generates the same path as project_tree_path' do
|
||||
expect(fast_path).to eq(std_path)
|
||||
end
|
||||
|
||||
it 'respects a configured relative URL' do
|
||||
allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
|
||||
|
||||
expect(fast_path).to start_with('/gitlab/root')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'flatten_tree' do
|
||||
let(:tree) { repository.tree(sha, 'files') }
|
||||
let(:root_path) { 'files' }
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe 'projects/tree/_blob_item' do
|
||||
describe 'projects/tree/_tree_row' do
|
||||
let(:project) { create(:project, :repository) }
|
||||
let(:repository) { project.repository }
|
||||
let(:blob_item) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID, 'files/ruby').first }
|
||||
|
|
@ -31,10 +31,7 @@ describe 'projects/tree/_blob_item' do
|
|||
end
|
||||
end
|
||||
|
||||
def render_partial(blob_item)
|
||||
render partial: 'projects/tree/blob_item', locals: {
|
||||
blob_item: blob_item,
|
||||
type: 'blob'
|
||||
}
|
||||
def render_partial(items)
|
||||
render partial: 'projects/tree/tree_row', collection: [items].flatten
|
||||
end
|
||||
end
|
||||
Loading…
Reference in New Issue