Permalink
Browse files

Merge pull request #11546 from swoop-inc/ss_memory_store_cache_size

[Fixes #11512] improves cache size calculation in MemoryStore
  • Loading branch information...
2 parents ecd7003 + 51d9b9a commit 782d2f6f83c39e9cfd2a8e650250a74c65458f88 @drogus drogus committed Jul 22, 2013
View
@@ -143,4 +143,17 @@
*Daniel Schierbeck*
+* Improve `ActiveSupport::Cache::MemoryStore` cache size calculation.
+ The memory used by a key/entry pair is calculated via `#cached_size`:
+
+ def cached_size(key, entry)
+ key.to_s.bytesize + entry.size + PER_ENTRY_OVERHEAD
+ end
+
+ The value of `PER_ENTRY_OVERHEAD` is 240 bytes based on an [empirical
+ estimation](https://gist.github.com/ssimeonov/6047200) for 64-bit MRI on
+ 1.9.3 and 2.0. GH#11512
+
+ *Simeon Simeonov*
+
Please check [4-0-stable](https://github.com/rails/rails/blob/4-0-stable/activesupport/CHANGELOG.md) for previous changes.
@@ -123,6 +123,14 @@ def synchronize(&block) # :nodoc:
end
protected
+
+ # See https://gist.github.com/ssimeonov/6047200
+ PER_ENTRY_OVERHEAD = 240
+
+ def cached_size(key, entry)
+ key.to_s.bytesize + entry.size + PER_ENTRY_OVERHEAD
+ end
+
def read_entry(key, options) # :nodoc:
entry = @data[key]
synchronize do
@@ -140,8 +148,11 @@ def write_entry(key, entry, options) # :nodoc:
synchronize do
old_entry = @data[key]
return false if @data.key?(key) && options[:unless_exist]
- @cache_size -= old_entry.size if old_entry
- @cache_size += entry.size
+ if old_entry
+ @cache_size -= (old_entry.size - entry.size)
+ else
+ @cache_size += cached_size(key, entry)
+ end
@key_access[key] = Time.now.to_f
@data[key] = entry
prune(@max_size * 0.75, @max_prune_time) if @cache_size > @max_size
@@ -153,7 +164,7 @@ def delete_entry(key, options) # :nodoc:
synchronize do
@key_access.delete(key)
entry = @data.delete(key)
- @cache_size -= entry.size if entry
+ @cache_size -= cached_size(key, entry) if entry
!!entry
end
end
@@ -713,8 +713,8 @@ def test_log_exception_when_cache_read_fails
class MemoryStoreTest < ActiveSupport::TestCase
def setup
- @record_size = ActiveSupport::Cache::Entry.new("aaaaaaaaaa").size
- @cache = ActiveSupport::Cache.lookup_store(:memory_store, :expires_in => 60, :size => @record_size * 10)
+ @record_size = ActiveSupport::Cache.lookup_store(:memory_store).send(:cached_size, 1, ActiveSupport::Cache::Entry.new("aaaaaaaaaa"))
+ @cache = ActiveSupport::Cache.lookup_store(:memory_store, :expires_in => 60, :size => @record_size * 10 + 1)
end
include CacheStoreBehavior
@@ -764,6 +764,30 @@ def test_prune_size_on_write
assert !@cache.exist?(1), "no entry"
end
+ def test_prune_size_on_write_based_on_key_length
+ @cache.write(1, "aaaaaaaaaa") && sleep(0.001)
+ @cache.write(2, "bbbbbbbbbb") && sleep(0.001)
+ @cache.write(3, "cccccccccc") && sleep(0.001)
+ @cache.write(4, "dddddddddd") && sleep(0.001)
+ @cache.write(5, "eeeeeeeeee") && sleep(0.001)
+ @cache.write(6, "ffffffffff") && sleep(0.001)
+ @cache.write(7, "gggggggggg") && sleep(0.001)
+ @cache.write(8, "hhhhhhhhhh") && sleep(0.001)
+ @cache.write(9, "iiiiiiiiii") && sleep(0.001)
+ long_key = '*' * 2 * @record_size
+ @cache.write(long_key, "llllllllll")
+ assert @cache.exist?(long_key)
+ assert @cache.exist?(9)
+ assert @cache.exist?(8)
+ assert @cache.exist?(7)
+ assert @cache.exist?(6)
+ assert !@cache.exist?(5), "no entry"
+ assert !@cache.exist?(4), "no entry"
+ assert !@cache.exist?(3), "no entry"
+ assert !@cache.exist?(2), "no entry"
+ assert !@cache.exist?(1), "no entry"
+ end
+
def test_pruning_is_capped_at_a_max_time
def @cache.delete_entry (*args)
sleep(0.01)

0 comments on commit 782d2f6

Please sign in to comment.