Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PERF: Recover changes_applied performance #31698

Merged
merged 1 commit into from Jan 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions activemodel/lib/active_model/attribute_mutation_tracker.rb
Expand Up @@ -35,6 +35,10 @@ def change_to_attribute(attr_name)
end
end

def changed_attribute_names
attr_names.select { |attr| changed?(attr) }
end

def any_changes?
attr_names.any? { |attr| changed?(attr) }
end
Expand Down Expand Up @@ -109,8 +113,5 @@ def forget_change(*)

def original_value(*)
end

def force_change(*)
end
end
end
106 changes: 39 additions & 67 deletions activemodel/lib/active_model/dirty.rb
Expand Up @@ -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
Expand Down Expand Up @@ -142,9 +143,8 @@ def initialize_dup(other) # :nodoc:
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
Expand All @@ -155,7 +155,7 @@ def changes_applied # :nodoc:
# 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.
Expand All @@ -164,24 +164,24 @@ def changed?
# 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.
Expand All @@ -191,15 +191,12 @@ def restore_attributes(attributes = changed)

# 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
Expand All @@ -212,13 +209,7 @@ def clear_attribute_changes(attr_names)
# 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
Expand All @@ -228,9 +219,8 @@ def changed_attributes
# 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.
Expand All @@ -240,8 +230,7 @@ def changes
# 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:
Expand All @@ -257,11 +246,17 @@ def mutations_from_database
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
Expand All @@ -272,68 +267,45 @@ def mutations_before_last_save
@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
6 changes: 2 additions & 4 deletions activerecord/lib/active_record/attribute_methods/dirty.rb
Expand Up @@ -32,9 +32,7 @@ module Dirty
# <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
Expand Down Expand Up @@ -114,12 +112,12 @@ def changes_to_save

# 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
Expand Down
1 change: 0 additions & 1 deletion activerecord/lib/active_record/persistence.rb
Expand Up @@ -362,7 +362,6 @@ def becomes(klass)
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)
Expand Down