Commit 74a750d3 authored by Yorick Peterse's avatar Yorick Peterse Committed by Alejandro Rodríguez

Merge branch '24669-merge-request-dashboard-page-takes-over-a-minute-to-load' into 'master'

Resolve "Merge request dashboard page takes over a minute to load"

See merge request !7760
parent c16a5701
...@@ -51,6 +51,32 @@ class IssuableFinder ...@@ -51,6 +51,32 @@ class IssuableFinder
execute.find_by(*params) execute.find_by(*params)
end end
# We often get counts for each state by running a query per state, and
# counting those results. This is typically slower than running one query
# (even if that query is slower than any of the individual state queries) and
# grouping and counting within that query.
#
def count_by_state
count_params = params.merge(state: nil, sort: nil)
labels_count = label_names.any? ? label_names.count : 1
finder = self.class.new(current_user, count_params)
counts = Hash.new(0)
# Searching by label includes a GROUP BY in the query, but ours will be last
# because it is added last. Searching by multiple labels also includes a row
# per issuable, so we have to count those in Ruby - which is bad, but still
# better than performing multiple queries.
#
finder.execute.reorder(nil).group(:state).count.each do |key, value|
counts[Array(key).last.to_sym] += value / labels_count
end
counts[:all] = counts.values.sum
counts[:opened] += counts[:reopened]
counts
end
def group def group
return @group if defined?(@group) return @group if defined?(@group)
......
...@@ -170,12 +170,9 @@ module IssuablesHelper ...@@ -170,12 +170,9 @@ module IssuablesHelper
end end
def issuables_count_for_state(issuable_type, state) def issuables_count_for_state(issuable_type, state)
issuables_finder = public_send("#{issuable_type}_finder") @counts ||= {}
@counts[issuable_type] ||= public_send("#{issuable_type}_finder").count_by_state
params = issuables_finder.params.merge(state: state) @counts[issuable_type][state]
finder = issuables_finder.class.new(issuables_finder.current_user, params)
finder.execute.page(1).total_count
end end
IRRELEVANT_PARAMS_FOR_CACHE_KEY = %i[utf8 sort page] IRRELEVANT_PARAMS_FOR_CACHE_KEY = %i[utf8 sort page]
...@@ -185,6 +182,7 @@ module IssuablesHelper ...@@ -185,6 +182,7 @@ module IssuablesHelper
opts = params.with_indifferent_access opts = params.with_indifferent_access
opts[:state] = state opts[:state] = state
opts.except!(*IRRELEVANT_PARAMS_FOR_CACHE_KEY) opts.except!(*IRRELEVANT_PARAMS_FOR_CACHE_KEY)
opts.delete_if { |_, value| value.blank? }
hexdigest(['issuables_count', issuable_type, opts.sort].flatten.join('-')) hexdigest(['issuables_count', issuable_type, opts.sort].flatten.join('-'))
end end
......
- if @issues.reorder(nil).any? - if @issues.to_a.any?
- @issues.group_by(&:project).each do |group| - @issues.group_by(&:project).each do |group|
.panel.panel-default.panel-small .panel.panel-default.panel-small
- project = group[0] - project = group[0]
......
- if @merge_requests.reorder(nil).any? - if @merge_requests.to_a.any?
- @merge_requests.group_by(&:target_project).each do |group| - @merge_requests.group_by(&:target_project).each do |group|
.panel.panel-default.panel-small .panel.panel-default.panel-small
- project = group[0] - project = group[0]
......
---
title: Speed up issuable dashboards
merge_request:
author:
RSpec::Matchers.define :have_issuable_counts do |opts| RSpec::Matchers.define :have_issuable_counts do |opts|
match do |actual| expected_counts = opts.map do |state, count|
expected_counts = opts.map do |state, count| "#{state.to_s.humanize} #{count}"
"#{state.to_s.humanize} #{count}" end
end
match do |actual|
actual.within '.issues-state-filters' do actual.within '.issues-state-filters' do
expected_counts.each do |expected_count| expected_counts.each do |expected_count|
expect(actual).to have_content(expected_count) expect(actual).to have_content(expected_count)
......
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