aboutsummaryrefslogtreecommitdiffstats
path: root/activemodel
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2018-01-22 10:46:36 +0900
committerGitHub <noreply@github.com>2018-01-22 10:46:36 +0900
commita19e91f0fab13cca61acdb1f33e27be2323b9786 (patch)
tree232542abf240f11d86d0dbb4b4d3849b28eba650 /activemodel
parentcf1c48478d1f48d763c3bee92d6bc6cfb3e63dba (diff)
downloadrails-a19e91f0fab13cca61acdb1f33e27be2323b9786.tar.gz
rails-a19e91f0fab13cca61acdb1f33e27be2323b9786.tar.bz2
rails-a19e91f0fab13cca61acdb1f33e27be2323b9786.zip
PERF: Recover `changes_applied` performance (#31698)
#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 ```
Diffstat (limited to 'activemodel')
-rw-r--r--activemodel/lib/active_model/attribute_mutation_tracker.rb7
-rw-r--r--activemodel/lib/active_model/dirty.rb106
2 files changed, 43 insertions, 70 deletions
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 <tt>*_changed?</tt> 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 <tt>*_was</tt> 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 <tt>*_previously_changed?</tt> 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 <tt>*_change</tt> 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 <tt>*_previous_change</tt> 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 <tt>*_will_change!</tt> 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 <tt>restore_*!</tt> 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