Merge branch 'performance-improvements' into 'master'

Performance improvements

* store @participants in variable
* store result of subscribed? call into variable

In total it reduce amount of SQL queries for issue or merge_request with 10 comments/participants almost twice.

See merge request !883

10 files changed, 33 insertions, 16 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 0c20479c8b1..3a50dffbeee 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -19,6 +19,7 @@ v 7.13.0 (unreleased)
   - Allow Administrators to filter the user list by those with or without Two-factor Authentication enabled.
   - Show a user's Two-factor Authentication status in the administration area.
   - Explicit error when commit not found in the CI
+  - Improve performance for issue and merge request pages 
 
 v 7.12.0 (unreleased)
   - Fix Error 500 when one user attempts to access a personal, internal snippet (Stan Hu)
diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss
index 1419a9cded9..961ac793de2 100644
--- a/app/assets/stylesheets/generic/common.scss
+++ b/app/assets/stylesheets/generic/common.scss
@@ -364,3 +364,12 @@ table {
     margin-top: 8px;
   }
 }
+
+.profiler-results {
+  top: 50px !important;
+
+  .profiler-button,
+  .profiler-controls {
+    border-color: #EEE !important;
+  }
+}
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 7d168aa827b..69bd1f58449 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -55,6 +55,7 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def show
+    @participants = @issue.participants(current_user, @project)
     @note = @project.notes.new(noteable: @issue)
     @notes = @issue.notes.inc_author.fresh
     @noteable = @issue
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 51ecbfd561a..a13688305b7 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -246,6 +246,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def define_show_vars
+    @participants = @merge_request.participants(current_user, @project)
+
     # Build a note object for comment form
     @note = @project.notes.new(noteable: @merge_request)
     @notes = @merge_request.mr_and_commit_notes.inc_author.fresh
diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb
index 9f667f47e0d..7c9597333dd 100644
--- a/app/models/concerns/participable.rb
+++ b/app/models/concerns/participable.rb
@@ -14,7 +14,7 @@
 #
 #       participant :author, :assignee, :mentioned_users, :notes
 #     end
-#     
+#
 #     issue = Issue.last
 #     users = issue.participants
 #     # `users` will contain the issue's author, its assignee,
@@ -35,11 +35,13 @@ module Participable
     end
   end
 
+  # Be aware that this method makes a lot of sql queries.
+  # Save result into variable if you are going to reuse it inside same request
   def participants(current_user = self.author, project = self.project)
     participants = self.class.participant_attrs.flat_map do |attr|
       meth = method(attr)
 
-      value = 
+      value =
         if meth.arity == 1 || meth.arity == -1
           meth.call(current_user)
         else
@@ -59,7 +61,7 @@ module Participable
   end
 
   private
-  
+
   def participants_for(value, current_user = nil, project = nil)
     case value
     when User
diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml
index 48858fa32da..656e06ca105 100644
--- a/app/views/projects/issues/_discussion.html.haml
+++ b/app/views/projects/issues/_discussion.html.haml
@@ -12,8 +12,8 @@
     .votes-holder.pull-right
       #votes= render 'votes/votes_block', votable: @issue
     .participants
-      %span= pluralize(@issue.participants(current_user).count, 'participant')
-      - @issue.participants(current_user).each do |participant|
+      %span= pluralize(@participants.count, 'participant')
+      - @participants.each do |participant|
         = link_to_member(@project, participant, name: false, size: 24)
     .voting_notes#notes= render 'projects/notes/notes_with_form'
   %aside.col-md-3
diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml
index 323f5c84a85..88b63946905 100644
--- a/app/views/projects/issues/_issue_context.html.haml
+++ b/app/views/projects/issues/_issue_context.html.haml
@@ -28,18 +28,19 @@
       = f.submit class: 'btn'
 
   - if current_user
+    - subscribed = @issue.subscribed?(current_user)
     %div.prepend-top-20.clearfix
       .issuable-context-title
         %label
           Subscription:
       %button.btn.btn-block.subscribe-button{:type => 'button'}
         %i.fa.fa-eye
-        %span= @issue.subscribed?(current_user) ? "Unsubscribe" : "Subscribe"
-      - subscribtion_status = @issue.subscribed?(current_user) ? "subscribed" : "unsubscribed"
+        %span= subscribed ? "Unsubscribe" : "Subscribe"
+      - subscribtion_status = subscribed ? "subscribed" : "unsubscribed"
       .subscription-status{"data-status" => subscribtion_status}
-        .description-block.unsubscribed{class: ( "hidden" if @issue.subscribed?(current_user) )}
+        .description-block.unsubscribed{class: ( "hidden" if subscribed )}
           You're not receiving notifications from this thread.
-        .description-block.subscribed{class: ( "hidden" unless @issue.subscribed?(current_user) )}
+        .description-block.subscribed{class: ( "hidden" unless subscribed )}
           You're receiving notifications because you're subscribed to this thread.
 
 :coffeescript
diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml
index 1d0e2e350b0..5f2f65e0087 100644
--- a/app/views/projects/merge_requests/show/_context.html.haml
+++ b/app/views/projects/merge_requests/show/_context.html.haml
@@ -30,18 +30,19 @@
         = f.submit class: 'btn'
 
   - if current_user
+    - subscribed = @merge_request.subscribed?(current_user)
     %div.prepend-top-20.clearfix
       .issuable-context-title
         %label
           Subscription:
       %button.btn.btn-block.subscribe-button{:type => 'button'}
         = icon('eye')
-        %span= @merge_request.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe'
-      - subscribtion_status = @merge_request.subscribed?(current_user) ? 'subscribed' : 'unsubscribed'
+        %span= subscribed ? 'Unsubscribe' : 'Subscribe'
+      - subscribtion_status = subscribed ? 'subscribed' : 'unsubscribed'
       .subscription-status{data: {status: subscribtion_status}}
-        .description-block.unsubscribed{class: ( 'hidden' if @merge_request.subscribed?(current_user) )}
+        .description-block.unsubscribed{class: ( 'hidden' if subscribed )}
           You're not receiving notifications from this thread.
-        .description-block.subscribed{class: ( 'hidden' unless @merge_request.subscribed?(current_user) )}
+        .description-block.subscribed{class: ( 'hidden' unless subscribed )}
           You're receiving notifications because you're subscribed to this thread.
 
 :coffeescript
diff --git a/app/views/projects/merge_requests/show/_participants.html.haml b/app/views/projects/merge_requests/show/_participants.html.haml
index 9c93fa55fe6..c67afe963e7 100644
--- a/app/views/projects/merge_requests/show/_participants.html.haml
+++ b/app/views/projects/merge_requests/show/_participants.html.haml
@@ -1,4 +1,4 @@
 .participants
-  %span #{@merge_request.participants(current_user).count} participants
-  - @merge_request.participants(current_user).each do |participant|
+  %span #{@participants.count} participants
+  - @participants.each do |participant|
     = link_to_member(@project, participant, name: false, size: 24)
diff --git a/config/initializers/6_rack_profiler.rb b/config/initializers/6_rack_profiler.rb
index 5312fd8e89a..1d958904e8f 100644
--- a/config/initializers/6_rack_profiler.rb
+++ b/config/initializers/6_rack_profiler.rb
@@ -5,6 +5,6 @@ if Rails.env.development?
   Rack::MiniProfilerRails.initialize!(Rails.application)
 
   Rack::MiniProfiler.config.position = 'right'
-  Rack::MiniProfiler.config.start_hidden = true
+  Rack::MiniProfiler.config.start_hidden = false
   Rack::MiniProfiler.config.skip_paths << '/teaspoon'
 end