SamSaffron / lru_redux

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

TTL Cache and New Reader Methods #10

Closed Seberius closed 9 years ago

Seberius commented 9 years ago

This pull request is NOT complete, as it is missing the updated readme docs for the TTL cache. This is to give a little extra time for review as it is a large update.

TTL Cache

This branch introduces a TTL cache to LruRedux. The TTL cache behaves similarly to the LRU cache, but with time based eviction.

Notes/Features:

test/ttl/cache_test.rb and the benchmarks in README.md offer some additional behavior and performance information.

Credits: @ssimeonov - FastCache, a TTL cache based on LruRedux - (Performance Improvement) Source of the idea to convert Time objects to floats for comparison and storage.

New Reader Methods

The reader methods #max_size and #ttl have been added to both caches. The method #ttl= has also been added to the LRU cache and simply returns nil (not a valid return value for the TTL cache). This was done to provide a consistent api between the two caches.

Gemspec Update

This branch is currently passing all tests on 2.2.0 and jruby-1.7.19.

Fabulous run in 0.007995s, 7254.5341 runs/s, 16135.0844 assertions/s.

58 runs, 129 assertions, 0 failures, 0 errors, 0 skips
SamSaffron commented 9 years ago

so we need any of the 2.0 hacks here to ensure perf? also we should probably include a 2.0 bench and 2.2 bench, cause 2.0 is a slightly diff implementation

Seberius commented 9 years ago

so we need any of the 2.0 hacks here to ensure perf? also we should probably include a 2.0 bench and 2.2 bench, cause 2.0 is a slightly diff implementation

The 2.0 hacks affected the use of Hash#shift, which is not used in the TTL cache. The TTL cache requires finding a key and then deleting from two Hashes, so Hash#shift doesn't offer a benefit like it does for LRU.

The benchmark would only be different for the LRU cache, should we have a Ruby 2.0 benchmark just for that cache?

SamSaffron commented 9 years ago

yeah probably, seems correct and 2.0 is still pretty prevalent (we should definitely include the exact version of Ruby we ran the benches in the Readme for all of them)

On Mon, Mar 30, 2015 at 11:44 AM, Kaijah Hougham notifications@github.com wrote:

so we need any of the 2.0 hacks here to ensure perf? also we should probably include a 2.0 bench and 2.2 bench, cause 2.0 is a slightly diff implementation

The 2.0 hacks affected the use of Hash#shift, which is not used in the TTL cache. The TTL cache requires finding a key and then deleting from two Hashes, so Hash#shift doesn't offer a benefit like it does for LRU.

The benchmark would only be different for the LRU cache, should we have a Ruby 2.0 just for that cache?

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

Seberius commented 9 years ago

#valid? has been changed to be a protected method (in both caches). The readme now has some TTL cache usage examples, a 2.0.0 benchmark (along with exact versions used added to all benchmarks), a cache methods list, and links to the other caches used in the benchmarks. If there are any issues with the read me or something is missing from the instructions let me know. I am going to give it another look over tomorrow to see if there are any issues, but it should be 'complete' then I believe.

Edit: Oh, and the readme also has an index, as it was getting a bit long.

Seberius commented 9 years ago

I made some additional content and readability improvements to the readme, and I believe the branch is complete, sans any fixes that may be needed after review.

Tests

This branch is currently passing all tests on 1.9.3-p551, 2.0.0-p643, 2.1.5, 2.2.0, jruby-1.7.19 and jruby-9.0.0.0-pre1.

Fabulous run in 0.006060s, 9570.9571 runs/s, 27062.7063 assertions/s.

58 runs, 164 assertions, 0 failures, 0 errors, 0 skips
SamSaffron commented 9 years ago

awesome! thanks heaps

we should get a gem out with this soon

Seberius commented 9 years ago

Thanks! I did not bump the version to 1.1.0, so that still needs to be done. Also, should we add a tag for version v1.0.0 at ca99733eec696e6b397f90b5ff6d5f0ede19cadc?

SamSaffron commented 9 years ago

cool, yeah lets add tags, will get a release out next week.

On Wed, Apr 1, 2015 at 5:16 PM, Kaijah Hougham notifications@github.com wrote:

Thanks! I did not bump the version to 1.1.0, so that still needs to be done. Also, should we add a tag for version v1.0.0 at ca99733 https://github.com/SamSaffron/lru_redux/commit/ca99733eec696e6b397f90b5ff6d5f0ede19cadc ?

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

Seberius commented 9 years ago

OK, I bumped the version number on master and added tag v1.0.0 (with RubyGems like annotations) to ca99733eec696e6b397f90b5ff6d5f0ede19cadc.

SamSaffron commented 9 years ago

Cool, I went ahead and released 1.1.0, having some issue running guard but rake test is working fine

On Wed, Apr 8, 2015 at 7:35 AM, Kaijah Hougham notifications@github.com wrote:

OK, I bumped the version number on master and added tag v1.0.0 (with RubyGems like annotations) to ca99733 https://github.com/SamSaffron/lru_redux/commit/ca99733eec696e6b397f90b5ff6d5f0ede19cadc .

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

Seberius commented 9 years ago

I don't personally use guard, but the Rakefile had to be changed to catch all the tests in 0a53ceaa35b7dc218554a73fdc35fa8de6fb7a95, so that may need to be done in the Guardfile as well. If that doesn't help, some require statements were also removed from the tests as rake test didn't need them and it was causing some compatibility issues (1.8 I believe).

Seberius commented 9 years ago

I pushed a branch called guard-fix that I believe resolves the guard issue (it was the removed require statements). I didn't put it in a pull request because it is such a small fix, but I can if it is preferred.

SamSaffron commented 9 years ago

sure, just push the change.

On Thu, Apr 16, 2015 at 2:55 AM, Kaijah Hougham notifications@github.com wrote:

I pushed a branch called guard-fix that I believe resolves the guard issue (it was the removed require statements). I didn't put it in a pull request because it is such a small fix, but I can if it is preferred.

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