[ 
https://issues.apache.org/jira/browse/CASSANDRA-10855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15641960#comment-15641960
 ] 

Robert Stupp commented on CASSANDRA-10855:
------------------------------------------

Alright, finally found some time to look at this.

Some comments & questions:
* I've added {{.executor(MoreExecutors.directExecutor())}} - hope I got your 
suggestion right.
* {{AuthCache}}: there's a bunch of 
{{cache.policy().refreshAfterWrite().ifPresent(}} code sequences. I guess this 
is just to use the existing API "right" so it always sets all the values, 
right? Maybe just add a line comment that all values are always set since these 
are mandatory.
* {{ConcurrentLinkedHashCache}} is no longer used in the whole code base. You 
can remove it.
* {{SerializingCache}} can you correct the typo in {{throw new 
IllegalArgumentException("Serialized size cannot be more than 2GB");}} to {{.. 
must not be more...}}
* There are a couple of {{cache.asMap()}} calls. Would it be an option to 
eagerly create the {{AsMapView}}, {{Values}} and {{EntrySet}} instances in 
{{LocalAsyncLoadingCache}} to get around the ternaries in {{asMap()}}, 
{{values()}}, {{entrySet()}} and {{keySet}}?

Beside these, I've got no comments. Pretty straight forward change. Good work 
so far!

Triggered a new CI round with the changes mentioned above.
Do you have some micro-benchmarks in place to actually test against the 
previous implementation(s)? If [~benedict] (IIRC he already worked on Caffeine) 
says it buys us something, I'd be fine omitting the benchmarks.

When you wanna change something, can you fork from my branch? This makes 
merging and checking changes easier.

Regarding OHC, I'd need to fully understand Caffeine internals, i.e. the 
eviction algorithm, first. Mind opening a ticket for OHC on gh?

> Use Caffeine (W-TinyLFU) for on-heap caches
> -------------------------------------------
>
>                 Key: CASSANDRA-10855
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10855
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Ben Manes
>              Labels: performance
>         Attachments: CASSANDRA-10855.patch, CASSANDRA-10855.patch
>
>
> Cassandra currently uses 
> [ConcurrentLinkedHashMap|https://code.google.com/p/concurrentlinkedhashmap] 
> for performance critical caches (key, counter) and Guava's cache for 
> non-critical (auth, metrics, security). All of these usages have been 
> replaced by [Caffeine|https://github.com/ben-manes/caffeine], written by the 
> author of the previously mentioned libraries.
> The primary incentive is to switch from LRU policy to W-TinyLFU, which 
> provides [near optimal|https://github.com/ben-manes/caffeine/wiki/Efficiency] 
> hit rates. It performs particularly well in database and search traces, is 
> scan resistant, and as adds a very small time/space overhead to LRU.
> Secondarily, Guava's caches never obtained similar 
> [performance|https://github.com/ben-manes/caffeine/wiki/Benchmarks] to CLHM 
> due to some optimizations not being ported over. This change results in 
> faster reads and not creating garbage as a side-effect.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to