JuliaCollections / LRUCache.jl

An implementation of an LRU Cache in Julia
Other
56 stars 23 forks source link

Implement eviction callback. #23

Closed GunnarFarneback closed 3 years ago

GunnarFarneback commented 3 years ago

This PR adds an eviction callback, which if set is called for each element that leaves the cache. This is useful if your cached values contain some resource that you want to recover. PR #1 had the same idea but is very much out of date with respect to the code.

Unfortunately this functionality doesn't come entirely for free, even if no callback is set. In terms of test/benchmark.jl the timing differences seem to be within the measurement noise but what is definitely noticeable is that all functions that may cause eviction (setindex!, get!, pop!, delete!, resize!, empty!) get one extra allocation of 80 bytes to hold the empty list of pending evictions.

To avoid this allocation will either require:

  1. Notify the callback while the lock is held. This seems inadvisable.
  2. Increase the implementation complexity in some way to avoid the allocation, possibly with function barriers to induce code specialization via some type trick or constant propagation.
  3. Some brilliant insight that avoids the allocation in a simple way. Haven't been that lucky so far.
GunnarFarneback commented 3 years ago

I suspect the failures of the 4 thread tests on Julia 1.0 is because the combination of @threads and Channel is too difficult there. It should be fine just to disable the test under those circumstances.

codecov-io commented 3 years ago

Codecov Report

Merging #23 (a3145c7) into master (1fadc28) will increase coverage by 3.70%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   72.81%   76.51%   +3.70%     
==========================================
  Files           2        2              
  Lines         217      247      +30     
==========================================
+ Hits          158      189      +31     
+ Misses         59       58       -1     
Impacted Files Coverage Δ
src/LRUCache.jl 96.62% <100.00%> (+0.89%) :arrow_up:
src/cyclicorderedset.jl 46.46% <0.00%> (+0.46%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1fadc28...a3145c7. Read the comment docs.

Krastanov commented 3 years ago

I was wondering what the status of this is? I have a use case where I my cache includes image files saved to disk, and this pull request would be incredibly helpful.

Jutho commented 3 years ago

My apologies; I completely forgot about this. I am occupied at least until Tuesday, but will try to look at later this week.

Jutho commented 3 years ago

My apologies for the late response; I finally have some time for this. Is it ok if I try to come up with some other implementation fo this which is free of cost without callback. Also, eviction_callback is a bit long and obscure. How do you think about finalizer as a keyword argument, in analogy to how you can register a finalizer to a mutable julia object.

Jutho commented 3 years ago

Actually, I will merge this and only then make a few changes. Are you fine with renaming eviction_callback to finalizer as keyword argument?