SamSaffron / lru_redux

An efficient optionally thread safe LRU Cache
MIT License
285 stars 20 forks source link

Fix for bug in ThreadSafeCache (JRuby 1.7) and improvement to Cache#delete (legacy) #6

Closed Seberius closed 9 years ago

Seberius commented 9 years ago

Hello, While working on another project I ran into a bug that affects the current ThreadSafeCache implementation.

ThreadSafeCache Bug (JRuby 1.7)

The bug that was introduced with #5 (I am ashamed to say) and affects JRuby 1.7. It does not affect JRuby 9000.

# JRuby 1.7.19
cache = LruRedux::ThreadSafeCache.new(3)

cache.getset(:a) { :value }
# throws LocalJumpError: yield called out of block

This error appears to indicate that JRuby 1.7 does not pass the block down to super like the MRI ( http://stackoverflow.com/questions/119207/what-does-yield-called-out-of-block-mean-in-ruby ). This branch changes ThreadSafeCache to explicitly pass &block if it is run by a JRuby version lower than 9.0. This allows us to keep the performance boost of #5 on all but JRuby 1.7.

Improvement to Cache#delete (legacy)

The code of Cache#delete seemed verbose when I submitted it (it was). This branch simplifies its code and also modifies its behavior. It will now act like the 1.9 version and return the deleted value or nil.

Benchmarks

This branch does not change the runtime performance.

Tests

This branch is passing all tests, including JRuby 1.7.19 and JRuby 9.0.0.0-pre1

Fabulous run in 0.002235s, 9395.9732 runs/s, 27293.0649 assertions/s.

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

8742.71 tests/s, 25395.50 assertions/s
SamSaffron commented 9 years ago

looks good, will do an updated gem