Thanks Otis;

> > Object get(Object key) {
> >     synchronized (map) {
> >         ... .incrementAndGet()
> >         ...
> >     }

Existing code does not slow down performance in simplest cases. However, it
slows down with read-only Faceted queries because each such query hits cache
thousands times even in simplest scenario.

This is ACCESS-ORDERED (LRU implementation) LinkedHashMap map = new
LinkedHashMap(initialSize, 0.75f, true)
- so that even GET is a structural modification (changind an order of a
list!), it should be synchronized... even if it is not, "changing order"
costs some CPU time. 

Simplest way to improve: go with INSERTION-ORDERED (FIFO implementation)
linked hash map, and unsynchronize GET(); acceptable for many cases.

I also don't understand why regenerateItem() is not synchronized (warm()
method in LRUCache should be synchronized on _local_ map).


P.S.
Most probably we can safely remove 'synchronized' from GET() in
Access-Ordered LRU, accepting fact that some entries could be wrongly
removed from LRU cache and we are not iterating the map... Same with PUT(),
no any risk with size() and removeEldestEntry() (I hope, no any OOM)...

EHCache also bases their LRU on LinkedHashMap:
  public final class SpoolingLinkedHashMap extends java.util.LinkedHashMap
Their net.sf.ehcache.Cache class has internally synchronized get() and put()
methods.


P.P.S.
What  about this double-synchronization:
public synchronized Object put(Object key, Object value) {
  if (state == State.LIVE) {
    stats.inserts.incrementAndGet();
  }
  synchronized (map) {
    // increment local inserts regardless of state???
    // it does make it more consistent with the current size...
    inserts++;
    return map.put(key,value);
  }
}
On Windows/Intel, single-threaded 'synchronized' statement requires
additionally 600 CPU cycles...

-Fuad
-- 
View this message in context: 
http://www.nabble.com/LRUCache---synchronized%21--tp16439831p16967253.html
Sent from the Solr - Dev mailing list archive at Nabble.com.

Reply via email to