Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: difference in performance between Cache#get() and Cache.asMap()#compute #1761

Closed
SHxKM opened this issue Aug 15, 2024 · 5 comments
Closed

Comments

@SHxKM
Copy link

SHxKM commented Aug 15, 2024

This is more a question than anything else.

After optimizing a service heavily, Caffeine is now appearing in our CPU hot spot reports, especially scheduleDrainBuffers which is invoked by compute.

Cache config:

Cache<String, T> cache = Caffeine.newBuilder()
            .expireAfterWrite(Duration.ofHours(12))
            .maximumSize(500)
            .build();

The size is more than enough than the expected number of items, it's only there as a defense mechanism against memory catastrophes.

Currently, when accessing the cache, we're always doing the following:

cache.asMap().compute(...)

And I wonder if we can gain anything in terms of CPU usage if we do:

cache.get(...)

If so, it'd be lovely to understand why.

I assume both are thread-safe in the same manner.

@SHxKM
Copy link
Author

SHxKM commented Aug 15, 2024

Attaching the profiler dump screenshots for reference
image
image

@ben-manes
Copy link
Owner

Cache.get forwards to asMap().computeIfAbsent, so if that's an acceptable semantic then it is ideal. This would mean that if the value is found it acts as an inexpensive Map.get by being a read, whereas compute is always a exclusive write as it locks the entry to determine the new value. Do you need compute semantics?

Since this is always writing and that resets the entry's expiration time, that update needs to propagate to the expiration policy which maintains a time-ordered queue. To avoid global locks we use an intermediate buffer to publish an event (pub-sub message), which is typically asynchronously consumed to maintain this shared state with minimal blocking. However if that buffer is full then since we can't drop writes, it applies back pressure by forcing the writer to assist in processing this pending work. At a high write rate then it fills up quickly and degrades to your bottleneck by serializing the operations.

A small configuration change that might help is to use Caffeine.executor(Runnable::run) so that the policy maintenance is performed on a calling thread rather than being scheduled on the ForkJoinPool.commonPool(). Since its a write, the cache tries to drain that buffer asap using a tryLock and its own maintenance work is trivial (but users can configure callbacks so we default as async). Unfortunately commonPool() can be easily swamped by I/O tasks due to being the default executor for CompletableFuture.xxxAsync calls, but ForkJoinPool has limited threads as it is strictly designed for cpu-bound work. This could mean the async eviction is happening far later due to a blocked FJP threads causing a task backlog, and moving that onto the caller makes it prompt. In some cases then your problem simply goes away if the write rate isn't too high.

Otherwise, there is pending work in #1320, which optimizes for your exact use-case. That is to bring in an optimization on Cache.put to skip updating the entry's expireAfterWrite timestamp if within a 1s tolerance. This way if the write is a no-op (same value, effectively a read) it can avoid flooding the write buffer for TTL updates. Unfortunately I haven't had much time to dedicate towards more involved code changes. In that case the user found Guava to be faster and satisfy their needs in the time being.

The last option is to perform this TTL yourself by lazily relying only on the maximumSize and letting expired entries pollute the cache until size evicted. This would mean you would have to add the TTL onto your value, check if its still valid, and if not then reload it during the compute. Then when it exceeds capacity the size bounding will discard something, likely dead entries. This is how memcached and redis implemented it for high concurrency, which led to a lot of waste for a large cache server, so they now also do a periodic sweep. We don't do it since requiring a maximum size isn't valid for every user, but in your case its a very easy and convenient hack.

@SHxKM
Copy link
Author

SHxKM commented Aug 15, 2024

Thank you so much @ben-manes for taking the time not only to dispel the mystery but also educate at such a level. We indeed did not need asMap().compute(), it was a misunderstanding on my part that the two are equivalent.

@SHxKM SHxKM closed this as completed Aug 15, 2024
@SHxKM
Copy link
Author

SHxKM commented Aug 15, 2024

And I guess it's redundant to say Ben, but caffeine has been an indispensable tool.

@ben-manes
Copy link
Owner

thank you! I'm glad it's helpful. always feel more than welcome to reach out with questions, keeps me motivated 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants