Permalink
Browse files

Merge pull request #7800 from bdurand/optimize_cache_entry_take_2

Optimize ActiveSupport::Cache::Entry to reduce overhead
  • Loading branch information...
2 parents da27fa1 + e3a746b commit 3b97f6d4850b79a22d0e573cedfc1abc42724659 @fxn fxn committed Sep 30, 2012
View
2 activesupport/CHANGELOG.md
@@ -237,4 +237,6 @@
* Remove deprecated ActiveSupport::JSON::Variable. *Erich Menge*
+* Optimize ActiveSupport::Cache::Entry to reduce memory and processing overhead. *Brian Durand*
+
Please check [3-2-stable](https://github.com/rails/rails/blob/3-2-stable/activesupport/CHANGELOG.md) for previous changes.
View
148 activesupport/lib/active_support/cache.rb
@@ -284,7 +284,9 @@ def fetch(name, options = nil)
end
if entry && entry.expired?
race_ttl = options[:race_condition_ttl].to_i
- if race_ttl and Time.now.to_f - entry.expires_at <= race_ttl
+ if race_ttl && (Time.now - entry.expires_at <= race_ttl)
+ # When an entry has :race_condition_ttl defined, put the stale entry back into the cache
+ # for a brief period while the entry is begin recalculated.
entry.expires_at = Time.now + race_ttl
write_entry(key, entry, :expires_in => race_ttl * 2)
else
@@ -532,102 +534,122 @@ def log(operation, key, options = nil)
end
end
- # Entry that is put into caches. It supports expiration time on entries and
- # can compress values to save space in the cache.
- class Entry
- attr_reader :created_at, :expires_in
-
+ # This class is used to represent cache entries. Cache entries have a value and an optional
+ # expiration time. The expiration time is used to support the :race_condition_ttl option
+ # on the cache.
+ #
+ # Since cache entries in most instances will be serialized, the internals of this class are highly optimized
+ # using short instance variable names that are lazily defined.
+ class Entry # :nodoc:
DEFAULT_COMPRESS_LIMIT = 16.kilobytes
- class << self
- # Create an entry with internal attributes set. This method is intended
- # to be used by implementations that store cache entries in a native
- # format instead of as serialized Ruby objects.
- def create(raw_value, created_at, options = {})
- entry = new(nil)
- entry.instance_variable_set(:@value, raw_value)
- entry.instance_variable_set(:@created_at, created_at.to_f)
- entry.instance_variable_set(:@compressed, options[:compressed])
- entry.instance_variable_set(:@expires_in, options[:expires_in])
- entry
- end
- end
-
# Create a new cache entry for the specified value. Options supported are
# +:compress+, +:compress_threshold+, and +:expires_in+.
def initialize(value, options = {})
- @compressed = false
- @expires_in = options[:expires_in]
- @expires_in = @expires_in.to_f if @expires_in
- @created_at = Time.now.to_f
- if value.nil?
- @value = nil
+ if should_compress?(value, options)
+ @v = compress(value)
+ @c = true
else
- @value = Marshal.dump(value)
- if should_compress?(@value, options)
- @value = Zlib::Deflate.deflate(@value)
- @compressed = true
- end
+ @v = value
end
- end
-
- # Get the raw value. This value may be serialized and compressed.
- def raw_value
- @value
- end
-
- # Get the value stored in the cache.
- def value
- # If the original value was exactly false @value is still true because
- # it is marshalled and eventually compressed. Both operations yield
- # strings.
- if @value
- Marshal.load(compressed? ? Zlib::Inflate.inflate(@value) : @value)
+ if expires_in = options[:expires_in]
+ @x = (Time.now + expires_in).to_i
end
end
- def compressed?
- @compressed
+ def value
+ convert_version_3_entry! if defined?(@value)
+ compressed? ? uncompress(@v) : @v
end
# Check if the entry is expired. The +expires_in+ parameter can override
# the value set when the entry was created.
def expired?
- @expires_in && @created_at + @expires_in <= Time.now.to_f
- end
-
- # Set a new time when the entry will expire.
- def expires_at=(time)
- if time
- @expires_in = time.to_f - @created_at
+ convert_version_3_entry! if defined?(@value)
+ if defined?(@x)
+ @x && @x < Time.now.to_i
else
- @expires_in = nil
+ false
end
end
- # Seconds since the epoch when the entry will expire.
def expires_at
- @expires_in ? @created_at + @expires_in : nil
+ Time.at(@x) if defined?(@x)
+ end
+
+ def expires_at=(value)
+ @x = value.to_i
end
# Returns the size of the cached value. This could be less than
# <tt>value.size</tt> if the data is compressed.
def size
- if @value.nil?
- 0
+ if defined?(@s)
+ @s
else
- @value.bytesize
+ case value
+ when NilClass
+ 0
+ when String
+ value.bytesize
+ else
+ @s = Marshal.dump(value).bytesize
+ end
+ end
+ end
+
+ # Duplicate the value in a class. This is used by cache implementations that don't natively
+ # serialize entries to protect against accidental cache modifications.
+ def dup_value!
+ convert_version_3_entry! if defined?(@value)
+ if @v && !compressed? && !(@v.is_a?(Numeric) || @v == true || @v == false)
+ if @v.is_a?(String)
+ @v = @v.dup
+ else
+ @v = Marshal.load(Marshal.dump(@v))
+ end
end
end
private
- def should_compress?(serialized_value, options)
- if options[:compress]
+ def should_compress?(value, options)
+ if value && options[:compress]
compress_threshold = options[:compress_threshold] || DEFAULT_COMPRESS_LIMIT
- return true if serialized_value.size >= compress_threshold
+ serialized_value_size = (value.is_a?(String) ? value : Marshal.dump(value)).bytesize
+ return true if serialized_value_size >= compress_threshold
end
false
end
+
+ def compressed?
+ defined?(@c) ? @c : false
+ end
+
+ def compress(value)
+ Zlib::Deflate.deflate(Marshal.dump(value))
+ end
+
+ def uncompress(value)
+ Marshal.load(Zlib::Inflate.inflate(value))
+ end
+
+ # The internals of this method changed between Rails 3.x and 4.0. This method provides the glue
+ # to ensure that cache entries created under the old version still work with the new class definition.
+ def convert_version_3_entry!
+ if defined?(@value)
+ @v = @value
+ remove_instance_variable(:@value)
+ end
+ if defined?(@compressed)
+ @c = @compressed
+ remove_instance_variable(:@compressed)
+ end
+ if defined?(@expires_in) && defined?(@created_at)
+ @x = (@created_at + @expires_in).to_i
+ remove_instance_variable(:@created_at)
+ remove_instance_variable(:@expires_in)
+ end
+ end
end
end
end
View
1 activesupport/lib/active_support/cache/memory_store.rb
@@ -135,6 +135,7 @@ def read_entry(key, options) # :nodoc:
end
def write_entry(key, entry, options) # :nodoc:
+ entry.dup_value!
synchronize do
old_entry = @data[key]
return false if @data.key?(key) && options[:unless_exist]
View
91 activesupport/test/caching_test.rb
@@ -224,25 +224,22 @@ def test_read_multi
end
def test_read_multi_with_expires
- @cache.write('foo', 'bar', :expires_in => 0.001)
+ time = Time.now
+ @cache.write('foo', 'bar', :expires_in => 10)
@cache.write('fu', 'baz')
@cache.write('fud', 'biz')
- sleep(0.002)
+ Time.stubs(:now).returns(time + 11)
assert_equal({"fu" => "baz"}, @cache.read_multi('foo', 'fu'))
end
def test_read_and_write_compressed_small_data
@cache.write('foo', 'bar', :compress => true)
- raw_value = @cache.send(:read_entry, 'foo', {}).raw_value
assert_equal 'bar', @cache.read('foo')
- assert_equal 'bar', Marshal.load(raw_value)
end
def test_read_and_write_compressed_large_data
@cache.write('foo', 'bar', :compress => true, :compress_threshold => 2)
- raw_value = @cache.send(:read_entry, 'foo', {}).raw_value
assert_equal 'bar', @cache.read('foo')
- assert_equal 'bar', Marshal.load(Zlib::Inflate.inflate(raw_value))
end
def test_read_and_write_compressed_nil
@@ -301,14 +298,6 @@ def test_delete
assert !@cache.exist?('foo')
end
- def test_read_should_return_a_different_object_id_each_time_it_is_called
- @cache.write('foo', 'bar')
- assert_not_equal @cache.read('foo').object_id, @cache.read('foo').object_id
- value = @cache.read('foo')
- value << 'bingo'
- assert_not_equal value, @cache.read('foo')
- end
-
def test_original_store_objects_should_not_be_immutable
bar = 'bar'
@cache.write('foo', bar)
@@ -363,7 +352,7 @@ def test_race_condition_protection_is_safe
rescue ArgumentError
end
assert_equal "bar", @cache.read('foo')
- Time.stubs(:now).returns(time + 71)
+ Time.stubs(:now).returns(time + 91)
assert_nil @cache.read('foo')
end
@@ -646,9 +635,9 @@ def test_prune_size
@cache.prune(@record_size * 3)
assert @cache.exist?(5)
assert @cache.exist?(4)
- assert !@cache.exist?(3)
+ assert "no entry", !@cache.exist?(3)
assert @cache.exist?(2)
- assert !@cache.exist?(1)
+ assert "no entry", !@cache.exist?(1)
end
def test_prune_size_on_write
@@ -670,12 +659,12 @@ def test_prune_size_on_write
assert @cache.exist?(9)
assert @cache.exist?(8)
assert @cache.exist?(7)
- assert !@cache.exist?(6)
- assert !@cache.exist?(5)
+ assert "no entry", !@cache.exist?(6)
+ assert "no entry", !@cache.exist?(5)
assert @cache.exist?(4)
- assert !@cache.exist?(3)
+ assert "no entry", !@cache.exist?(3)
assert @cache.exist?(2)
- assert !@cache.exist?(1)
+ assert "no entry", !@cache.exist?(1)
end
def test_pruning_is_capped_at_a_max_time
@@ -764,6 +753,14 @@ def test_local_cache_raw_values_with_marshal
assert_equal [], cache.read("foo")
end
end
+
+ def test_read_should_return_a_different_object_id_each_time_it_is_called
+ @cache.write('foo', 'bar')
+ assert_not_equal @cache.read('foo').object_id, @cache.read('foo').object_id
+ value = @cache.read('foo')
+ value << 'bingo'
+ assert_not_equal value, @cache.read('foo')
+ end
end
class NullStoreTest < ActiveSupport::TestCase
@@ -844,15 +841,6 @@ def test_mute_logging
end
class CacheEntryTest < ActiveSupport::TestCase
- def test_create_raw_entry
- time = Time.now
- entry = ActiveSupport::Cache::Entry.create("raw", time, :compress => false, :expires_in => 300)
- assert_equal "raw", entry.raw_value
- assert_equal time.to_f, entry.created_at
- assert !entry.compressed?
- assert_equal 300, entry.expires_in
- end
-
def test_expired
entry = ActiveSupport::Cache::Entry.new("value")
assert !entry.expired?, 'entry not expired'
@@ -864,16 +852,43 @@ def test_expired
end
def test_compress_values
- entry = ActiveSupport::Cache::Entry.new("value", :compress => true, :compress_threshold => 1)
- assert_equal "value", entry.value
- assert entry.compressed?
- assert_equal "value", Marshal.load(Zlib::Inflate.inflate(entry.raw_value))
+ value = "value" * 100
+ entry = ActiveSupport::Cache::Entry.new(value, :compress => true, :compress_threshold => 1)
+ assert_equal value, entry.value
+ assert "value is compressed", (value.bytesize > entry.size)
end
def test_non_compress_values
- entry = ActiveSupport::Cache::Entry.new("value")
- assert_equal "value", entry.value
- assert_equal "value", Marshal.load(entry.raw_value)
- assert !entry.compressed?
+ value = "value" * 100
+ entry = ActiveSupport::Cache::Entry.new(value)
+ assert_equal value, entry.value
+ assert_equal value.bytesize, entry.size
+ end
+
+ def test_restoring_version_3_entries
+ version_3_entry = ActiveSupport::Cache::Entry.allocate
+ version_3_entry.instance_variable_set(:@value, "hello")
+ version_3_entry.instance_variable_set(:@created_at, Time.now - 60)
+ entry = Marshal.load(Marshal.dump(version_3_entry))
+ assert_equal "hello", entry.value
+ assert_equal false, entry.expired?
+ end
+
+ def test_restoring_compressed_version_3_entries
+ version_3_entry = ActiveSupport::Cache::Entry.allocate
+ version_3_entry.instance_variable_set(:@value, Zlib::Deflate.deflate(Marshal.dump("hello")))
+ version_3_entry.instance_variable_set(:@compressed, true)
+ entry = Marshal.load(Marshal.dump(version_3_entry))
+ assert_equal "hello", entry.value
+ end
+
+ def test_restoring_expired_version_3_entries
+ version_3_entry = ActiveSupport::Cache::Entry.allocate
+ version_3_entry.instance_variable_set(:@value, "hello")
+ version_3_entry.instance_variable_set(:@created_at, Time.now - 60)
+ version_3_entry.instance_variable_set(:@expires_in, 58.9)
+ entry = Marshal.load(Marshal.dump(version_3_entry))
+ assert_equal "hello", entry.value
+ assert_equal true, entry.expired?
end
end

0 comments on commit 3b97f6d

Please sign in to comment.