From ff26ea818cb92eabc8fbea1a8385cb5bc6b0a66f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Fri, 17 Nov 2017 11:48:32 +0000 Subject: [PATCH] Resolve "Performance issues when loading large number of wiki pages" --- app/controllers/projects/wikis_controller.rb | 8 ++- app/models/project_wiki.rb | 4 +- app/models/wiki_page.rb | 19 ++++--- .../projects/wikis/_pages_wiki_page.html.haml | 2 +- app/views/projects/wikis/history.html.haml | 3 +- app/views/projects/wikis/show.html.haml | 4 +- .../34600-performance-wiki-pages.yml | 5 ++ config/initializers/gollum.rb | 26 +++++++++ lib/gitlab/git/wiki.rb | 56 +++++++++++++++---- spec/models/wiki_page_spec.rb | 2 +- 10 files changed, 102 insertions(+), 27 deletions(-) create mode 100644 changelogs/unreleased/34600-performance-wiki-pages.yml diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index f7a9c98629d..b51261a49e0 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -74,7 +74,11 @@ class Projects::WikisController < Projects::ApplicationController def history @page = @project_wiki.find_page(params[:id]) - unless @page + if @page + @page_versions = Kaminari.paginate_array(@page.versions(page: params[:page]), + total_count: @page.count_versions) + .page(params[:page]) + else redirect_to( project_wiki_path(@project, :home), notice: "Page not found" @@ -101,7 +105,7 @@ class Projects::WikisController < Projects::ApplicationController # Call #wiki to make sure the Wiki Repo is initialized @project_wiki.wiki - @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages.first(15)) + @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages(limit: 15)) rescue ProjectWiki::CouldNotCreateWikiError flash[:notice] = "Could not create Wiki Repository at this time. Please try again later." redirect_to project_path(@project) diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 3eecbea8cbf..a0af749a93f 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -76,8 +76,8 @@ class ProjectWiki # Returns an Array of Gitlab WikiPage instances or an # empty Array if this Wiki has no pages. - def pages - wiki.pages.map { |page| WikiPage.new(self, page, true) } + def pages(limit: nil) + wiki.pages(limit: limit).map { |page| WikiPage.new(self, page, true) } end # Finds a page within the repository based on a tile diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 5f710961f95..bdfef677ef3 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -127,19 +127,24 @@ class WikiPage @version ||= @page.version end - # Returns an array of Gitlab Commit instances. - def versions + def versions(options = {}) return [] unless persisted? - wiki.wiki.page_versions(@page.path) + wiki.wiki.page_versions(@page.path, options) end - def commit - versions.first + def count_versions + return [] unless persisted? + + wiki.wiki.count_page_versions(@page.path) + end + + def last_version + @last_version ||= versions(limit: 1).first end def last_commit_sha - commit&.sha + last_version&.sha end # Returns the Date that this latest version was @@ -151,7 +156,7 @@ class WikiPage # Returns boolean True or False if this instance # is an old version of the page. def historical? - @page.historical? && versions.first.sha != version.sha + @page.historical? && last_version.sha != version.sha end # Returns boolean True or False if this instance diff --git a/app/views/projects/wikis/_pages_wiki_page.html.haml b/app/views/projects/wikis/_pages_wiki_page.html.haml index 0a1ccbc5f1c..efa16d38f84 100644 --- a/app/views/projects/wikis/_pages_wiki_page.html.haml +++ b/app/views/projects/wikis/_pages_wiki_page.html.haml @@ -2,4 +2,4 @@ = link_to wiki_page.title, project_wiki_path(@project, wiki_page) %small (#{wiki_page.format}) .pull-right - %small= (s_("Last edited %{date}") % { date: time_ago_with_tooltip(wiki_page.commit.authored_date) }).html_safe + %small= (s_("Last edited %{date}") % { date: time_ago_with_tooltip(wiki_page.last_version.authored_date) }).html_safe diff --git a/app/views/projects/wikis/history.html.haml b/app/views/projects/wikis/history.html.haml index 9ee09262324..969a1677d9a 100644 --- a/app/views/projects/wikis/history.html.haml +++ b/app/views/projects/wikis/history.html.haml @@ -21,7 +21,7 @@ %th= _("Last updated") %th= _("Format") %tbody - - @page.versions.each_with_index do |version, index| + - @page_versions.each_with_index do |version, index| - commit = version %tr %td @@ -37,5 +37,6 @@ %td %strong = version.format += paginate @page_versions, theme: 'gitlab' = render 'sidebar' diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml index de15fc99eda..b3b83cee81a 100644 --- a/app/views/projects/wikis/show.html.haml +++ b/app/views/projects/wikis/show.html.haml @@ -11,8 +11,8 @@ .nav-text %h2.wiki-page-title= @page.title.capitalize %span.wiki-last-edit-by - = (_("Last edited by %{name}") % { name: "#{@page.commit.author_name}" }).html_safe - #{time_ago_with_tooltip(@page.commit.authored_date)} + = (_("Last edited by %{name}") % { name: "#{@page.last_version.author_name}" }).html_safe + #{time_ago_with_tooltip(@page.last_version.authored_date)} .nav-controls = render 'main_links' diff --git a/changelogs/unreleased/34600-performance-wiki-pages.yml b/changelogs/unreleased/34600-performance-wiki-pages.yml new file mode 100644 index 00000000000..541ae8f8e60 --- /dev/null +++ b/changelogs/unreleased/34600-performance-wiki-pages.yml @@ -0,0 +1,5 @@ +--- +title: Performance issues when loading large number of wiki pages +merge_request: 15276 +author: +type: performance diff --git a/config/initializers/gollum.rb b/config/initializers/gollum.rb index 1ebe3c7a742..2ca32791bb1 100644 --- a/config/initializers/gollum.rb +++ b/config/initializers/gollum.rb @@ -10,4 +10,30 @@ module Gollum index.send(name, *args) end end + + class Wiki + def pages(treeish = nil, limit: nil) + tree_list((treeish || @ref), limit: limit) + end + + def tree_list(ref, limit: nil) + if (sha = @access.ref_to_sha(ref)) + commit = @access.commit(sha) + tree_map_for(sha).inject([]) do |list, entry| + next list unless @page_class.valid_page_name?(entry.name) + list << entry.page(self, commit) + break list if limit && list.size >= limit + list + end + else + [] + end + end + end +end + +Rails.application.configure do + config.after_initialize do + Gollum::Page.per_page = Kaminari.config.default_per_page + end end diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index 022d1f249a9..d4a53d32c28 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -58,12 +58,12 @@ module Gitlab end end - def pages - @repository.gitaly_migrate(:wiki_get_all_pages) do |is_enabled| + def pages(limit: nil) + @repository.gitaly_migrate(:wiki_get_all_pages, status: Gitlab::GitalyClient::MigrationStatus::DISABLED) do |is_enabled| if is_enabled gitaly_get_all_pages else - gollum_get_all_pages + gollum_get_all_pages(limit: limit) end end end @@ -88,14 +88,23 @@ module Gitlab end end - def page_versions(page_path) + # options: + # :page - The Integer page number. + # :per_page - The number of items per page. + # :limit - Total number of items to return. + def page_versions(page_path, options = {}) current_page = gollum_page_by_path(page_path) - current_page.versions.map do |gollum_git_commit| - gollum_page = gollum_wiki.page(current_page.title, gollum_git_commit.id) - new_version(gollum_page, gollum_git_commit.id) + + commits_from_page(current_page, options).map do |gitlab_git_commit| + gollum_page = gollum_wiki.page(current_page.title, gitlab_git_commit.id) + Gitlab::Git::WikiPageVersion.new(gitlab_git_commit, gollum_page&.format) end end + def count_page_versions(page_path) + @repository.count_commits(ref: 'HEAD', path: page_path) + end + def preview_slug(title, format) # Adapted from gollum gem (Gollum::Wiki#preview_page) to avoid # using Rugged through a Gollum::Wiki instance @@ -110,6 +119,22 @@ module Gitlab private + # options: + # :page - The Integer page number. + # :per_page - The number of items per page. + # :limit - Total number of items to return. + def commits_from_page(gollum_page, options = {}) + unless options[:limit] + options[:offset] = ([1, options.delete(:page).to_i].max - 1) * Gollum::Page.per_page + options[:limit] = (options.delete(:per_page) || Gollum::Page.per_page).to_i + end + + @repository.log(ref: gollum_page.last_version.id, + path: gollum_page.path, + limit: options[:limit], + offset: options[:offset]) + end + def gollum_wiki @gollum_wiki ||= Gollum::Wiki.new(@repository.path) end @@ -126,8 +151,17 @@ module Gitlab end def new_version(gollum_page, commit_id) - commit = Gitlab::Git::Commit.find(@repository, commit_id) - Gitlab::Git::WikiPageVersion.new(commit, gollum_page&.format) + Gitlab::Git::WikiPageVersion.new(version(commit_id), gollum_page&.format) + end + + def version(commit_id) + commit_find_proc = -> { Gitlab::Git::Commit.find(@repository, commit_id) } + + if RequestStore.active? + RequestStore.fetch([:wiki_version_commit, commit_id]) { commit_find_proc.call } + else + commit_find_proc.call + end end def assert_type!(object, klass) @@ -185,8 +219,8 @@ module Gitlab Gitlab::Git::WikiFile.new(gollum_file) end - def gollum_get_all_pages - gollum_wiki.pages.map { |gollum_page| new_page(gollum_page) } + def gollum_get_all_pages(limit: nil) + gollum_wiki.pages(limit: limit).map { |gollum_page| new_page(gollum_page) } end def gitaly_write_page(name, format, content, commit_details) diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index a7227b38850..ea75434e399 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -373,7 +373,7 @@ describe WikiPage do end it 'returns commit sha' do - expect(@page.last_commit_sha).to eq @page.commit.sha + expect(@page.last_commit_sha).to eq @page.last_version.sha end it 'is changed after page updated' do