Merge branch 'jej-memoize-milestoneish-visible-to-user' into 'master'

Memoize Milestoneish#issues_visible_to_user to reduce lookups

https://gitlab.com/gitlab-org/gitlab-ce/issues/25748

See merge request !8146

4 files changed, 39 insertions, 6 deletions

diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 99f17d668e5..45d18ed3288 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -1,10 +1,15 @@
 module Milestoneish
   def closed_items_count(user)
-    issues_visible_to_user(user).closed.size + merge_requests.closed_and_merged.size
+    memoize_per_user(user, :closed_items_count) do
+      (count_issues_by_state(user)['closed'] || 0) + merge_requests.closed_and_merged.size
+    end
   end
 
   def total_items_count(user)
-    issues_visible_to_user(user).size + merge_requests.size
+    memoize_per_user(user, :total_items_count) do
+      issues_count = count_issues_by_state(user).values.sum
+      issues_count + merge_requests.size
+    end
   end
 
   def complete?(user)
@@ -24,6 +29,23 @@ module Milestoneish
   end
 
   def issues_visible_to_user(user)
-    IssuesFinder.new(user).execute.where(id: issues)
+    memoize_per_user(user, :issues_visible_to_user) do
+      params = try(:project_id) ? { project_id: project_id } : {}
+      IssuesFinder.new(user, params).execute.where(milestone_id: milestoneish_ids)
+    end
+  end
+
+  private
+
+  def count_issues_by_state(user)
+    memoize_per_user(user, :count_issues_by_state) do
+      issues_visible_to_user(user).reorder(nil).group(:state).count
+    end
+  end
+
+  def memoize_per_user(user, method_name)
+    @memoized ||= {}
+    @memoized[method_name] ||= {}
+    @memoized[method_name][user.try!(:id)] ||= yield
   end
 end
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index cde4a568577..0dfbcb76e42 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -24,6 +24,10 @@ class GlobalMilestone
     @first_milestone = milestones.find {|m| m.description.present? } || milestones.first
   end
 
+  def milestoneish_ids
+    milestones.select(:id)
+  end
+
   def safe_title
     @title.to_slug.normalize.to_s
   end
@@ -37,7 +41,7 @@ class GlobalMilestone
   end
 
   def projects
-    @projects ||= Project.for_milestones(milestones.select(:id))
+    @projects ||= Project.for_milestones(milestoneish_ids)
   end
 
   def state
@@ -59,11 +63,11 @@ class GlobalMilestone
   end
 
   def issues
-    @issues ||= Issue.of_milestones(milestones.select(:id)).includes(:project, :assignee, :labels)
+    @issues ||= Issue.of_milestones(milestoneish_ids).includes(:project, :assignee, :labels)
   end
 
   def merge_requests
-    @merge_requests ||= MergeRequest.of_milestones(milestones.select(:id)).includes(:target_project, :assignee, :labels)
+    @merge_requests ||= MergeRequest.of_milestones(milestoneish_ids).includes(:target_project, :assignee, :labels)
   end
 
   def participants
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 23aecbfa3a6..1b2ff16f3d2 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -147,6 +147,9 @@ class Milestone < ActiveRecord::Base
         "expires on #{due_date.to_s(:medium)}"
       end
     end
+
+  def milestoneish_ids
+    id
   end
 
   def can_be_closed?
diff --git a/changelogs/unreleased/jej-memoize-milestoneish-visible-to-user.yml b/changelogs/unreleased/jej-memoize-milestoneish-visible-to-user.yml
new file mode 100644
index 00000000000..ab7f39a4178
--- /dev/null
+++ b/changelogs/unreleased/jej-memoize-milestoneish-visible-to-user.yml
@@ -0,0 +1,4 @@
+---
+title: Milestoneish SQL performance partially improved and memoized
+merge_request: 8146
+author: