From 07723c23a7dc570beae73c074ad37227e3e8a06e Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 28 Sep 2015 17:12:28 -0400 Subject: Further encapsulate dirty checking on `Attribute` We can skip the allocation of a full `AttributeSet` by changing the semantics of how we structure things. Instead of comparing two separate `AttributeSet` objects, and `Attribute` is now a singly linked list of every change that has happened to it. Since the attribute objects are immutable, to apply the changes we simply need to copy the head of the list. It's worth noting that this causes one subtle change in the behavior of AR. When a record is saved successfully, the `before_type_cast` version of everything will be what was sent to the database. I honestly think these semantics make more sense, as we could have just as easily had the DB do `RETURNING *` and updated the record with those if we had things like timestamps implemented at the DB layer. This brings our performance closer to 4.2, but we're still not quite there. --- activerecord/lib/active_record/attribute.rb | 54 ++++++++++++++++++---- .../attribute/user_provided_default.rb | 13 +----- .../lib/active_record/attribute_methods/dirty.rb | 11 +++-- .../active_record/attribute_mutation_tracker.rb | 30 +++--------- 4 files changed, 61 insertions(+), 47 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 74b44810d4..3c4c8f10ec 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -5,8 +5,8 @@ module ActiveRecord FromDatabase.new(name, value, type) end - def from_user(name, value, type) - FromUser.new(name, value, type) + def from_user(name, value, type, original_attribute = nil) + FromUser.new(name, value, type, original_attribute) end def with_cast_value(name, value, type) @@ -26,10 +26,11 @@ module ActiveRecord # This method should not be called directly. # Use #from_database or #from_user - def initialize(name, value_before_type_cast, type) + def initialize(name, value_before_type_cast, type, original_attribute = nil) @name = name @value_before_type_cast = value_before_type_cast @type = type + @original_attribute = original_attribute end def value @@ -38,21 +39,33 @@ module ActiveRecord @value end + def original_value + if assigned? + original_attribute.original_value + else + type_cast(value_before_type_cast) + end + end + def value_for_database type.serialize(value) end - def changed_from?(old_value) - type.changed?(old_value, value, value_before_type_cast) + def changed? + changed_from_assignment? || changed_in_place? + end + + def changed_in_place? + has_been_read? && type.changed_in_place?(original_value_for_database, value) end - def changed_in_place_from?(old_value) - has_been_read? && type.changed_in_place?(old_value, value) + def forgetting_assignment + with_value_from_database(value_for_database) end def with_value_from_user(value) type.assert_valid_value(value) - self.class.from_user(name, value, type) + self.class.from_user(name, value, type, self) end def with_value_from_database(value) @@ -64,7 +77,7 @@ module ActiveRecord end def with_type(type) - self.class.new(name, value_before_type_cast, type) + self.class.new(name, value_before_type_cast, type, original_attribute) end def type_cast(*) @@ -97,16 +110,39 @@ module ActiveRecord protected + attr_reader :original_attribute + alias_method :assigned?, :original_attribute + def initialize_dup(other) if defined?(@value) && @value.duplicable? @value = @value.dup end end + def changed_from_assignment? + assigned? && type.changed?(original_value, value, value_before_type_cast) + end + + def original_value_for_database + if assigned? + original_attribute.original_value_for_database + else + _original_value_for_database + end + end + + def _original_value_for_database + value_for_database + end + class FromDatabase < Attribute # :nodoc: def type_cast(value) type.deserialize(value) end + + def _original_value_for_database + value_before_type_cast + end end class FromUser < Attribute # :nodoc: diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activerecord/lib/active_record/attribute/user_provided_default.rb index 501590cf0e..fb8ad9163e 100644 --- a/activerecord/lib/active_record/attribute/user_provided_default.rb +++ b/activerecord/lib/active_record/attribute/user_provided_default.rb @@ -4,8 +4,7 @@ module ActiveRecord class Attribute # :nodoc: class UserProvidedDefault < FromUser def initialize(name, value, type, database_default) - super(name, value, type) - @database_default = database_default + super(name, value, type, database_default) end def type_cast(value) @@ -16,17 +15,9 @@ module ActiveRecord end end - def changed_from?(old_value) - super || changed_from?(database_default.value) - end - def with_type(type) - self.class.new(name, value_before_type_cast, type, database_default) + self.class.new(name, value_before_type_cast, type, original_attribute) end - - protected - - attr_reader :database_default end end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 43c15841c2..a2e4e0ac31 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -41,13 +41,15 @@ module ActiveRecord def init_internals super - @mutation_tracker = AttributeMutationTracker.new(@attributes, @attributes.dup) + @mutation_tracker = AttributeMutationTracker.new(@attributes) end def initialize_dup(other) # :nodoc: super - @mutation_tracker = AttributeMutationTracker.new(@attributes, - self.class._default_attributes.deep_dup) + @attributes = self.class._default_attributes.map do |attr| + attr.with_value_from_user(@attributes.fetch_value(attr.name)) + end + @mutation_tracker = AttributeMutationTracker.new(@attributes) end def changes_applied @@ -122,7 +124,8 @@ module ActiveRecord end def store_original_attributes - @mutation_tracker = @mutation_tracker.now_tracking(@attributes) + @attributes = @attributes.map(&:forgetting_assignment) + @mutation_tracker = AttributeMutationTracker.new(@attributes) end def previous_mutation_tracker diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activerecord/lib/active_record/attribute_mutation_tracker.rb index 08eb8ba7ac..ba7348918b 100644 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ b/activerecord/lib/active_record/attribute_mutation_tracker.rb @@ -1,14 +1,13 @@ module ActiveRecord class AttributeMutationTracker # :nodoc: - def initialize(attributes, original_attributes) + def initialize(attributes) @attributes = attributes - @original_attributes = original_attributes end def changed_values attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| if changed?(attr_name) - result[attr_name] = original_attributes.fetch_value(attr_name) + result[attr_name] = attributes[attr_name].original_value end end end @@ -16,49 +15,34 @@ module ActiveRecord def changes attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| if changed?(attr_name) - result[attr_name] = [original_attributes.fetch_value(attr_name), attributes.fetch_value(attr_name)] + result[attr_name] = [attributes[attr_name].original_value, attributes.fetch_value(attr_name)] end end end def changed?(attr_name) attr_name = attr_name.to_s - modified?(attr_name) || changed_in_place?(attr_name) + attributes[attr_name].changed? end def changed_in_place?(attr_name) - original_database_value = original_attributes[attr_name].value_before_type_cast - attributes[attr_name].changed_in_place_from?(original_database_value) + attributes[attr_name].changed_in_place? end def forget_change(attr_name) attr_name = attr_name.to_s - original_attributes[attr_name] = attributes[attr_name].dup - end - - def now_tracking(new_attributes) - AttributeMutationTracker.new(new_attributes, clean_copy_of(new_attributes)) + attributes[attr_name] = attributes[attr_name].forgetting_assignment end protected - attr_reader :attributes, :original_attributes + attr_reader :attributes private def attr_names attributes.keys end - - def modified?(attr_name) - attributes[attr_name].changed_from?(original_attributes.fetch_value(attr_name)) - end - - def clean_copy_of(attributes) - attributes.map do |attr| - attr.with_value_from_database(attr.value_for_database) - end - end end class NullMutationTracker # :nodoc: -- cgit v1.2.3