Skip to content

Commit

Permalink
Merge pull request #41882 from Shopify/local-store-dup-value
Browse files Browse the repository at this point in the history
Refactor LocalCache to avoid calling Marshal.dump as much
  • Loading branch information
byroot committed Apr 10, 2021
2 parents 9904926 + 084ba0e commit 466a54f
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 15 deletions.
12 changes: 5 additions & 7 deletions activesupport/lib/active_support/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -845,9 +845,7 @@ def expires_at=(value)
end
end

# Returns the size of the cached value. This could be less than
# <tt>value.bytesize</tt> if the data is compressed.
def bytesize
def bytesize # :nodoc:
case value
when NilClass
0
Expand All @@ -858,6 +856,10 @@ def bytesize
end
end

def compressed? # :nodoc:
defined?(@compressed)
end

# Duplicates 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!
Expand Down Expand Up @@ -893,10 +895,6 @@ def compress!(compress_threshold)
end
end

def compressed?
defined?(@compressed)
end

def uncompress(value)
Marshal.load(Zlib::Inflate.inflate(value))
end
Expand Down
71 changes: 63 additions & 8 deletions activesupport/lib/active_support/cache/strategy/local_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,65 @@ def self.cache_for(l); instance.cache_for l; end
# Simple memory backed cache. This cache is not thread safe and is intended only
# for serving as a temporary memory cache for a single thread.
class LocalStore < Store
class Entry # :nodoc:
class << self
def build(cache_entry)
return if cache_entry.nil?
return cache_entry if cache_entry.compressed?

value = cache_entry.value
if value.is_a?(String)
DupableEntry.new(cache_entry)
elsif !value || value == true || value.is_a?(Numeric)
new(cache_entry)
else
MutableEntry.new(cache_entry)
end
end
end

attr_reader :value, :version, :expires_at

def initialize(cache_entry)
@value = cache_entry.value
@expires_at = cache_entry.expires_at
@version = cache_entry.version
end

def mismatched?(version)
@version && version && @version != version
end

def expired?
expires_at && expires_at <= Time.now.to_f
end
end

class DupableEntry < Entry # :nodoc:
def initialize(_cache_entry)
super
unless @value.frozen?
@value = @value.dup.freeze
end
end

def value
@value.dup
end
end

class MutableEntry < Entry # :nodoc:
def initialize(cache_entry)
@payload = Marshal.dump(cache_entry.value)
@expires_at = cache_entry.expires_at
@version = cache_entry.version
end

def value
Marshal.load(@payload)
end
end

def initialize
super
@data = {}
Expand Down Expand Up @@ -65,8 +124,7 @@ def read_multi_entries(keys, **options)
end

def write_entry(key, entry, **options)
entry.dup_value!
@data[key] = entry
@data[key] = Entry.build(entry)
true
end

Expand All @@ -75,10 +133,7 @@ def delete_entry(key, **options)
end

def fetch_entry(key, options = nil) # :nodoc:
entry = @data.fetch(key) { @data[key] = yield }
dup_entry = entry.dup
dup_entry&.dup_value!
dup_entry
@data.fetch(key) { @data[key] = Entry.build(yield) }
end
end

Expand Down Expand Up @@ -131,12 +186,12 @@ def decrement(name, amount = 1, **options) # :nodoc:
def read_entry(key, **options)
if cache = local_cache
hit = true
value = cache.fetch_entry(key) do
entry = cache.fetch_entry(key) do
hit = false
super
end
options[:event][:store] = cache.class.name if hit && options[:event]
value
entry
else
super
end
Expand Down

0 comments on commit 466a54f

Please sign in to comment.