[grid] Migrate from Guava's CacheBuilder to Caffeine#15547
Conversation
|
Can you share some context for this? Why do we need this? |
|
This relates to #15370 where we try to see how caffeine could resolve something better than guava. |
|
Hard to tell from the thread as an outside observer, but is that thread’s issue that removal notifications occur sometime after expiration? In guava/caffeine the default is to evict as part of maintenance triggered by other activity, so the explicit cleanUp calls are required for promptness. Caffeine offers a scheduler option for a thread that does this based on the next ttl event. Since the removalListener is called asynchronously, caffeine also offers an evictionListener if you need it as part of the eviction’s atomic map operation. Neither provides linearizable invalidateAll so in-flight loads would be skipped. Guava’s loads are never linearizable whereas Caffeine’s are except for that method because otherwise we’d need to fork ConcurrentHashMap which suppresses initial loads during a traverse. but I don’t know if that’s helpful or if I grok the issues |
|
@diemol the javadoc of Guava's More details here: https://github.com/google/guava/blob/master/guava/src/com/google/common/cache/CacheBuilder.java#L49 |
|
@VietND96, should we proceed with this? |
|
I will resume to check this PR in this release |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 87249f2
Previous suggestions✅ Suggestions up to commit ea55a8a
|
|||||||||||||||||||||
87249f2 to
0b94b77
Compare
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Motivation and Context
The javadoc of Guava's CacheBuilder recommends moving to Caffeine. So, besides concrete issues, it makes sense to switch.
https://github.com/google/guava/blob/master/guava/src/com/google/common/cache/package-info.java#L16-L17
More details here: https://github.com/google/guava/blob/master/guava/src/com/google/common/cache/CacheBuilder.java#L49
Types of changes
Checklist
PR Type
Enhancement
Description
Migrate from Guava's CacheBuilder to Caffeine for improved performance
Add custom cache weigher with memory-aware sizing in GraphqlHandler
Implement proper resource cleanup with AutoCloseable interface
Update dependency versions (GraphQL, Netty, OpenTelemetry, Log4j)
Diagram Walkthrough
File Walkthrough
GraphqlHandler.java
Migrate GraphQL handler to Caffeine cachejava/src/org/openqa/selenium/grid/graphql/GraphqlHandler.java
LocalNode.java
Update LocalNode caching to use Caffeinejava/src/org/openqa/selenium/grid/node/local/LocalNode.java
MODULE.bazel
Update build dependencies and add CaffeineMODULE.bazel
maven_install.json
Maven dependency resolution updatesjava/maven_install.json
BUILD.bazel
Add Caffeine to GraphQL build depsjava/src/org/openqa/selenium/grid/graphql/BUILD.bazel
BUILD.bazel
Add Caffeine to LocalNode build depsjava/src/org/openqa/selenium/grid/node/local/BUILD.bazel