Skip to content
Browse files

Only cache attributes which need it for performance reasons. Closes #…

…9767 [skaes]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7752 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent b96c298 commit 4db718e2bb514be7a2e76e56cb5027c4007528b4 @NZKoz NZKoz committed
Showing with 75 additions and 1 deletion.
  1. +24 −1 activerecord/lib/active_record/attribute_methods.rb
  2. +51 −0 activerecord/test/attribute_methods_test.rb
View
25 activerecord/lib/active_record/attribute_methods.rb
@@ -1,10 +1,13 @@
module ActiveRecord
module AttributeMethods #:nodoc:
DEFAULT_SUFFIXES = %w(= ? _before_type_cast)
+ ATTRIBUTE_TYPES_CACHED_BY_DEFAULT = [:datetime, :timestamp, :time, :date]
def self.included(base)
base.extend ClassMethods
base.attribute_method_suffix *DEFAULT_SUFFIXES
+ base.cattr_accessor :attribute_types_cached_by_default, :instance_writer => false
+ base.attribute_types_cached_by_default = ATTRIBUTE_TYPES_CACHED_BY_DEFAULT
end
# Declare and check for suffixed attribute methods.
@@ -88,6 +91,23 @@ def instance_method_already_implemented?(method_name)
alias :define_read_methods :define_attribute_methods
+ # +cache_attributes+ allows you to declare which converted attribute values should
+ # be cached. Usually caching only pays off for attributes with expensive conversion
+ # methods, like date columns (e.g. created_at, updated_at).
+ def cache_attributes(*attribute_names)
+ attribute_names.each {|attr| cached_attributes << attr.to_s}
+ end
+
+ # returns the attributes where
+ def cached_attributes
+ @cached_attributes ||=
+ columns.select{|c| attribute_types_cached_by_default.include?(c.type)}.map(&:name).to_set
+ end
+
+ def cache_attribute?(attr_name)
+ cached_attributes.include?(attr_name)
+ end
+
private
# Suffixes a, ?, c become regexp /(a|\?|c)$/
def rebuild_attribute_method_regexp
@@ -109,7 +129,10 @@ def define_read_method(symbol, attr_name, column)
access_code = access_code.insert(0, "missing_attribute('#{attr_name}', caller) unless @attributes.has_key?('#{attr_name}'); ")
end
- evaluate_attribute_method attr_name, "def #{symbol}; @attributes_cache['#{attr_name}'] ||= begin; #{access_code}; end; end"
+ if cache_attribute?(attr_name)
+ access_code = "@attributes_cache['#{attr_name}'] ||= (#{access_code})"
+ end
+ evaluate_attribute_method attr_name, "def #{symbol}; #{access_code}; end"
end
# Define read method for serialized attribute.
View
51 activerecord/test/attribute_methods_test.rb
@@ -2,6 +2,7 @@
require 'fixtures/topic'
class AttributeMethodsTest < Test::Unit::TestCase
+ fixtures :topics
def setup
@old_suffixes = ActiveRecord::Base.send(:attribute_method_suffixes).dup
@target = Class.new(ActiveRecord::Base)
@@ -92,4 +93,54 @@ def test_raises_dangerous_attribute_error_when_defining_activerecord_method_in_m
end
end
end
+
+ def test_only_time_related_columns_are_meant_to_be_cached_by_default
+ expected = %w(datetime timestamp time date).sort
+ assert_equal expected, ActiveRecord::Base.attribute_types_cached_by_default.map(&:to_s).sort
+end
+
+ def test_declaring_attributes_as_cached_adds_them_to_the_attributes_cached_by_default
+ default_attributes = Topic.cached_attributes
+ Topic.cache_attributes :replies_count
+ expected = default_attributes + ["replies_count"]
+ assert_equal expected.sort, Topic.cached_attributes.sort
+ Topic.instance_variable_set "@cached_attributes", nil
+ end
+
+ def test_time_related_columns_are_actually_cached
+ column_types = %w(datetime timestamp time date).map(&:to_sym)
+ column_names = Topic.columns.select{|c| column_types.include?(c.type) }.map(&:name)
+
+ assert_equal column_names.sort, Topic.cached_attributes.sort
+ assert_equal time_related_columns_on_topic.sort, Topic.cached_attributes.sort
+ end
+
+ def test_accessing_cached_attributes_caches_the_converted_values_and_nothing_else
+ t = topics(:first)
+ cache = t.instance_variable_get "@attributes_cache"
+
+ assert_not_nil cache
+ assert cache.empty?
+
+ all_columns = Topic.columns.map(&:name)
+ cached_columns = time_related_columns_on_topic
+ uncached_columns = all_columns - cached_columns
+
+ all_columns.each do |attr_name|
+ attribute_gets_cached = Topic.cache_attribute?(attr_name)
+ val = t.send attr_name unless attr_name == "type"
+ if attribute_gets_cached
+ assert cached_columns.include?(attr_name)
+ assert_equal val, cache[attr_name]
+ else
+ assert uncached_columns.include?(attr_name)
+ assert !cache.include?(attr_name)
+ end
+ end
+ end
+
+ private
+ def time_related_columns_on_topic
+ Topic.columns.select{|c| [:time, :date, :datetime, :timestamp].include?(c.type)}.map(&:name)
+ end
end

0 comments on commit 4db718e

Please sign in to comment.
Something went wrong with that request. Please try again.