diff --git a/.agents/skills/go-memoize-package/SKILL.md b/.agents/skills/go-memoize-package/SKILL.md index 8d81f8f..86f3749 100644 --- a/.agents/skills/go-memoize-package/SKILL.md +++ b/.agents/skills/go-memoize-package/SKILL.md @@ -44,7 +44,8 @@ Choose the API and store before editing: - `memory.NewSingle[K,V]()` is read-mostly and avoids LRU/hash overhead for one logical value. - Stores persist raw `memoize.Stored[V]` envelopes. The cache engine owns fresh, stale, and expired decisions. - Built-in stores may expose private fast paths used by the cache engine. Do not document those as public extension points. -- `WithGetRecencySample(n)` makes direct `Store.Get` recency approximate when `n > 1`. +- `WithGetRecencySample(n)` makes direct `Store.Get` and cache-engine fresh-hit recency approximate when `n > 1`. +- Default clocks and clocks created by `WithTickerClock` are cache-owned and stopped by `Cache.Stop`; clocks injected with `WithClock` are caller-owned and may be shared across caches. - Metrics use one public method: `RecordMetric(memoize.MetricEvent)`. - `background.Keep` and `loader.New` share internal periodic refresh-loop infrastructure. Cache stale refresh uses cache flight machinery, not the shared refresh loop. - Source compatibility may change for performance or clarity; users can pin module versions. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b8e87d..2c19894 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ jobs: with: # Use a patched toolchain for security scanning; go.mod still defines # the module compatibility target. - go-version: "1.25.10" + go-version: "1.25.13" check-latest: true cache: true cache-dependency-path: | diff --git a/README.md b/README.md index 687f4eb..8cb00ed 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ user, err := cache.GetOrCompute(ctx, "user:42", func(ctx context.Context) (User, | Store | None. Use `Opts().WithStore(store)` unless using `Bypass()`. | | Expiration policy | None. Choose `WithTTL`, `NoExpiration`, or `Bypass`. | | Metrics | Disabled with a noop metrics implementation. Enable with `WithMetrics`. | -| Clock | Ticker-backed clock with a 1ms tick. Call `cache.Stop()` to release it. | +| Clock | Cache-owned ticker clock with a 1ms tick. Call `cache.Stop()` to release it. Clocks passed with `WithClock` remain caller-owned. | | Refresh timeout | 30 seconds for background stale refresh. Override with `WithRefreshTimeout`. | | Same-key miss coalescing | Enabled internally for concurrent `GetOrCompute` misses on the same key. | @@ -112,7 +112,7 @@ Top-line results on this machine: `BenchmarkMemoryHotHit` was `29.80 ns/op` with - Use `memory.NewSingle` or `background.Keep` for one logical value such as config, feature flags, or exchange rates. - Use `memory.NewSharded` only when many different supported primitive keys are hot concurrently; one key still maps to one shard. - Use the Redis adapter or another shared store when multiple processes or hosts need the same backing cache. -- Always `defer cache.Stop()` for explicit caches using the default ticker clock. +- Always `defer cache.Stop()` for explicit caches using the default clock or `WithTickerClock`; stop caller-owned clocks passed with `WithClock` at their owning lifecycle boundary. - Run `go test ./... -count=1` and `go test ./... -race -count=1` before release. ## Documentation diff --git a/cache_engine_test.go b/cache_engine_test.go index bdd4d2e..61c75b0 100644 --- a/cache_engine_test.go +++ b/cache_engine_test.go @@ -120,6 +120,41 @@ func TestGetOrComputeCachesFreshValue(t *testing.T) { } } +func TestGetOrComputeFreshHitRefreshesLRURecency(t *testing.T) { + ctx := t.Context() + store := memory.New[string, string](2) + cache, err := memoize.New[string, string]( + memoize.Opts().WithStore(store).NoExpiration(), + ) + if err != nil { + t.Fatalf("new cache failed: %v", err) + } + t.Cleanup(cache.Stop) + + if err := cache.Set(ctx, "a", "A"); err != nil { + t.Fatalf("set a failed: %v", err) + } + if err := cache.Set(ctx, "b", "B"); err != nil { + t.Fatalf("set b failed: %v", err) + } + if got, err := cache.GetOrCompute(ctx, "a", func(context.Context) (string, error) { + t.Fatal("fresh value should not be recomputed") + return "", nil + }); err != nil || got != "A" { + t.Fatalf("fresh hit returned value=%q err=%v", got, err) + } + if err := cache.Set(ctx, "c", "C"); err != nil { + t.Fatalf("set c failed: %v", err) + } + + if _, ok, err := cache.Get(ctx, "b"); err != nil || ok { + t.Fatalf("least recently used key b returned ok=%v err=%v", ok, err) + } + if got, ok, err := cache.Get(ctx, "a"); err != nil || !ok || got != "A" { + t.Fatalf("recently used key a returned value=%q ok=%v err=%v", got, ok, err) + } +} + func TestGetOrComputeUsesPeekForFreshHit(t *testing.T) { ctx := context.Background() now := time.Date(2026, 5, 12, 12, 0, 0, 0, time.UTC) diff --git a/clock_ownership_test.go b/clock_ownership_test.go new file mode 100644 index 0000000..36aac00 --- /dev/null +++ b/clock_ownership_test.go @@ -0,0 +1,120 @@ +package memoize + +import ( + "context" + "testing" + "time" +) + +type clockTestStore struct{} + +func (*clockTestStore) Get(context.Context, string) (Stored[string], bool, error) { + return Stored[string]{}, false, nil +} + +func (*clockTestStore) Set(context.Context, string, Stored[string]) error { return nil } +func (*clockTestStore) Delete(context.Context, string) error { return nil } +func (*clockTestStore) Clear(context.Context) error { return nil } + +func TestCacheStopDoesNotStopInjectedClock(t *testing.T) { + clock := NewTickerClock(time.Hour) + t.Cleanup(clock.Stop) + + cache, err := New[string, string]( + Opts(). + WithStore(&clockTestStore{}). + WithTTL(time.Minute). + WithClock(clock), + ) + if err != nil { + t.Fatalf("new cache failed: %v", err) + } + cache.Stop() + + select { + case <-clock.stop: + t.Fatal("cache stopped a caller-owned clock") + default: + } + + if err := cache.Set(t.Context(), "key", "value"); err != nil { + t.Fatalf("cache should remain usable with the injected clock: %v", err) + } +} + +func TestCacheStopStopsOwnedClock(t *testing.T) { + tests := []struct { + name string + opts Options + }{ + { + name: "default", + opts: Opts().WithStore(&clockTestStore{}).NoExpiration(), + }, + { + name: "configured ticker", + opts: Opts().WithStore(&clockTestStore{}).NoExpiration().WithTickerClock(time.Hour), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache, err := New[string, string](tt.opts) + if err != nil { + t.Fatalf("new cache failed: %v", err) + } + clock, ok := cache.clock.(*TickerClock) + if !ok || !cache.clockOwned { + t.Fatalf("cache clock = %T owned=%v, want owned TickerClock", cache.clock, cache.clockOwned) + } + + cache.Stop() + select { + case <-clock.stop: + default: + t.Fatal("cache did not stop its owned clock") + } + }) + } +} + +func TestClockOptionOrder(t *testing.T) { + shared := NewTickerClock(time.Hour) + t.Cleanup(shared.Stop) + + borrowed, err := New[string, string]( + Opts(). + WithStore(&clockTestStore{}). + NoExpiration(). + WithTickerClock(time.Hour). + WithClock(shared), + ) + if err != nil { + t.Fatalf("new cache with final injected clock failed: %v", err) + } + if borrowed.clock != shared || borrowed.clockOwned { + t.Fatalf("final WithClock selected clock=%T owned=%v, want borrowed shared clock", borrowed.clock, borrowed.clockOwned) + } + borrowed.Stop() + + owned, err := New[string, string]( + Opts(). + WithStore(&clockTestStore{}). + NoExpiration(). + WithClock(shared). + WithTickerClock(time.Hour), + ) + if err != nil { + t.Fatalf("new cache with final ticker clock failed: %v", err) + } + if owned.clock == shared || !owned.clockOwned { + t.Fatalf("final WithTickerClock selected clock=%T owned=%v, want new owned clock", owned.clock, owned.clockOwned) + } + owned.Stop() + + select { + case <-shared.stop: + t.Fatal("cache stopped the caller-owned shared clock") + default: + } +} diff --git a/direct_options.go b/direct_options.go index 1b3e894..37f31ef 100644 --- a/direct_options.go +++ b/direct_options.go @@ -56,6 +56,7 @@ func (o Options) WithMetrics(metrics Metrics) Options { func (o Options) WithClock(clock Clock) Options { if clock != nil { o.clock = clock + o.tickerInterval = 0 } return o } @@ -69,6 +70,7 @@ func (o Options) WithRefreshTimeout(timeout time.Duration) Options { func (o Options) WithTickerClock(interval time.Duration) Options { if interval > 0 { + o.clock = nil o.tickerInterval = interval } return o @@ -115,12 +117,16 @@ func applyOptions[K comparable, V any](c *Cache[K, V], opts Options) error { } if opts.clock != nil { c.clock = opts.clock + c.clockOwned = false + c.tickerInterval = 0 } if opts.refreshTimeout > 0 { c.refreshTimeout = opts.refreshTimeout } if opts.tickerInterval > 0 { - c.clock = NewTickerClock(opts.tickerInterval) + c.clock = nil + c.clockOwned = false + c.tickerInterval = opts.tickerInterval } return nil } diff --git a/docs/API.md b/docs/API.md index c75e203..b74d962 100644 --- a/docs/API.md +++ b/docs/API.md @@ -120,7 +120,7 @@ defer cache.Stop() | `Delete(ctx, key)` | Deletes one key. | | `Clear(ctx)` | Clears the backing store. | | `GetOrCompute(ctx, key, fn)` | Returns a fresh cached value or computes, stores, and returns it. Concurrent misses for the same key are coalesced. | -| `Stop()` | Releases ticker-clock resources. Safe to call more than once. | +| `Stop()` | Releases cache-owned ticker-clock resources. It does not stop clocks supplied with `WithClock`. Safe to call more than once. | The cache engine owns freshness decisions. Stores persist `memoize.Stored[V]` envelopes and return entries even when they might be stale or expired; `Cache[K,V]` decides whether to serve, refresh, or miss. @@ -157,8 +157,8 @@ Build options with the non-generic root builder `memoize.Opts()`. | `NoExpiration()` | Direct memoizers and explicit caches | Values remain fresh until overwritten, deleted, or cleared. | Satisfies the required expiration-policy validation. | | `Bypass()` | Direct memoizers and explicit caches | Always computes and never stores. Useful for feature flags, tests, or temporarily disabling caching. | Satisfies expiration-policy validation and does not require a store. | | `WithMetrics(metrics)` | Direct memoizers and explicit caches | Records cache events through `RecordMetric(MetricEvent)`. Nil is ignored. | No error; nil leaves metrics disabled. | -| `WithClock(clock)` | Direct memoizers and explicit caches | Injects a clock, mainly for tests or custom timing. | Nil is ignored. | -| `WithTickerClock(interval)` | Direct memoizers and explicit caches | Uses a root ticker-backed clock at the given interval. | Non-positive intervals are ignored. | +| `WithClock(clock)` | Direct memoizers and explicit caches | Injects a caller-owned clock, mainly for tests, shared clocks, or custom timing. The cache does not stop it. | Nil is ignored. | +| `WithTickerClock(interval)` | Direct memoizers and explicit caches | Creates a cache-owned ticker-backed clock at the given interval. `Cache.Stop` releases it. | Non-positive intervals are ignored. | | `WithRefreshTimeout(timeout)` | Direct memoizers and explicit caches | Timeout used for background stale refresh work. | Non-positive values are ignored; default remains in effect. | Every cache needs exactly one expiration strategy in practice: `WithTTL`, `NoExpiration`, or `Bypass`. `WithStaleTTL` extends a TTL policy; it is not a standalone expiration policy. @@ -174,7 +174,7 @@ Every cache needs exactly one expiration strategy in practice: `WithTTL`, `NoExp | Refresh timeout | `30 * time.Second`. | Same. | | Concurrent miss coalescing | Enabled by an internal per-key flight map. | Same. | -Call `cache.Stop()` for explicit caches when you own the cache lifetime. Direct memoizers own their internal cache; use an explicit cache if shutdown control is required. +Call `cache.Stop()` for explicit caches when you own the cache lifetime. It stops the default clock and clocks created by `WithTickerClock`; the caller remains responsible for clocks injected through `WithClock`. Direct memoizers own their internal cache; use an explicit cache if shutdown control is required. ## Errors @@ -296,10 +296,10 @@ Memory options: | Option | Meaning | |---|---| | `memory.WithMaxBytes(n)` | Shallow byte budget; evicts LRU entries when exceeded. | -| `memory.WithGetRecencySample(n)` | Refreshes LRU recency every `n` hits. `n <= 1` keeps exact LRU on every get. | +| `memory.WithGetRecencySample(n)` | Refreshes LRU recency every `n` direct-store or cache-engine fresh hits. `n <= 1` keeps exact LRU on every hit. | | `memory.WithShards(n)` | Shard count for `NewSharded`; must be a positive power of two. | -Memory stores support `Get`, `Peek`, `Set`, `Delete`, `Clear`, `DeleteByTag`, `Len`, and `UsedBytes`. `Peek` is used by the cache engine to read without recency updates; user code should usually call `Cache` methods instead. +Memory stores support `Get`, `Peek`, `Set`, `Delete`, `Clear`, `DeleteByTag`, `Len`, and `UsedBytes`. The cache engine updates recency for fresh hits and uses `Peek` for policy inspection without making stale or expired entries recent; user code should usually call `Cache` methods instead. ## Chain Store diff --git a/docs/PERFORMANCE.md b/docs/PERFORMANCE.md index a7a60ca..a385e2d 100644 --- a/docs/PERFORMANCE.md +++ b/docs/PERFORMANCE.md @@ -16,7 +16,7 @@ By default, direct memoizers use an internal unbounded map-backed store. Use `me The cache engine coalesces concurrent same-key misses internally. One caller becomes the leader and computes the value; followers wait for the active flight and receive the same result. This avoids stampedes for cold keys and for refresh paths that converge on the same key. -Explicit caches own a default ticker clock and should be shut down with `Stop` when the cache lifetime ends. `Stop` releases the default clock goroutine and is safe to call more than once. +Explicit caches own a default ticker clock and should be shut down with `Stop` when the cache lifetime ends. `WithTickerClock` also creates a cache-owned clock. `WithClock` injects a caller-owned clock instead, allowing several caches to share one clock without an individual cache stopping it. Clock construction happens only after cache options validate, so failed construction and injected clocks do not leave a hidden default ticker behind. ## Direct Memoizer Keying @@ -34,7 +34,7 @@ External stores only need to implement the public `memoize.Store[K, V]` interfac ## Memory Store Design -`memory.New` keeps an exact LRU with a fixed item capacity. It also supports optional byte limits using a shallow entry-size estimate; heap allocations inside values such as string contents or slice backing arrays are not counted. +`memory.New` keeps an exact LRU with a fixed item capacity. Direct `Store.Get` calls and fresh cache-engine hits both refresh recency; `WithGetRecencySample(n)` can make those updates approximate to reduce contention. Optional byte limits use a shallow entry-size estimate; heap allocations inside values such as string contents or slice backing arrays are not counted. `memory.NewSingle` stores one logical value and avoids LRU and hash overhead on the hot read path. Use it for one cached snapshot, one global configuration value, or one hot key. It is not a general replacement for many-key caches. diff --git a/docs/PRODUCTION.md b/docs/PRODUCTION.md index 77e032c..f75abd5 100644 --- a/docs/PRODUCTION.md +++ b/docs/PRODUCTION.md @@ -14,7 +14,7 @@ This guide describes production choices for the root module `github.com/agkloop/ | Direct memoizer freshness | No default expiration policy. | Choose `WithTTL`, `NoExpiration`, or `Bypass`. | | Metrics | Disabled/noop. | Pass `WithMetrics` where hit rate, stale behavior, refresh errors, or writes need observability. | | Refresh timeout | 30 seconds. | Use `WithRefreshTimeout` when stale refresh must respect a stricter dependency SLO. | -| Clock lifecycle | Default ticker-backed clock. | Call `cache.Stop()` for explicit caches during shutdown. | +| Clock lifecycle | Default cache-owned ticker-backed clock. | Call `cache.Stop()` for explicit caches during shutdown. Clocks injected with `WithClock` remain caller-owned and can be shared safely. | ## Which API Should I Use? @@ -319,7 +319,8 @@ Track hit rate, stale hits, refresh errors, refresh latency, set/delete rates, a ## Shutdown -- Call `cache.Stop()` for explicit caches using the default ticker clock. +- Call `cache.Stop()` for explicit caches using the default clock or `WithTickerClock`. +- Stop a shared clock passed through `WithClock` once, at its owning lifecycle boundary; individual caches do not stop injected clocks. - Cancel the context passed to `background.Keep` or `background.Mirror` to stop refresh loops. - Call `loader.Stop()` to stop loader goroutines. - Pass request contexts into `GetOrCompute`; compute functions receive the same context. diff --git a/options.go b/options.go index 94f4604..533f861 100644 --- a/options.go +++ b/options.go @@ -17,6 +17,8 @@ type Cache[K comparable, V any] struct { metrics Metrics metricsEnabled bool clock Clock + clockOwned bool + tickerInterval time.Duration refreshTimeout time.Duration flightMu sync.Mutex flights map[K]*flight[V] @@ -25,7 +27,6 @@ type Cache[K comparable, V any] struct { func New[K comparable, V any](opts ...Options) (*Cache[K, V], error) { c := &Cache[K, V]{ metrics: noopMetrics{}, - clock: NewTickerClock(time.Millisecond), refreshTimeout: 30 * time.Second, flights: make(map[K]*flight[V]), } @@ -49,14 +50,22 @@ func New[K comparable, V any](opts ...Options) (*Cache[K, V], error) { if c.store == nil && !c.bypass { return nil, ErrMissingStore } + if c.tickerInterval > 0 { + c.clock = NewTickerClock(c.tickerInterval) + c.clockOwned = true + } else if c.clock == nil { + c.clock = NewTickerClock(time.Millisecond) + c.clockOwned = true + } return c, nil } // Stop releases background resources held by the cache. -// If the cache uses a TickerClock (the default), Stop shuts down its -// background goroutine. Safe to call multiple times. +// If the cache owns a TickerClock (the default or WithTickerClock), Stop shuts +// down its background goroutine. Clocks supplied with WithClock remain owned +// by the caller. Safe to call multiple times. func (c *Cache[K, V]) Stop() { - if tc, ok := c.clock.(*TickerClock); ok { + if tc, ok := c.clock.(*TickerClock); ok && c.clockOwned { tc.Stop() } } diff --git a/stores/memory/options.go b/stores/memory/options.go index e88abdb..c0d1bce 100644 --- a/stores/memory/options.go +++ b/stores/memory/options.go @@ -31,8 +31,9 @@ func WithMaxBytes[K comparable, V any](n int64) Option[K, V] { } } -// WithGetRecencySample makes Get refresh LRU recency once every n hits. -// n <= 1 preserves exact LRU behavior by refreshing recency on every hit. +// WithGetRecencySample makes direct Get calls and cache-engine fresh hits +// refresh LRU recency once every n hits. n <= 1 preserves exact LRU behavior +// by refreshing recency on every hit. func WithGetRecencySample[K comparable, V any](n uint32) Option[K, V] { return func(o *options) { o.getRecencySample = n diff --git a/stores/memory/store.go b/stores/memory/store.go index 4ab7ac1..059db8d 100644 --- a/stores/memory/store.go +++ b/stores/memory/store.go @@ -89,6 +89,9 @@ func (s *Store[K, V]) PeekFreshValue(_ context.Context, key K, now time.Time) (V } entry := &s.elements[pos].value if entry.NoExpire || now.Before(entry.FreshUntil) || now.Equal(entry.FreshUntil) { + if s.refreshOnGet() { + s.moveToFront(pos) + } return entry.Value, true, nil } var zero V diff --git a/stores/memory/store_test.go b/stores/memory/store_test.go index 72533ed..35c598e 100644 --- a/stores/memory/store_test.go +++ b/stores/memory/store_test.go @@ -125,6 +125,37 @@ func TestGetRecencySampling(t *testing.T) { } } +func TestPeekFreshValueRecencySampling(t *testing.T) { + ctx := t.Context() + now := time.Now() + stored := func(v string) memoize.Stored[string] { + return memoize.Stored[string]{Value: v, FreshUntil: now.Add(time.Minute)} + } + + skipped := New[string, string](2, WithGetRecencySample[string, string](3)) + _ = skipped.Set(ctx, "a", stored("A")) + _ = skipped.Set(ctx, "b", stored("B")) + _, _, _ = skipped.PeekFreshValue(ctx, "a", now) + _ = skipped.Set(ctx, "c", stored("C")) + if _, ok, _ := skipped.Get(ctx, "a"); ok { + t.Fatal("first sampled fresh hit should not refresh recency; expected a to be evicted") + } + + refreshed := New[string, string](2, WithGetRecencySample[string, string](3)) + _ = refreshed.Set(ctx, "a", stored("A")) + _ = refreshed.Set(ctx, "b", stored("B")) + for range 3 { + _, _, _ = refreshed.PeekFreshValue(ctx, "a", now) + } + _ = refreshed.Set(ctx, "c", stored("C")) + if _, ok, _ := refreshed.Get(ctx, "a"); !ok { + t.Fatal("third sampled fresh hit should refresh recency; expected a to survive") + } + if _, ok, _ := refreshed.Get(ctx, "b"); ok { + t.Fatal("expected b to be evicted after sampled fresh hit of a") + } +} + func TestCapacityFirstConstructorEvictsLeastRecentlyUsed(t *testing.T) { ctx := context.Background() s := New[string, string](2)