songchuanyuan66 / concurrentlinkedhashmap

Automatically exported from code.google.com/p/concurrentlinkedhashmap
Apache License 2.0
0 stars 1 forks source link

Provide getQuiet() method? #34

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Current Behavior:

  The existing code exposes a get() call which updates alters the eviction characteristics for any cache entry it returns. 

Desired Behavior:

  I'd like to see an additional method called something like getQuiet() which returns the cache entry but does not interact with the eviction policy at all.  It's used for "peeking" into the map.  Caches like Ehcache and JCS have getQuiet() methods, which are used by reporting and memory management tools.

My use case right now is that I'd like to replace the JCS default memory cache 
(which has awful concurrent performance; it locks the whole cache during gets 
and puts) with an implementation backed by ConcurrentLinkedHashMap.  I cannot 
currently implement the getQuiet() method of the JCS MemoryCache interface 
(well, not properly).

Complexity / Amount of Change Required:

  It's a small change.  Something like, for example:

  public V getQuiet(Object key) {
      final Node node = data.get(key);
      if (node==null) {
          return null;
      }
      return node.getValue();
  }

  I know I could make this change myself to a fork of the code but before I do that I'm wondering if this is something you'd be interested in providing.  

Any thoughts?  Thanks!

Yours,
Joseph Calzaretta

Original issue reported on code.google.com by jc...@mit.edu on 9 Jul 2012 at 6:14

GoogleCodeExporter commented 9 years ago
That sounds reasonable. 

In Guava's CacheBuilder we added the asMap() view specifically to allow 
bypassing the cache management routines. There a cache.asMap().get(key) is a 
peek operation.

If CacheBuilder works for your use-cases then it would save me the trouble of 
an official release. Otherwise I can make the changes and push a release for 
you.

Differences:
- CacheBuilder has more features, Guava team, and was based lessons learned 
from CLHM
- CLHM has a more advanced algorithm, snapshot copies, and is easier to hack

Cheers,
Ben

Original comment by B...@addepar.com on 9 Jul 2012 at 7:11

GoogleCodeExporter commented 9 years ago
Thanks for your quick and helpful response!

The javadoc for CacheBuilder.expireAfterAccess() indicates that 
Cache.asMap().get(key) will update the last access time, which seems to imply 
it's not truly a peek (although this presumably doesn't affect the LRU 
behavior... or does it?).  

I was also using ConcurrentLinkedHashMap.ascendingKeySetWithLimit() to find the 
head of the queue (to implement JCS's MemoryCache.freeElements()) and I don't 
know if the iterators exposed by Guava's Cache.asMap() have any guaranteed 
ordering that helps me.  Doesn't look like it from the docs (although I haven't 
studied the source yet).

So, I would love to use CacheBuilder, but I can't tell if it would work.  If 
you're willing to push a release that would be great; if you'd think I should 
still head over to Guava that would be fine too.  Let me know.  Thanks again.

Yours,
Joseph Calzaretta 

Original comment by jc...@mit.edu on 9 Jul 2012 at 11:11

GoogleCodeExporter commented 9 years ago
CacheBuilder has continued to evolve since I left Google, so the slight 
contract change isn't surprising. If I recall correctly, it was originally 
intended to not affect the LRU or expiration policy and be used for lower-level 
scenarios like the one you mentioned. Cache#getIfPresent(K) was supposed to 
provide a peek that would be recorded by the eviction policies. There may have 
been some communication breakdown, bug that solidified the that contract, or 
simplification of the internal logic. Charles should know.

CacheBuilder cannot provide an ordered iterator in any form. It uses a 
per-segment LRU chain that is deeply integrated into a fork of 
ConcurrentHashMap. CLHM provides a global ordering, which required a more 
advanced approach to avoid blocking user operations to maintain the eviction 
policy. I was against forking CHM, which unfortunately will require rewriting 
CacheBuilder if it migrates to JDK8's new ConcurrentHashMap implementation.

I'm pretty swamped. If you prefer to keep a fork for a week or so then I can 
try to get to it. Your change appears correct and I would just add the 
appropriate unit tests, JavaDoc, and make a minor point release (v1.3.1).

Original comment by Ben.Manes@gmail.com on 9 Jul 2012 at 11:59

GoogleCodeExporter commented 9 years ago
Feel free to file a feature request with Guava for similar behavior with 
CacheBuilder.

Original comment by fry@google.com on 10 Jul 2012 at 8:43

GoogleCodeExporter commented 9 years ago
Ben,
  Waiting a week or so for v1.3.1 is absolutely fine with me.  Thanks for your help!

Fry,
  Will do.  Thanks!

Original comment by jc...@mit.edu on 10 Jul 2012 at 2:59

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r774.

Original comment by Ben.Manes@gmail.com on 19 Jul 2012 at 12:30

GoogleCodeExporter commented 9 years ago
Please confirm that no other additions are needed for the JCS migration, etc. 
and I'll perform a release.

Original comment by Ben.Manes@gmail.com on 19 Jul 2012 at 12:31

GoogleCodeExporter commented 9 years ago
The r774 code looks good to me and I don't need anything else for JCS.  Thanks 
for everything!

Original comment by jc...@mit.edu on 19 Jul 2012 at 12:50