[ 
https://issues.apache.org/jira/browse/SOLR-665?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12618064#action_12618064
 ] 

noble.paul edited comment on SOLR-665 at 7/29/08 10:08 PM:
-----------------------------------------------------------

There are a few obvious issues w/ your patch.
Effendi your implementation is wrong

{code:java}
         if (wrapper.lastAccessed < pointer.lastAccessed) {
                        ref.set(new Pointer(key, wrapper.lastAccessed));
}
        {code}
will always evaluate to false. And the reference will always have one value
We must be removing the entry which was accessed first (not last).. To get that 
you must maintian a linkedlist the way linkedhashmap maintains. No other 
shortcut. Keeping one reference *will not* work. 

In that implementation there is always a contention on the head so gets are 
bound to be slow. That is why i did not go that route
And the static volatile counter is not threadsafe. 
static in general is not a good idea


There is no need to use a WeakReference anywhere. This is a cache. So it *must* 
maintain strong reference

      was (Author: noble.paul):
    There are a few obvious issues w/ your patch.
Effendi your implementation is wrong

{code:java}
         if (wrapper.lastAccessed < pointer.lastAccessed) {
                        ref.set(new Pointer(key, wrapper.lastAccessed));
}
        {code}
will always evaluate to true. We must be removing the entry which was accessed 
first (not last)
. To get that you must maintian a linkedlist the way linkedhashmap maintains. 
No other shortcut. Keeping one reference *will not* work. 

In that implementation there is always a contention on the head so gets are 
bound to be slow. That is why i did not go that route
And the static volatile counter is not threadsafe.


There is no need to use a WeakReference anywhere. This is a cache. So it *must* 
maintain strong reference
  
> FIFO Cache (Unsynchronized): 9x times performance boost
> -------------------------------------------------------
>
>                 Key: SOLR-665
>                 URL: https://issues.apache.org/jira/browse/SOLR-665
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: 1.3
>         Environment: JRockit R27 (Java 6)
>            Reporter: Fuad Efendi
>         Attachments: ConcurrentFIFOCache.java, ConcurrentFIFOCache.java, 
> ConcurrentLRUCache.java, ConcurrentLRUWeakCache.java, 
> ConcurrentLRUWeakCache.java, ConcurrentLRUWeakCache.java, FIFOCache.java
>
>   Original Estimate: 672h
>  Remaining Estimate: 672h
>
> Attached is modified version of LRUCache where 
> 1. map = new LinkedHashMap(initialSize, 0.75f, false) - so that 
> "reordering"/true (performance bottleneck of LRU) is replaced to 
> "insertion-order"/false (so that it became FIFO)
> 2. Almost all (absolutely unneccessary) synchronized statements commented out
> See discussion at 
> http://www.nabble.com/LRUCache---synchronized%21--td16439831.html
> Performance metrics (taken from SOLR Admin):
> LRU
> Requests: 7638
> Average Time-Per-Request: 15300
> Average Request-per-Second: 0.06
> FIFO:
> Requests: 3355
> Average Time-Per-Request: 1610
> Average Request-per-Second: 0.11
> Performance increased 9 times which roughly corresponds to a number of CPU in 
> a system, http://www.tokenizer.org/ (Shopping Search Engine at Tokenizer.org)
> Current number of documents: 7494689
> name:          filterCache  
> class:        org.apache.solr.search.LRUCache  
> version:      1.0  
> description:  LRU Cache(maxSize=10000000, initialSize=1000)  
> stats:        lookups : 15966954582
> hits : 16391851546
> hitratio : 0.102
> inserts : 4246120
> evictions : 0
> size : 2668705
> cumulative_lookups : 16415839763
> cumulative_hits : 16411608101
> cumulative_hitratio : 0.99
> cumulative_inserts : 4246246
> cumulative_evictions : 0 
> Thanks

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to