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

6 files changed, 40 insertions, 12 deletions

diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 9a74e36870b..001c83ccb4b 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -49,6 +49,32 @@ class IssuableFinder
     execute.find_by(*params)
   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
     return @group if defined?(@group)
 
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 6584aa3edd5..8231f8fa334 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -180,12 +180,9 @@ module IssuablesHelper
   end
 
   def issuables_count_for_state(issuable_type, state)
-    issuables_finder = public_send("#{issuable_type}_finder")
-
-    params = issuables_finder.params.merge(state: state)
-    finder = issuables_finder.class.new(issuables_finder.current_user, params)
-
-    finder.execute.page(1).total_count
+    @counts ||= {}
+    @counts[issuable_type] ||= public_send("#{issuable_type}_finder").count_by_state
+    @counts[issuable_type][state]
   end
 
   IRRELEVANT_PARAMS_FOR_CACHE_KEY = %i[utf8 sort page]
@@ -195,6 +192,7 @@ module IssuablesHelper
     opts = params.with_indifferent_access
     opts[:state] = state
     opts.except!(*IRRELEVANT_PARAMS_FOR_CACHE_KEY)
+    opts.delete_if { |_, value| value.blank? }
 
     hexdigest(['issuables_count', issuable_type, opts.sort].flatten.join('-'))
   end
diff --git a/app/views/shared/_issues.html.haml b/app/views/shared/_issues.html.haml
index baa6d5f8206..26b349e8a62 100644
--- a/app/views/shared/_issues.html.haml
+++ b/app/views/shared/_issues.html.haml
@@ -1,4 +1,4 @@
-- if @issues.reorder(nil).any?
+- if @issues.to_a.any?
   - @issues.group_by(&:project).each do |group|
     .panel.panel-default.panel-small
       - project = group[0]
diff --git a/app/views/shared/_merge_requests.html.haml b/app/views/shared/_merge_requests.html.haml
index ca3178395c1..2f3605b4d27 100644
--- a/app/views/shared/_merge_requests.html.haml
+++ b/app/views/shared/_merge_requests.html.haml
@@ -1,4 +1,4 @@
-- if @merge_requests.reorder(nil).any?
+- if @merge_requests.to_a.any?
   - @merge_requests.group_by(&:target_project).each do |group|
     .panel.panel-default.panel-small
       - project = group[0]
diff --git a/changelogs/unreleased/24669-merge-request-dashboard-page-takes-over-a-minute-to-load.yml b/changelogs/unreleased/24669-merge-request-dashboard-page-takes-over-a-minute-to-load.yml
new file mode 100644
index 00000000000..01b19a47ecd
--- /dev/null
+++ b/changelogs/unreleased/24669-merge-request-dashboard-page-takes-over-a-minute-to-load.yml
@@ -0,0 +1,4 @@
+---
+title: Speed up issuable dashboards
+merge_request:
+author:
diff --git a/spec/support/matchers/have_issuable_counts.rb b/spec/support/matchers/have_issuable_counts.rb
index 02605d6b70e..92cf3de5448 100644
--- a/spec/support/matchers/have_issuable_counts.rb
+++ b/spec/support/matchers/have_issuable_counts.rb
@@ -1,9 +1,9 @@
 RSpec::Matchers.define :have_issuable_counts do |opts|
-  match do |actual|
-    expected_counts = opts.map do |state, count|
-      "#{state.to_s.humanize} #{count}"
-    end
+  expected_counts = opts.map do |state, count|
+    "#{state.to_s.humanize} #{count}"
+  end
 
+  match do |actual|
     actual.within '.issues-state-filters' do
       expected_counts.each do |expected_count|
         expect(actual).to have_content(expected_count)