Improve the performance of save and friends

The biggest source of the performance regression in these methods occurred because dirty tracking required eagerly materializing and type casting the assigned values. In the previous commits, I've changed dirty tracking to perform the comparisons lazily. However, all of this is moot when calling `save`, since `changes_applied` will be called, which just ends up eagerly materializing everything, anyway. With the new mutation tracker, it's easy to just compare the previous two hashes in the same lazy fashion. We will not have aliasing issues with this setup, which is proven by the fact that we're able to detect nested mutation. Before: User.create! 2.007k (± 7.1%) i/s - 10.098k After: User.create! 2.557k (± 3.5%) i/s - 12.789k Fixes #19859
rails · Sep 24, 2015 · 136fc65 · claudiob · May 8, 2018 · kaspth
1 parent 8a811c8
commit 136fc65
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 4 deletions.
diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb
@@ -203,7 +203,7 @@ def changes_include?(attr_name)
       # Returns +true+ if attr_name were changed before the model was saved,
       # +false+ otherwise.
       def previous_changes_include?(attr_name)
-        @previously_changed.include?(attr_name)
+        previous_changes.include?(attr_name)
       end
 
       # Removes current changes and makes them accessible through +previous_changes+.
@@ -225,7 +225,7 @@ def attribute_change(attr)
 
       # Handles <tt>*_previous_change</tt> for +method_missing+.
       def attribute_previous_change(attr)
-        @previously_changed[attr] if attribute_previously_changed?(attr)
+        previous_changes[attr] if attribute_previously_changed?(attr)
       end
 
       # Handles <tt>*_will_change!</tt> for +method_missing+.

diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -51,12 +51,12 @@ def initialize_dup(other) # :nodoc:
       end
 
       def changes_applied
-        super
+        @previous_mutation_tracker = @mutation_tracker
         store_original_attributes
       end
 
       def clear_changes_information
-        super
+        @previous_mutation_tracker = nil
         store_original_attributes
       end
 
@@ -89,6 +89,10 @@ def changes
         end
       end
 
+      def previous_changes
+        previous_mutation_tracker.changes
+      end
+
       def attribute_changed_in_place?(attr_name)
         @mutation_tracker.changed_in_place?(attr_name)
       end
@@ -119,6 +123,10 @@ def store_original_attributes
         @mutation_tracker = @mutation_tracker.now_tracking(@attributes)
       end
 
+      def previous_mutation_tracker
+        @previous_mutation_tracker ||= NullMutationTracker.new
+      end
+
       def cache_changed_attributes
         @cached_changed_attributes = changed_attributes
         yield

diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activerecord/lib/active_record/attribute_mutation_tracker.rb
@@ -13,6 +13,14 @@ def changed_values
       end
     end
 
+    def changes
+      attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result|
+        if changed?(attr_name)
+          result[attr_name] = [original_attributes.fetch_value(attr_name), attributes.fetch_value(attr_name)]
+        end
+      end
+    end
+
     def changed?(attr_name)
       attr_name = attr_name.to_s
       modified?(attr_name) || changed_in_place?(attr_name)
@@ -52,4 +60,25 @@ def clean_copy_of(attributes)
       end
     end
   end
+
+  class NullMutationTracker
+    def changed_values
+      {}
+    end
+
+    def changes
+      {}
+    end
+
+    def changed?(*)
+      false
+    end
+
+    def changed_in_place?(*)
+      false
+    end
+
+    def forget_change(*)
+    end
+  end
 end