From 5fcbdcfb574c731841be12764c50d9587b58345f Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 6 Mar 2018 15:07:24 -0700 Subject: Revert "PERF: Recover `changes_applied` performance (#31698)" This reverts commit a19e91f0fab13cca61acdb1f33e27be2323b9786. --- .../lib/active_model/attribute_mutation_tracker.rb | 7 +- activemodel/lib/active_model/dirty.rb | 106 +++++++++++++-------- .../lib/active_record/attribute_methods/dirty.rb | 6 +- activerecord/lib/active_record/persistence.rb | 1 + 4 files changed, 75 insertions(+), 45 deletions(-) diff --git a/activemodel/lib/active_model/attribute_mutation_tracker.rb b/activemodel/lib/active_model/attribute_mutation_tracker.rb index 8e92c8807f..493be5bb88 100644 --- a/activemodel/lib/active_model/attribute_mutation_tracker.rb +++ b/activemodel/lib/active_model/attribute_mutation_tracker.rb @@ -35,10 +35,6 @@ module ActiveModel end end - def changed_attribute_names - attr_names.select { |attr| changed?(attr) } - end - def any_changes? attr_names.any? { |attr| changed?(attr) } end @@ -108,5 +104,8 @@ module ActiveModel def original_value(*) end + + def force_change(*) + end end end diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index 0044fde6c5..d2ebd18107 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -3,7 +3,6 @@ require "active_support/hash_with_indifferent_access" require "active_support/core_ext/object/duplicable" require "active_model/attribute_mutation_tracker" -require "active_model/attribute_set" module ActiveModel # == Active \Model \Dirty @@ -143,8 +142,9 @@ module ActiveModel end def changes_applied # :nodoc: - _prepare_changes + @previously_changed = changes @mutations_before_last_save = mutations_from_database + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new forget_attribute_assignments @mutations_from_database = nil end @@ -155,7 +155,7 @@ module ActiveModel # person.name = 'bob' # person.changed? # => true def changed? - mutations_from_database.any_changes? + changed_attributes.present? end # Returns an array with the name of the attributes with unsaved changes. @@ -164,24 +164,24 @@ module ActiveModel # person.name = 'bob' # person.changed # => ["name"] def changed - mutations_from_database.changed_attribute_names + changed_attributes.keys end # Handles *_changed? for +method_missing+. def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: - !!mutations_from_database.changed?(attr) && + !!changes_include?(attr) && (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && - (from == OPTION_NOT_GIVEN || from == attribute_was(attr)) + (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) end # Handles *_was for +method_missing+. def attribute_was(attr) # :nodoc: - mutations_from_database.original_value(attr) + attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) end # Handles *_previously_changed? for +method_missing+. def attribute_previously_changed?(attr) #:nodoc: - mutations_before_last_save.changed?(attr) + previous_changes_include?(attr) end # Restore all previous data of the provided attributes. @@ -191,12 +191,15 @@ module ActiveModel # Clears all dirty data: current changes and previous changes. def clear_changes_information + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new forget_attribute_assignments @mutations_from_database = nil end def clear_attribute_changes(attr_names) + attributes_changed_by_setter.except!(*attr_names) attr_names.each do |attr_name| clear_attribute_change(attr_name) end @@ -209,7 +212,13 @@ module ActiveModel # person.name = 'robert' # person.changed_attributes # => {"name" => "bob"} def changed_attributes - mutations_from_database.changed_values.freeze + # This should only be set by methods which will call changed_attributes + # multiple times when it is known that the computed value cannot change. + if defined?(@cached_changed_attributes) + @cached_changed_attributes + else + attributes_changed_by_setter.reverse_merge(mutations_from_database.changed_values).freeze + end end # Returns a hash of changed attributes indicating their original @@ -219,8 +228,9 @@ module ActiveModel # person.name = 'bob' # person.changes # => { "name" => ["bill", "bob"] } def changes - _prepare_changes - mutations_from_database.changes + cache_changed_attributes do + ActiveSupport::HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] + end end # Returns a hash of attributes that were changed before the model was saved. @@ -230,7 +240,8 @@ module ActiveModel # person.save # person.previous_changes # => {"name" => ["bob", "robert"]} def previous_changes - mutations_before_last_save.changes + @previously_changed ||= ActiveSupport::HashWithIndifferentAccess.new + @previously_changed.merge(mutations_before_last_save.changes) end def attribute_changed_in_place?(attr_name) # :nodoc: @@ -246,17 +257,11 @@ module ActiveModel unless defined?(@mutations_from_database) @mutations_from_database = nil end - - unless defined?(@attributes) - @_pseudo_attributes = true - @attributes = AttributeSet.new( - Hash.new { |h, attr| - h[attr] = Attribute.with_cast_value(attr, _clone_attribute(attr), Type.default_value) - } - ) + @mutations_from_database ||= if defined?(@attributes) + ActiveModel::AttributeMutationTracker.new(@attributes) + else + NullMutationTracker.instance end - - @mutations_from_database ||= ActiveModel::AttributeMutationTracker.new(@attributes) end def forget_attribute_assignments @@ -267,45 +272,68 @@ module ActiveModel @mutations_before_last_save ||= ActiveModel::NullMutationTracker.instance end + def cache_changed_attributes + @cached_changed_attributes = changed_attributes + yield + ensure + clear_changed_attributes_cache + end + + def clear_changed_attributes_cache + remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) + end + + # Returns +true+ if attr_name is changed, +false+ otherwise. + def changes_include?(attr_name) + attributes_changed_by_setter.include?(attr_name) || mutations_from_database.changed?(attr_name) + end + alias attribute_changed_by_setter? changes_include? + + # Returns +true+ if attr_name were changed before the model was saved, + # +false+ otherwise. + def previous_changes_include?(attr_name) + previous_changes.include?(attr_name) + end + # Handles *_change for +method_missing+. def attribute_change(attr) - [attribute_was(attr), _read_attribute(attr)] if attribute_changed?(attr) + [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr) end # Handles *_previous_change for +method_missing+. def attribute_previous_change(attr) - mutations_before_last_save.change_to_attribute(attr) + previous_changes[attr] if attribute_previously_changed?(attr) end # Handles *_will_change! for +method_missing+. def attribute_will_change!(attr) - attr = attr.to_s - mutations_from_database.force_change(attr).tap do - @attributes[attr] if defined?(@_pseudo_attributes) + unless attribute_changed?(attr) + begin + value = _read_attribute(attr) + value = value.duplicable? ? value.clone : value + rescue TypeError, NoMethodError + end + + set_attribute_was(attr, value) end + mutations_from_database.force_change(attr) end # Handles restore_*! for +method_missing+. def restore_attribute!(attr) if attribute_changed?(attr) - __send__("#{attr}=", attribute_was(attr)) + __send__("#{attr}=", changed_attributes[attr]) clear_attribute_changes([attr]) end end - def _prepare_changes - if defined?(@_pseudo_attributes) - changed.each do |attr| - @attributes.write_from_user(attr, _read_attribute(attr)) - end - end + def attributes_changed_by_setter + @attributes_changed_by_setter ||= ActiveSupport::HashWithIndifferentAccess.new end - def _clone_attribute(attr) - value = _read_attribute(attr) - value.duplicable? ? value.clone : value - rescue TypeError, NoMethodError - value + # Force an attribute to have a particular "before" value + def set_attribute_was(attr, old_value) + attributes_changed_by_setter[attr] = old_value end end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index df4c79b0f6..3de6fe566d 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -32,7 +32,9 @@ module ActiveRecord # reload the record and clears changed attributes. def reload(*) super.tap do + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new @mutations_from_database = nil end end @@ -112,12 +114,12 @@ module ActiveRecord # Alias for +changed+ def changed_attribute_names_to_save - mutations_from_database.changed_attribute_names + changes_to_save.keys end # Alias for +changed_attributes+ def attributes_in_database - mutations_from_database.changed_values + changes_to_save.transform_values(&:first) end private diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index d4f4a5887a..6ec477c7f3 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -374,6 +374,7 @@ module ActiveRecord became.send(:initialize) became.instance_variable_set("@attributes", @attributes) became.instance_variable_set("@mutations_from_database", @mutations_from_database) if defined?(@mutations_from_database) + became.instance_variable_set("@changed_attributes", attributes_changed_by_setter) became.instance_variable_set("@new_record", new_record?) became.instance_variable_set("@destroyed", destroyed?) became.errors.copy!(errors) -- cgit v1.2.3