Introduce a DelayManager that lets us expire and refresh elements #99
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new DelayManager to schedule expiration and optional refresh of cached elements, updates the core cache implementation to use it, and extends the public Arc API to allow configuring expiry/refresh durations.
- Introduce
DelayManagerandDelayedElementto track element lifetimes. - Refactor
InnerArc,ElementList,Element, andArcto integrate expiry and refresh logic. - Expand
ArcTestwith arepeatedElementsAreRefreshedtest andMockInstantSourceto verify timing behavior.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/src/test/java/eu/aylett/arc/ArcTest.java | Added timing-based test case (repeatedElementsAreRefreshed) and MockInstantSource. |
| lib/src/main/java/eu/aylett/arc/internal/InnerArc.java | Inject DelayManager, refactor constructors, processElement, and enqueueNewElement. |
| lib/src/main/java/eu/aylett/arc/internal/ElementList.java | Replace name string with Name enum, preload values for expiring lists in push. |
| lib/src/main/java/eu/aylett/arc/internal/Element.java | Refactor get/load, add delayExpired, reload, and integrate DelayManager. |
| lib/src/main/java/eu/aylett/arc/internal/DelayedElement.java | New class representing a Delayed wrapper for expiry and refresh hooks. |
| lib/src/main/java/eu/aylett/arc/internal/DelayManager.java | New class managing two DelayQueues: one for expiry and one for early refresh. |
| lib/src/main/java/eu/aylett/arc/Arc.java | Update public API to accept expiry and refresh durations, add validation in constructors. |
Comments suppressed due to low confidence (2)
lib/src/main/java/eu/aylett/arc/internal/DelayManager.java:66
- [nitpick] The interface name
GetDelayis vague; consider renaming it toDelayCalculatororDelayProviderto better convey its purpose.
public interface GetDelay {
lib/src/main/java/eu/aylett/arc/Arc.java:70
- The removal of the simple two-argument
Arcconstructor is a breaking change for existing clients; consider reintroducing or deprecating the old overload to maintain backwards compatibility.
public Arc(int capacity, Function<K, V> loader, Duration expiry, Duration refresh) {
| @@ -108,7 +115,7 @@ public InnerArc(int capacity, boolean safetyChecks) { | |||
| */ | |||
| @ReleasesNoLocks | |||
| @Holding("this") | |||
| public CompletableFuture<V> processElement(@GuardSatisfied InnerArc<K, V> this, Element<K, V> e) { | |||
| public CompletableFuture<V> processElement(@GuardedBy InnerArc<K, V> this, Element<?, V> e) { | |||
There was a problem hiding this comment.
The lock annotation on the receiver is inconsistent with the @GuardSatisfied annotation used elsewhere; consider using a consistent annotation to clarify the intended lock contract.
| public CompletableFuture<V> processElement(@GuardedBy InnerArc<K, V> this, Element<?, V> e) { | |
| public CompletableFuture<V> processElement(Element<?, V> e) { |
Copilot uses AI. Check for mistakes.
| if (expiryTarget != null && !newElement.containsValue()) { | ||
| newElement.get(); |
There was a problem hiding this comment.
Calling newElement.get() inside push may block the calling thread under lock and impact throughput; consider loading asynchronously or off the critical path.
| if (expiryTarget != null && !newElement.containsValue()) { | |
| newElement.get(); | |
| Object preloadedValue = null; | |
| if (expiryTarget != null && !newElement.containsValue()) { | |
| preloadedValue = newElement.get(); |
Copilot uses AI. Check for mistakes.
| var record = recordedValues.stream().collect(Collectors.groupingBy((v) -> v, Collectors.counting())); | ||
| // Would be 100 without any caching or refreshing | ||
| assertThat(record.get(0), lessThan(50L)); | ||
| // Would be ~15 with no early refreshes |
There was a problem hiding this comment.
[nitpick] The comment suggests an expected count of ~15, but the assertion checks greaterThan(25L); please align the comment with the actual assertion thresholds to avoid confusion.
Copilot uses AI. Check for mistakes.
No description provided.