Optimize ActiveSupport::Cache::Entry to reduce memory and processing overhead.

4 files changed, 141 insertions, 101 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 2df3d1f69b..70d9ec5613 100644
--- a/activesupport/CHANGELOG.md
+++ b/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.
diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb
index f98ba16cdd..a681a8d6a9 100644
--- a/activesupport/lib/active_support/cache.rb
+++ b/activesupport/lib/active_support/cache.rb
@@ -284,7 +284,9 @@ module ActiveSupport
           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 @@ module ActiveSupport
         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
diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb
index 7fd5e3b53d..4d26fb7e42 100644
--- a/activesupport/lib/active_support/cache/memory_store.rb
+++ b/activesupport/lib/active_support/cache/memory_store.rb
@@ -135,6 +135,7 @@ module ActiveSupport
         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]
diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb
index 71cd9d81b3..3832f11754 100644
--- a/activesupport/test/caching_test.rb
+++ b/activesupport/test/caching_test.rb
@@ -224,25 +224,22 @@ module CacheStoreBehavior
   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 @@ module CacheStoreBehavior
     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 @@ module CacheStoreBehavior
     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 @@ class MemoryStoreTest < ActiveSupport::TestCase
     @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 @@ class MemoryStoreTest < ActiveSupport::TestCase
     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 @@ class MemCacheStoreTest < ActiveSupport::TestCase
       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 @@ class CacheStoreLoggerTest < ActiveSupport::TestCase
 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 @@ class CacheEntryTest < ActiveSupport::TestCase
   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