From a19e91f0fab13cca61acdb1f33e27be2323b9786 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 22 Jan 2018 10:46:36 +0900 Subject: PERF: Recover `changes_applied` performance (#31698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #30985 caused `object.save` performance regression since calling `changes` in `changes_applied` is very slow. We don't need to call the expensive method in `changes_applied` as long as `@attributes` is tracked by mutation tracker. https://gist.github.com/kamipo/1a9f4f3891803b914fc72ede98268aa2 Before: ``` Warming up -------------------------------------- create_string_columns 73.000 i/100ms Calculating ------------------------------------- create_string_columns 722.256 (± 5.8%) i/s - 3.650k in 5.073031s ``` After: ``` Warming up -------------------------------------- create_string_columns 96.000 i/100ms Calculating ------------------------------------- create_string_columns 950.224 (± 7.7%) i/s - 4.800k in 5.084837s ``` --- .../lib/active_model/attribute_mutation_tracker.rb | 7 +- activemodel/lib/active_model/dirty.rb | 106 ++++++++------------- 2 files changed, 43 insertions(+), 70 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_mutation_tracker.rb b/activemodel/lib/active_model/attribute_mutation_tracker.rb index c67e1b809a..f55613ecd5 100644 --- a/activemodel/lib/active_model/attribute_mutation_tracker.rb +++ b/activemodel/lib/active_model/attribute_mutation_tracker.rb @@ -35,6 +35,10 @@ 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 @@ -109,8 +113,5 @@ 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 d2ebd18107..0044fde6c5 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -3,6 +3,7 @@ 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 @@ -142,9 +143,8 @@ module ActiveModel end def changes_applied # :nodoc: - @previously_changed = changes + _prepare_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? - changed_attributes.present? + mutations_from_database.any_changes? 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 - changed_attributes.keys + mutations_from_database.changed_attribute_names end # Handles *_changed? for +method_missing+. def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: - !!changes_include?(attr) && + !!mutations_from_database.changed?(attr) && (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && - (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) + (from == OPTION_NOT_GIVEN || from == attribute_was(attr)) end # Handles *_was for +method_missing+. def attribute_was(attr) # :nodoc: - attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) + mutations_from_database.original_value(attr) end # Handles *_previously_changed? for +method_missing+. def attribute_previously_changed?(attr) #:nodoc: - previous_changes_include?(attr) + mutations_before_last_save.changed?(attr) end # Restore all previous data of the provided attributes. @@ -191,15 +191,12 @@ 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 @@ -212,13 +209,7 @@ module ActiveModel # person.name = 'robert' # person.changed_attributes # => {"name" => "bob"} def changed_attributes - # 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 + mutations_from_database.changed_values.freeze end # Returns a hash of changed attributes indicating their original @@ -228,9 +219,8 @@ module ActiveModel # person.name = 'bob' # person.changes # => { "name" => ["bill", "bob"] } def changes - cache_changed_attributes do - ActiveSupport::HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] - end + _prepare_changes + mutations_from_database.changes end # Returns a hash of attributes that were changed before the model was saved. @@ -240,8 +230,7 @@ module ActiveModel # person.save # person.previous_changes # => {"name" => ["bob", "robert"]} def previous_changes - @previously_changed ||= ActiveSupport::HashWithIndifferentAccess.new - @previously_changed.merge(mutations_before_last_save.changes) + mutations_before_last_save.changes end def attribute_changed_in_place?(attr_name) # :nodoc: @@ -257,11 +246,17 @@ module ActiveModel unless defined?(@mutations_from_database) @mutations_from_database = nil end - @mutations_from_database ||= if defined?(@attributes) - ActiveModel::AttributeMutationTracker.new(@attributes) - else - NullMutationTracker.instance + + 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) + } + ) end + + @mutations_from_database ||= ActiveModel::AttributeMutationTracker.new(@attributes) end def forget_attribute_assignments @@ -272,68 +267,45 @@ 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) - [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr) + [attribute_was(attr), _read_attribute(attr)] if attribute_changed?(attr) end # Handles *_previous_change for +method_missing+. def attribute_previous_change(attr) - previous_changes[attr] if attribute_previously_changed?(attr) + mutations_before_last_save.change_to_attribute(attr) end # Handles *_will_change! for +method_missing+. def attribute_will_change!(attr) - unless attribute_changed?(attr) - begin - value = _read_attribute(attr) - value = value.duplicable? ? value.clone : value - rescue TypeError, NoMethodError - end - - set_attribute_was(attr, value) + attr = attr.to_s + mutations_from_database.force_change(attr).tap do + @attributes[attr] if defined?(@_pseudo_attributes) end - mutations_from_database.force_change(attr) end # Handles restore_*! for +method_missing+. def restore_attribute!(attr) if attribute_changed?(attr) - __send__("#{attr}=", changed_attributes[attr]) + __send__("#{attr}=", attribute_was(attr)) clear_attribute_changes([attr]) end end - def attributes_changed_by_setter - @attributes_changed_by_setter ||= ActiveSupport::HashWithIndifferentAccess.new + def _prepare_changes + if defined?(@_pseudo_attributes) + changed.each do |attr| + @attributes.write_from_user(attr, _read_attribute(attr)) + end + end end - # Force an attribute to have a particular "before" value - def set_attribute_was(attr, old_value) - attributes_changed_by_setter[attr] = old_value + def _clone_attribute(attr) + value = _read_attribute(attr) + value.duplicable? ? value.clone : value + rescue TypeError, NoMethodError + value end end end -- cgit v1.2.3