Improve the performance of reading attributes

We added a comparison to "id", and call to `self.class.primary_key` a
*lot*. We also have performance hits from `&block` all over the place.
We skip the check in a new method, in order to avoid breaking the
behavior of `read_attribute`

activerecord/lib/active_record/aggregations.rb

@@ -230,8 +230,8 @@ def composed_of(part_id, options = {})
       private
         def reader_method(name, class_name, mapping, allow_nil, constructor)
           define_method(name) do
-            if @aggregation_cache[name].nil? && (!allow_nil || mapping.any? {|key, _| !read_attribute(key).nil? })
+            if @aggregation_cache[name].nil? && (!allow_nil || mapping.any? {|key, _| !_read_attribute(key).nil? })
-              attrs = mapping.collect {|key, _| read_attribute(key)}
+              attrs = mapping.collect {|key, _| _read_attribute(key)}
               object = constructor.respond_to?(:call) ?
                 constructor.call(*attrs) :
                 class_name.constantize.send(constructor, *attrs)

activerecord/lib/active_record/associations/has_many_association.rb

@@ -66,7 +66,7 @@ def empty?
         # the loaded flag is set to true as well.
         def count_records
           count = if has_cached_counter?
-            owner.read_attribute cached_counter_attribute_name
+            owner._read_attribute cached_counter_attribute_name
           else
             scope.count
           end

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -20,7 +20,7 @@ def initialize(owner, reflection)
       # SELECT query will be generated by using #length instead.
       def size
         if has_cached_counter?
-          owner.read_attribute cached_counter_attribute_name(reflection)
+          owner._read_attribute cached_counter_attribute_name(reflection)
         elsif loaded?
           target.size
         else

activerecord/lib/active_record/attribute_methods.rb

@@ -332,7 +332,7 @@ def attribute_for_inspect(attr_name)
     #   task.attribute_present?(:title)   # => true
     #   task.attribute_present?(:is_done) # => true
     def attribute_present?(attribute)
-      value = read_attribute(attribute)
+      value = _read_attribute(attribute)
       !value.nil? && !(value.respond_to?(:empty?) && value.empty?)
     end
 
@@ -433,7 +433,7 @@ def pk_attribute?(name)
     end
 
     def typecasted_attribute_value(name)
-      read_attribute(name)
+      _read_attribute(name)
     end
   end
 end

activerecord/lib/active_record/attribute_methods/dirty.rb

@@ -110,7 +110,7 @@ def old_attribute_value(attr)
         if attribute_changed?(attr)
           changed_attributes[attr]
         else
-          clone_attribute_value(:read_attribute, attr)
+          clone_attribute_value(:_read_attribute, attr)
         end
       end
 

activerecord/lib/active_record/attribute_methods/primary_key.rb

@@ -17,7 +17,7 @@ def to_key
       def id
         if pk = self.class.primary_key
           sync_with_transaction_state
-          read_attribute(pk)
+          _read_attribute(pk)
         end
       end
 

activerecord/lib/active_record/attribute_methods/read.rb

@@ -27,7 +27,7 @@ def method_body(method_name, const_name)
           <<-EOMETHOD
           def #{method_name}
             name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{const_name}
-            read_attribute(name) { |n| missing_attribute(n, caller) }
+            _read_attribute(name) { |n| missing_attribute(n, caller) }
           end
           EOMETHOD
         end
@@ -64,7 +64,7 @@ def define_method_attribute(name)
             generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
               def #{temp_method}
                 name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name}
-                read_attribute(name) { |n| missing_attribute(n, caller) }
+                _read_attribute(name) { |n| missing_attribute(n, caller) }
               end
             STR
 
@@ -84,13 +84,19 @@ def #{temp_method}
       def read_attribute(attr_name, &block)
         name = attr_name.to_s
         name = self.class.primary_key if name == ID
-        @attributes.fetch_value(name, &block)
+        _read_attribute(name, &block)
+      end
+
+      # This method exists to avoid the expensive primary_key check internally, without
+      # breaking compatibility with the read_attribute API
+      def _read_attribute(attr_name) # :nodoc:
+        @attributes.fetch_value(attr_name.to_s) { |n| yield n if block_given? }
       end
 
       private
 
       def attribute(attribute_name)
-        read_attribute(attribute_name)
+        _read_attribute(attribute_name)
       end
     end
   end

activerecord/lib/active_record/attribute_set.rb

@@ -27,8 +27,8 @@ def keys
       attributes.initialized_keys
     end
 
-    def fetch_value(name, &block)
+    def fetch_value(name)
-      self[name].value(&block)
+      self[name].value { |n| yield n if block_given? }
     end
 
     def write_from_database(name, value)

activerecord/lib/active_record/enum.rb

@@ -142,7 +142,7 @@ def _enum_methods_module
             private
               def save_changed_attribute(attr_name, old)
                 if (mapping = self.class.defined_enums[attr_name.to_s])
-                  value = read_attribute(attr_name)
+                  value = _read_attribute(attr_name)
                   if attribute_changed?(attr_name)
                     if mapping[old] == value
                       clear_attribute_changes([attr_name])

activerecord/lib/active_record/reflection.rb

@@ -287,7 +287,7 @@ def initialize(name, scope, options, active_record)
       def association_scope_cache(conn, owner)
         key = conn.prepared_statements
         if polymorphic?
-          key = [key, owner.read_attribute(@foreign_type)]
+          key = [key, owner._read_attribute(@foreign_type)]
         end
         @association_scope_cache[key] ||= @scope_lock.synchronize {
           @association_scope_cache[key] ||= yield

activerecord/lib/active_record/validations/uniqueness.rb

@@ -79,7 +79,7 @@ def scope_relation(record, table, relation)
             scope_value = record.send(reflection.foreign_key)
             scope_item  = reflection.foreign_key
           else
-            scope_value = record.read_attribute(scope_item)
+            scope_value = record._read_attribute(scope_item)
           end
           relation = relation.and(table[scope_item].eq(scope_value))
         end

activerecord/test/cases/inheritance_test.rb

@@ -22,7 +22,7 @@ def test_class_with_blank_sti_name
     company = Company.first
     company = company.dup
     company.extend(Module.new {
-      def read_attribute(name)
+      def _read_attribute(name)
         return '  ' if name == 'type'
         super
       end