ThreadSafeCache performance improvement.

Seberius commented 9 years ago

Hello, I experienced a significant performance drop running the included benchmark test when I switched it to cache.getset(rand(2_000)) { :value } from cache[rand(2_000)] ||= :value . I believe the culprit to be the coercion of yield blocks to Procs that occurs with the ThreadSafeCache synchronize class method. The new implementation is more verbose, but the performance gain is significant and it can easily be refactored as a mixin if needed (e.g. to support any new caches).

Test:

New implementation was in its own class for testing.

require "benchmark"

$LOAD_PATH.unshift File.expand_path '../lib'
require File.expand_path('../../lib/lru_redux', __FILE__)

lru_redux_thread_safe = LruRedux::ThreadSafeCache.new(1_000)
lru_redux_new_safe = LruRedux::NewSafeCache.new(1_000)

Benchmark.bmbm do |bm|

  [[lru_redux_thread_safe, "lru_redux thread safe"],
   [lru_redux_new_safe, "lru_redux new safe"]
  ].each do |cache, name|
    bm.report name do
      1_000_000.times do
        cache.getset(rand(2_000)) { :value }
      end
    end
  end
end

Result:

Rehearsal ---------------------------------------------------------
lru_redux thread safe   4.390000   0.020000   4.410000 (  4.411816)
lru_redux new safe      2.670000   0.010000   2.680000 (  2.673842)
------------------------------------------------ total: 7.090000sec

user     system      total        real
lru_redux thread safe   4.310000   0.020000   4.330000 (  4.330839)
lru_redux new safe      2.640000   0.000000   2.640000 (  2.643422)

The branch was passing all tests.

Fabulous run in 0.003373s, 6225.9117 runs/s, 18084.7910 assertions/s.

21 runs, 61 assertions, 0 failures, 0 errors, 0 skips
21 tests
61 assertions, 0 failures, 0 errors

SamSaffron commented 9 years ago

This looks really good, its probably the closure that is adding all this cost.

Do you mind updating the benchmarks in the readme? We should try to stay in sync here. (though annoyingly you will have to rerun all of them)

Seberius commented 9 years ago

I updated bench.rb to use .getset for lru_redux as it was about 1 second faster (I think its the extra lock with ||=). The docs for 'lru' specify to use ||=, so I believe this is fair. The readme has been updated with the new times.

SamSaffron commented 9 years ago

Looks good to me!

Will leave it a few days before doing a new release, thanks heaps!

Seberius commented 9 years ago

No problem, it was an interesting project.

SamSaffron commented 9 years ago

Awesome, thanks heaps for all your help!

On Mon, Feb 16, 2015 at 4:37 PM, Seberius notifications@github.com wrote:

I updated bench.rb to use .getset for lru_redux as it was about 1 second faster (I think its the extra lock with ||=). The docs for 'lru' specify to use ||=, so I believe this is fair. The readme has been updated with the new times.

— Reply to this email directly or view it on GitHub https://github.com/SamSaffron/lru_redux/pull/5#issuecomment-74462056.

SamSaffron commented 9 years ago

thanks again, got a new gem out!

On Mon, Feb 16, 2015 at 5:13 PM, Sam Saffron sam.saffron@gmail.com wrote:

Awesome, thanks heaps for all your help!

On Mon, Feb 16, 2015 at 4:37 PM, Seberius notifications@github.com wrote:

I updated bench.rb to use .getset for lru_redux as it was about 1 second faster (I think its the extra lock with ||=). The docs for 'lru' specify to use ||=, so I believe this is fair. The readme has been updated with the new times.

— Reply to this email directly or view it on GitHub https://github.com/SamSaffron/lru_redux/pull/5#issuecomment-74462056.

SamSaffron / lru_redux