Merge pull request #190 from edx/perf/unnecessary-merges

Reduced merges, do them in place, and reduce allocations.

models/comment.rb

 require_relative 'content'
+require_relative 'constants'
 
 class Comment < Content
-
   include Mongoid::Tree
   include Mongoid::Timestamps
   include Mongoid::MagicCounterCache
@@ -25,7 +25,6 @@ class Comment < Content
   index({author_id: 1, course_id: 1})
   index({_type: 1, comment_thread_id: 1, author_id: 1, updated_at: 1})
 
-
   index_name Content::ES_INDEX_NAME
 
   mapping do
@@ -39,7 +38,6 @@ class Comment < Content
     indexes :updated_at, type: :date, included_in_all: false
   end
 
-
   belongs_to :comment_thread, index: true
   belongs_to :author, class_name: 'User', index: true
 
@@ -91,19 +89,20 @@ class Comment < Content
       subtree_hash = subtree(sort: sort_by_parent_and_time)
       self.class.hash_tree(subtree_hash).first
     else
-      as_document.slice(*%w[body course_id endorsed endorsement anonymous anonymous_to_peers created_at updated_at at_position_list])
-          .merge("id" => _id)
-          .merge("user_id" => author_id)
-          .merge("username" => author_username)
-          .merge("depth" => depth)
-          .merge("closed" => comment_thread.nil? ? false : comment_thread.closed)
-          .merge("thread_id" => comment_thread_id)
-          .merge("parent_id" => parent_ids[-1])
-          .merge("commentable_id" => comment_thread.nil? ? nil : comment_thread.commentable_id)
-          .merge("votes" => votes.slice(*%w[count up_count down_count point]))
-          .merge("abuse_flaggers" => abuse_flaggers)
-          .merge("type" => "comment")
-          .merge("child_count" => get_cached_child_count)
+      as_document
+        .slice(BODY, COURSE_ID, ENDORSED, ENDORSEMENT, ANONYMOUS, ANONYMOUS_TO_PEERS, CREATED_AT, UPDATED_AT, AT_POSITION_LIST)
+        .merge!("id" => _id,
+                "user_id" => author_id,
+                "username" => author_username,
+                "depth" => depth,
+                "closed" => comment_thread.nil? ? false : comment_thread.closed,
+                "thread_id" => comment_thread_id,
+                "parent_id" => parent_ids[-1],
+                "commentable_id" => comment_thread.nil? ? nil : comment_thread.commentable_id,
+                "votes" => votes.slice(COUNT, UP_COUNT, DOWN_COUNT, POINT),
+                "abuse_flaggers" => abuse_flaggers,
+                "type" => COMMENT,
+                "child_count" => get_cached_child_count)
     end
   end
 

models/comment_thread.rb

-# -*- coding: utf-8 -*-
 require 'new_relic/agent/method_tracer'
 require_relative 'content'
+require_relative 'constants'
 
 class CommentThread < Content
-
   include Mongoid::Timestamps
   include Mongoid::Attributes::Dynamic
   include ActiveModel::MassAssignmentSecurity
@@ -32,7 +31,6 @@ class CommentThread < Content
 
   index({author_id: 1, course_id: 1})
 
-
   index_name Content::ES_INDEX_NAME
 
   mapping do
@@ -122,17 +120,18 @@ class CommentThread < Content
   end
 
   def to_hash(params={})
-    as_document.slice(*%w[thread_type title body course_id anonymous anonymous_to_peers commentable_id created_at updated_at at_position_list closed context last_activity_at])
-        .merge('id' => _id,
-               'user_id' => author_id,
-               'username' => author_username,
-               'votes' => votes.slice(*%w[count up_count down_count point]),
-               'abuse_flaggers' => abuse_flaggers,
-               'tags' => [],
-               'type' => 'thread',
-               'group_id' => group_id,
-               'pinned' => pinned?,
-               'comments_count' => comment_count)
+    as_document
+      .slice(THREAD_TYPE, TITLE, BODY, COURSE_ID, ANONYMOUS, ANONYMOUS_TO_PEERS, COMMENTABLE_ID, CREATED_AT, UPDATED_AT, AT_POSITION_LIST, CLOSED, CONTEXT, LAST_ACTIVITY_AT)
+      .merge!("id" => _id,
+              "user_id" => author_id,
+              "username" => author_username,
+              "votes" => votes.slice(COUNT, UP_COUNT, DOWN_COUNT, POINT),
+              "abuse_flaggers" => abuse_flaggers,
+              "tags" => [],
+              "type" => THREAD,
+              "group_id" => group_id,
+              "pinned" => pinned?,
+              "comments_count" => comment_count)
 
   end
 

models/constants.rb

