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
```
This commit is contained in:
Ryuta Kamizono 2018-01-22 10:46:36 +09:00 committed by GitHub
parent cf1c48478d
commit a19e91f0fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 75 deletions

View File

@ -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

View File

@ -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

View File

@ -32,9 +32,7 @@ module ActiveRecord
# <tt>reload</tt> 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
@ -114,12 +112,12 @@ module ActiveRecord
# Alias for +changed+
def changed_attribute_names_to_save
changes_to_save.keys
mutations_from_database.changed_attribute_names
end
# Alias for +changed_attributes+
def attributes_in_database
changes_to_save.transform_values(&:first)
mutations_from_database.changed_values
end
private

View File

@ -362,7 +362,6 @@ module ActiveRecord
became = klass.new
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)