[jira] [Commented] (CASSANDRA-10855) Use Caffeine (W-TinyLFU) for on-heap caches

["Robert Stupp (JIRA)" <ji...@apache.org>]

    [ 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)