+BODY = "body".freeze
+COURSE_ID = "course_id".freeze
+ENDORSED = "endorsed".freeze
+ENDORSEMENT = "endorsement".freeze
+ANONYMOUS = "anonymous".freeze
+ANONYMOUS_TO_PEERS = "anonymous_to_peers".freeze
+CREATED_AT = "created_at".freeze
+UPDATED_AT = "updated_at".freeze
+AT_POSITION_LIST = "at_position_list".freeze
+THREAD_TYPE = "thread_type".freeze
+TITLE = "title".freeze
+COMMENTABLE_ID = "commentable_id".freeze
+CLOSED = "closed".freeze
+CONTEXT = "context".freeze
+LAST_ACTIVITY_AT = "last_activity_at".freeze
+NOTIFICATION_TYPE = "notification_type".freeze
+INFO = "info".freeze
+ACTOR_ID = "actor_id".freeze
+TARGET_ID = "target_id".freeze
+SUBSCRIBER_ID = "subscriber_id".freeze
+SOURCE_ID = "source_id".freeze
+SOURCE_TYPE = "source_type".freeze
+COUNT = "count".freeze
+UP_COUNT = "up_count".freeze
+DOWN_COUNT = "down_count".freeze
+POINT = "point".freeze
+USERNAME = "username".freeze
+EXTERNAL_ID = "external_id".freeze
+COMMENT = "comment".freeze
+THREAD = "thread".freeze

models/notification.rb

+require_relative 'constants'
+
 class Notification
   include Mongoid::Document
   include Mongoid::Timestamps
@@ -14,6 +16,8 @@ class Notification
   has_and_belongs_to_many :receivers, class_name: "User", inverse_of: :notifications, autosave: true
 
   def to_hash(params={})
-    as_document.slice(*%w[notification_type info actor_id target_id]).merge("id" => _id)
+    as_document
+      .slice(NOTIFICATION_TYPE, INFO, ACTOR_ID, TARGET_ID)
+      .merge!("id" => _id)
   end
 end

models/subscription.rb

+require_relative 'constants'
+
 class Subscription
   include Mongoid::Document
   include Mongoid::Timestamps
-  
+
   field :subscriber_id, type: String
   field :source_id, type: String
   field :source_type, type: String
-  
+
   index({subscriber_id: 1, source_id: 1, source_type: 1})
   index({subscriber_id: 1, source_type: 1})
   index({subscriber_id: 1})
   index({source_id: 1, source_type: 1}, {background: true})
 
   def to_hash
-    as_document.slice(*%w[subscriber_id source_id source_type]).merge("id" => _id)
+    as_document
+      .slice(SUBSCRIBER_ID, SOURCE_ID, SOURCE_TYPE)
+      .merge!("id" => _id)
   end
 
   def subscriber
@@ -22,5 +26,4 @@ class Subscription
   def source
     source_type.constantize.find(source_id)
   end
-
 end

models/user.rb

 require 'new_relic/agent/method_tracer'
+require_relative 'constants'
 
 class User
   include Mongoid::Document
@@ -35,18 +36,20 @@ class User
   end
 
   def to_hash(params={})
-    hash = as_document.slice(*%w[username external_id])
+    hash = as_document
+      .slice(USERNAME, EXTERNAL_ID)
+
     if params[:complete]
-      hash = hash.merge("subscribed_thread_ids" => subscribed_thread_ids,
+      hash = hash.merge!("subscribed_thread_ids" => subscribed_thread_ids,
                         "subscribed_commentable_ids" => [], # not used by comment client.  To be removed once removed from comment client.
                         "subscribed_user_ids" => [], # ditto.
                         "follower_ids" => [], # ditto.
                         "id" => id,
                         "upvoted_ids" => upvoted_ids,
                         "downvoted_ids" => downvoted_ids,
-                        "default_sort_key" => default_sort_key
-                       )
+                        "default_sort_key" => default_sort_key)
     end
+
     if params[:course_id]
       self.class.trace_execution_scoped(['Custom/User.to_hash/count_comments_and_threads']) do
         if not params[:group_ids].empty?
@@ -101,7 +104,7 @@ class User
             anonymous_to_peers: false
           ).reject{ |comment| comment.standalone_context? }.count
         end
-        hash = hash.merge("threads_count" => threads_count, "comments_count" => comments_count)
+        hash = hash.merge!("threads_count" => threads_count, "comments_count" => comments_count)
       end
     end
     hash

presenters/thread.rb

@@ -97,7 +97,7 @@ class ThreadPresenter
     top_level = []
     ancestry = []
     content.each do |item|
-      item_hash = item.to_hash.merge("children" => [])
+      item_hash = item.to_hash.merge!("children" => [])
       if item.parent_id.nil?
         top_level << item_hash
         ancestry = [item_hash]