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

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)

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;
+  }
+}

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

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

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

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

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

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

app/views/projects/merge_requests/show/_participants.html.haml

 .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)

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