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.

Author: sgrif

activerecord/lib/active_record/attribute.rb

@@ -5,8 +5,8 @@ def from_database(name, value, type)
         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 @@ def uninitialized(name, type)
 
     # 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 @@ def value
       @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 @@ def with_cast_value(value)
     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 @@ def hash
 
     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:

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 @@ def type_cast(value)
         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

activerecord/lib/active_record/attribute_methods/dirty.rb

@@ -41,13 +41,15 @@ def reload(*)
 
       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 @@ def keys_for_partial_write
       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

activerecord/lib/active_record/attribute_mutation_tracker.rb

@@ -1,64 +1,48 @@
 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
 
     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:

activerecord/test/cases/attribute_methods_test.rb

@@ -542,9 +542,6 @@ def test_converted_values_are_returned_after_assignment
 
     developer.save!
 
-    assert_equal "50000", developer.salary_before_type_cast
-    assert_equal 1337, developer.name_before_type_cast
-
     assert_equal 50000, developer.salary
     assert_equal "1337", developer.name
   end

activerecord/test/cases/attribute_test.rb

@@ -182,12 +182,49 @@ def assert_valid_value(*)
       assert attribute.has_been_read?
     end
 
+    test "an attribute is not changed if it hasn't been assigned or mutated" do
+      attribute = Attribute.from_database(:foo, 1, Type::Value.new)
+
+      refute attribute.changed?
+    end
+
+    test "an attribute is changed if it's been assigned a new value" do
+      attribute = Attribute.from_database(:foo, 1, Type::Value.new)
+      changed = attribute.with_value_from_user(2)
+
+      assert changed.changed?
+    end
+
+    test "an attribute is not changed if it's assigned the same value" do
+      attribute = Attribute.from_database(:foo, 1, Type::Value.new)
+      unchanged = attribute.with_value_from_user(1)
+
+      refute unchanged.changed?
+    end
+
     test "an attribute can not be mutated if it has not been read,
       and skips expensive calculations" do
       type_which_raises_from_all_methods = Object.new
       attribute = Attribute.from_database(:foo, "bar", type_which_raises_from_all_methods)
 
-      assert_not attribute.changed_in_place_from?("bar")
+      assert_not attribute.changed_in_place?
+    end
+
+    test "an attribute is changed if it has been mutated" do
+      attribute = Attribute.from_database(:foo, "bar", Type::String.new)
+      attribute.value << "!"
+
+      assert attribute.changed_in_place?
+      assert attribute.changed?
+    end
+
+    test "an attribute can forget its changes" do
+      attribute = Attribute.from_database(:foo, "bar", Type::String.new)
+      changed = attribute.with_value_from_user("foo")
+      forgotten = changed.forgetting_assignment
+
+      assert changed.changed? # sanity check
+      refute forgotten.changed?
     end
 
     test "with_value_from_user validates the value" do