Permalink
Browse files

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`
  • Loading branch information...
1 parent 78e7a0d commit 08576b94ad4f19dfc368619d7751e211d23dcad8 @sgrif sgrif committed Nov 18, 2014
@@ -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? })
- attrs = mapping.collect {|key, _| read_attribute(key)}
+ if @aggregation_cache[name].nil? && (!allow_nil || mapping.any? {|key, _| !_read_attribute(key).nil? })
+ attrs = mapping.collect {|key, _| _read_attribute(key)}
object = constructor.respond_to?(:call) ?
constructor.call(*attrs) :
class_name.constantize.send(constructor, *attrs)
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -27,8 +27,8 @@ def keys
attributes.initialized_keys
end
- def fetch_value(name, &block)
- self[name].value(&block)
+ def fetch_value(name)
+ self[name].value { |n| yield n if block_given? }
end
def write_from_database(name, value)
@@ -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])
@@ -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
@@ -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
@@ -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

0 comments on commit 08576b9

Please sign in to comment.