Improve checking if projects would be returned

In various places we check if the same relation would return projects.
This is done using "any?" which will run a COUNT query with any
LIMIT/OFFSET values still applied.

To work around all this we introduce 2 helper methods that take care of
doing the right thing. This leads to the produced queries being simpler
and fewer queries being executed.
This commit is contained in:
Yorick Peterse 2017-08-04 15:40:33 +02:00
parent 81933cfdd3
commit f77fda6437
No known key found for this signature in database
GPG key ID: EDD30D2BEB691AC9
7 changed files with 72 additions and 6 deletions

View file

@ -225,6 +225,26 @@ module ProjectsHelper
end
end
# Returns true if any projects are present.
#
# If the relation has a LIMIT applied we'll cast the relation to an Array
# since repeated any? checks would otherwise result in multiple COUNT queries
# being executed.
#
# If no limit is applied we'll just issue a COUNT since the result set could
# be too large to load into memory.
def any_projects?(projects)
if projects.limit_value
projects.to_a.any?
else
projects.except(:offset).any?
end
end
def has_projects_or_name?(projects, params)
!!(params[:name] || any_projects?(projects))
end
private
def repo_children_classes(field)

View file

@ -13,10 +13,8 @@
- if show_callout?('user_callout_dismissed')
= render 'shared/user_callout'
- if @projects.any? || params[:name]
- if has_projects_or_name?(@projects, params)
= render 'dashboard/projects_head'
- if @projects.any? || params[:name]
= render 'projects'
- else
= render "zero_authorized_projects"

View file

@ -9,7 +9,7 @@
%div{ class: container_class }
= render 'dashboard/projects_head'
- if @projects.any? || params[:filter_projects]
- if params[:filter_projects] || any_projects?(@projects)
= render 'projects'
- else
%h3 You don't have starred projects yet

View file

@ -1,4 +1,4 @@
- if @projects.any?
- if any_projects?(@projects)
.project-item-select-holder
= project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at', relative_path: local_assigns[:path] }, with_feature_enabled: local_assigns[:with_feature_enabled]
%a.btn.btn-new.new-project-item-select-button

View file

@ -10,7 +10,7 @@
- load_pipeline_status(projects)
.js-projects-list-holder
- if projects.any?
- if any_projects?(projects)
%ul.projects-list
- projects.each_with_index do |project, i|
- css_class = (i >= projects_limit) || project.pending_delete? ? 'hide' : nil

View file

@ -0,0 +1,4 @@
---
title: "Improve performance of checking for projects on the projects dashboard"
merge_request:
author:

View file

@ -411,4 +411,48 @@ describe ProjectsHelper do
end
end
end
describe '#has_projects_or_name?' do
let(:projects) do
create(:project)
Project.all
end
it 'returns true when there are projects' do
expect(helper.has_projects_or_name?(projects, {})).to eq(true)
end
it 'returns true when there are no projects but a name is given' do
expect(helper.has_projects_or_name?(Project.none, name: 'foo')).to eq(true)
end
it 'returns false when there are no projects and there is no name' do
expect(helper.has_projects_or_name?(Project.none, {})).to eq(false)
end
end
describe '#any_projects?' do
before do
create(:project)
end
it 'returns true when projects will be returned' do
expect(helper.any_projects?(Project.all)).to eq(true)
end
it 'returns false when no projects will be returned' do
expect(helper.any_projects?(Project.none)).to eq(false)
end
it 'only executes a single query when a LIMIT is applied' do
relation = Project.limit(1)
recorder = ActiveRecord::QueryRecorder.new do
2.times do
helper.any_projects?(relation)
end
end
expect(recorder.count).to eq(1)
end
end
end