Optimize ActiveSupport::Cache::Entry to reduce overhead #7800

Merged
merged 1 commit into from Sep 30, 2012

Projects

None yet

7 participants

@bdurand
bdurand commented Sep 30, 2012

This code provides a more efficient implementation of ActiveSupport::Cache::Entry that reduces the costs of serialization.

@fxn
Member
fxn commented Sep 30, 2012

Awesome work, thanks Brian!

@fxn fxn merged commit 3b97f6d into rails:master Sep 30, 2012
@exviva exviva commented on the diff Sep 30, 2012
activesupport/test/caching_test.rb
@@ -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)
@exviva
exviva Sep 30, 2012

I''m not using minitest, so I'm not sure, but won't this never fail? I thought assert checks if the first argument evaluates to false (and this case a string never would).

@steveklabnik
steveklabnik Sep 30, 2012 Ruby on Rails member

http://bfts.rubyforge.org/minitest/MiniTest/Assertions.html#method-i-assert

You would seem to be right, the message would go second.

@steveklabnik
steveklabnik Sep 30, 2012 Ruby on Rails member
@rafaelfranca
Member

This pull request broke the build, please fix it https://travis-ci.org/#!/rails/rails/jobs/2615327

@steveklabnik
Member

Looking, but since it wasn't my commit, might be rough...

@steveklabnik
Member

@bdurand any ideas?

@steveklabnik steveklabnik added a commit to steveklabnik/rails that referenced this pull request Oct 1, 2012
@steveklabnik steveklabnik fix cache store test
Pull #7800 broke the build, this should fix it.
55eba0a
@steveklabnik steveklabnik referenced this pull request Oct 1, 2012
Merged

fix cache store test #7810

@steveklabnik
Member

We fixed the other build errors, but https://travis-ci.org/#!/rails/rails/jobs/2618855 is still failing.

@bdurand
bdurand commented Oct 1, 2012

Sorry about that. The fixes for the failing tests are located here:

#7812

@steveklabnik
Member

Thanks. I tried to look into it, but since I wasn't that familiar with the code, I gave up pretty quick.

@josh

Since :expires_in is an optional value, we need to deal with it being nil here.

nil can't be coerced into Float
@ahoward

taking this out is a mistake because now store implementations MUST do lots of extra work.

def read_entry(key, options = nil)
   record = my_custom_bakend_find(key)

   value = deserialize(record.value)

   entry = Entry.new(value, options)
end

because the class level factory we get to take the object we just read from the cache, deserialize it, the go ahead and re-marshal and serialize it for fun because that's what the initializer does and we don't have another factory for building objects that doesn't do extra work.

to work around i have code that reads like this

      def read_entry(key, options = {})
        expires_at = Time.now.utc.to_i
        doc        = collection.find(_id: key, expires_at: {'$gt' = > expires_at}).first

        value      = Marshal.load(doc['value'].to_s)
        created_at = doc.created_at.to_f

        compressed = options[:compressed]
        expires_in = options[:expires_in]

        ActiveSupport::Cache::Entry.allocate.tap do |entry|       
          is_rails3 = entry.instance_variable_defined?('@value')

          if is_rails3
            burn_up_the_cpu_for_fun = Marshal.dump(value)

            entry.instance_variable_set(:@value, burn_up_the_cpu_for_fun)
            entry.instance_variable_set(:@compressed, false)
            entry.instance_variable_set(:@expires_in, expires_in)
            entry.instance_variable_set(:@created_at, created_at)
          else
            entry.instance_variable_set(:@v, value)
            entry.instance_variable_set(:@c, false)
            entry.instance_variable_set(:@x, expires_in)
          end
        end
      end 

which is now working around the poor cache interface in rails3 AND rails4. bletch.

more details on the kinds of work arounds 3/4 and the new interface require: https://gist.github.com/ahoward/6923703

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment