Migrate concurrent linked map cache to Caffeine - #15961
Conversation
Replace concurrentlinkedhashmap-lru internals with Caffeine while keeping public cache API. Assisted-by: Sisyphus:xai/grok-4.5 [gpt-coding]
There was a problem hiding this comment.
Pull request overview
This pull request migrates the GrailsConcurrentLinkedMapCache implementation from concurrentlinkedhashmap-lru to Caffeine, aiming to modernize the cache internals while keeping the existing public cache manager API stable for Grails applications.
Changes:
- Replaced the cache’s internal store with a Caffeine
Cacheand exposed itsasMap()view as theConcurrentMapnative cache. - Switched the
grails-cachemodule dependency fromconcurrentlinkedhashmap-lrutocom.github.ben-manes.caffeine:caffeine. - Updated cache documentation and migrated the cache unit test from JUnit-style assertions to Spock.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| grails-doc/src/en/guide/cache/cacheUsage/cacheConfiguration.adoc | Updates cache configuration documentation to describe the Caffeine-backed bounded cache manager. |
| grails-cache/src/test/groovy/grails/plugin/cache/GrailsConcurrentLinkedMapCacheTests.groovy | Rewrites tests as Spock specs and adjusts expectations for the new eviction behavior. |
| grails-cache/src/main/groovy/grails/plugin/cache/GrailsConcurrentLinkedMapCacheManager.groovy | Updates class-level documentation to reflect the new bounded-cache intent. |
| grails-cache/src/main/groovy/grails/plugin/cache/GrailsConcurrentLinkedMapCache.java | Implements Caffeine-backed storage and updates capacity/size/hottest-keys behavior accordingly. |
| grails-cache/build.gradle | Replaces the old LRU cache dependency with Caffeine. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| grails.cache.enabled,`true`,Whether to enable the plugin | ||
| grails.cache.clearAtStartup,`false`,Whether to clear all caches at startup | ||
| grails.cache.cacheManager,GrailsConcurrentMapCacheManager,Cache Manager to use. Default cache manager uses Spring Frameworks ConcurrentMapCache which might grow limitless. If you cannot predict how many cache entries you are going to generate use "GrailsConcurrentLinkedMapCacheManager" instead which uses com.googlecode.concurrentlinkedhashmap.ConcurrentLinkedHashMap and limits by default to 10000 entries per cache. | ||
| grails.cache.cacheManager,GrailsConcurrentMapCacheManager,Cache Manager to use. Default cache manager uses Spring Frameworks ConcurrentMapCache which might grow limitless. If you cannot predict how many cache entries you are going to generate use "GrailsConcurrentLinkedMapCacheManager" instead. In Grails 8.1 this manager keeps the public Grails cache API while using Caffeine-backed bounded caches; the old concurrentlinkedhashmap-lru dependency path is deprecated. |
| then: | ||
| cache.nativeCache.get('key') == 'value' | ||
| cache.nativeCache.getClass().name.startsWith('com.github.benmanes.caffeine.cache.') |
| when: | ||
| cache.get(1) | ||
| cache.get(2) | ||
|
|
🚨 TestLens detected 1 failed test 🚨Here is what you can do:
Test SummaryCI - Groovy Joint Validation Build / build_grails > :grails-cache:test
🏷️ Commit: e01d6d8 Test FailuresGrailsConcurrentLinkedMapCacheTests > returns hottest keys from cache eviction policy (:grails-cache:test in CI - Groovy Joint Validation Build / build_grails)Muted TestsSelect tests to mute in this pull request:
Reuse successful test results:
Click the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app. |
|
@jamesfredley have you discussed this with @matrei? I thought I remember him doing something similar here (#15091) |
|
This PR represents an architectural shift that was not discussed before being submitted. I do not think it should proceed until we've discussed it as a team. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #15961 +/- ##
===================================================
- Coverage 49.5403% 19.5852% -29.9550%
+ Complexity 16922 308 -16614
===================================================
Files 1999 61 -1938
Lines 93754 3472 -90282
Branches 16420 601 -15819
===================================================
- Hits 46446 680 -45766
+ Misses 40145 2662 -37483
+ Partials 7163 130 -7033 🚀 New features to boost your workflow:
|
|
Good call to check - I looked at #15091, and it's complementary rather than overlapping. #15091 (@matrei, "fix(deps): clarify and update usage of On the "architectural shift, not discussed as a team" point - fair, and I'll take it to the dev list / weekly before proceeding rather than merging unilaterally. For grounding, the migration is a documented codebase-improvement recommendation (pre-release review, Codebase 3):
The plan I'd bring to the discussion: swap the internals of |
Description
What was found
concurrentlinkedhashmap-lruWhat changed
GrailsConcurrentLinkedMapCacheinternals use CaffeineOut of scope / follow-up
Related MD topics
Contributor Checklist
Issue and Scope
8.0.x.Code Quality
Licensing and Attribution
ai-generated-starting-pointlabel applied.Documentation
Assisted-by: Sisyphus:xai/grok-4.5 [gpt-coding]