ben-manes / concurrentlinkedhashmap

A ConcurrentLinkedHashMap for Java
Apache License 2.0
470 stars 113 forks source link

Support a pluggable evaluator for capacity handling #19

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Current Behavior:

Capacity handling currently is not pluggable and thus cannot be configured to 
support different schemes for determining when eviction should occur.

Desired Behavior:

Allowing for pluggable capacity handling would allow the CLHM to be used OOTB 
with an improvement in the works in the voldemort project - that being caching 
based on used memory, not predetermined 
(http://code.google.com/p/project-voldemort/issues/detail?id=225)

Complexity / Amount of Change Required:

Unknown, but I suspect it wouldn't be a huge change to implement.

Original issue reported on code.google.com by bruce.ri...@gmail.com on 14 Sep 2010 at 10:15

GoogleCodeExporter commented 9 years ago
The check for overflow is only performed after a write, so while your 
enhancement is nice for a dedicated caching server it may not work well in an 
general application environment.

Perhaps a better approach is to provide this through a public eviction method 
to allow the client to trigger eviction. In Voldemort's case, the maximum size 
could be set to Integer.MAX_VALUE to disable the size bounding. A thread could 
periodically trigger an eviction if a constraint was reached regardless of 
cache's activity, such as by calling:

EvictionChecker overflowChecker = MaxMemoryFreeChecker(maxMemorySizeInMB);
...
cache.evictWhile(overflowChecker);

The "while" would be desired because eviction is performed under lock in a 
non-blocking fashion. Since multiple entries may be evicted, calling it with 
the constraint would allow efficient eviction of multiple items.

If using this approach then we'd need to come up with a nicer class definition 
than the ugly pseudo code listed above. ;)

What are your thoughts on making it a client's responsibility rather than a 
configuration parameter that overrides #hasOverflowed()?

Original comment by Ben.Manes@gmail.com on 23 Sep 2010 at 4:59

GoogleCodeExporter commented 9 years ago
The solution you outline works fine for me, though it would be somewhat 
expensive to do the memory checks in a loop rather than just evicting x %/count 
of the map.

Original comment by bruce.ri...@gmail.com on 28 Sep 2010 at 9:07

GoogleCodeExporter commented 9 years ago
I plan on implementing this over the weekend. I'll probably allow both 
variants: a builder method executed after a write and a trigger method for 
direct execution. That should allow the desired flexibility.

So far the best contract I've come up with might look something like:

public interface SizeLimiter {

  @GuardedBy("evictionLock")
  boolean hasExceededLimit(ConcurrentLinkedHashMap<?, ?> map);
}

The default implementation would then be:

// enum for safe serialization
enum WeightedSizeLimiter implements SizeLimiter {

  @GuardedBy("evictionLock")
  public boolean hasExceededLimit(ConcurrentLinkedHashMap<?, ?> map) {
    return map.getWeightedSize() > map.getCapacity();
  }
}

This would be operated using the helper method,

public void evictWith(limiter); // evictUntil() or evictWhile()?

As before, the count would be best determined through an evictionListener so a 
void type seems ideal here.

---

Open issues:
- Its bad practice to call foreign code under lock, but that can't be resolved. 
Best to document.
- Is there any value of having generics on the limiter (K, V)? I can't think of 
a reasonable use-case.
- Better names?

Original comment by Ben.Manes@gmail.com on 23 Oct 2010 at 3:41

GoogleCodeExporter commented 9 years ago
Fixed in r472, please code review and ensure that the contract is satisfactory.

I am not satisfied with the class definition (class & method names) and would 
appreciate suggestions.

Original comment by Ben.Manes@gmail.com on 24 Oct 2010 at 4:20

GoogleCodeExporter commented 9 years ago
Ben - I'll check it out this week and give feedback where I can.

Original comment by bruce.ri...@gmail.com on 27 Oct 2010 at 12:41