Skip to content

Commit 0feec6b

Browse files
committed
docs: update design documentation to reflect the addition of CAR policy and clarify policy counts
- Updated the builder and dynamic dispatch documentation to indicate that cachekit now ships 18 implemented eviction policies, with CAR being a concrete policy not yet exposed through `CachePolicy` / `DynCache`. - Clarified the distinction between implemented policies and runtime-dispatch variants, ensuring accurate representation of the current state of the library. - Revised concurrency and trait hierarchy documentation to reflect the updated policy count, enhancing clarity for users regarding available features and capabilities. These changes improve the accuracy and comprehensiveness of the design documentation, aiding developers in understanding the current state of cachekit's policy implementations.
1 parent 808aef5 commit 0feec6b

5 files changed

Lines changed: 114 additions & 71 deletions

File tree

docs/design/builder-and-dyn-dispatch.md

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@
55
> Companion to [`design.md`](design.md) §13, [`trait-hierarchy.md`](trait-hierarchy.md),
66
> and [`concurrency.md`](concurrency.md).
77
8-
cachekit ships 17 implemented eviction policies. Most application code
9-
wants to pick one of them — possibly at runtime, based on configuration
10-
— without writing 17 monomorphized call sites. This document explains
11-
why that runtime choice is delivered through an enum dispatcher rather
12-
than a `Box<dyn Cache>`, what the user-visible cost is, and how to
13-
extend the surface when a new policy lands.
8+
cachekit ships 18 implemented eviction policies. The runtime dispatcher
9+
currently wires 17 of them; CAR exists as a concrete policy but is not yet a
10+
`CachePolicy` / `DynCache` variant. Most application code wants to pick a
11+
policy — possibly at runtime, based on configuration — without writing one
12+
monomorphized call site per policy. This document explains why that runtime
13+
choice is delivered through an enum dispatcher rather than a `Box<dyn Cache>`,
14+
what the user-visible cost is, and how to extend the surface when a new policy
15+
lands.
1416

1517
## The problem
1618

@@ -22,8 +24,8 @@ cache.insert(key, value);
2224
cache.get(&key);
2325
```
2426

25-
without enumerating the 17 policies at every call site. The cache type
26-
must therefore be **uniform across policies** — the concrete type the
27+
without enumerating every builder-wired policy at each call site. The cache
28+
type must therefore be **uniform across policies** — the concrete type the
2729
caller holds cannot depend on which policy was chosen.
2830

2931
Two Rust mechanisms give a uniform type:
@@ -158,6 +160,21 @@ enum CacheInner<K, V> /* same bounds */ {
158160
impossible, which forces feature requests through method additions
159161
rather than match-arm proliferation in user code.
160162

163+
### CAR builder gap
164+
165+
CAR is implemented as a concrete policy (`src/policy/car.rs`) and has a
166+
`policy-car` feature flag, but this branch does **not** currently expose it
167+
through `CachePolicy` / `DynCache`. Users who want CAR instantiate the concrete
168+
`CarCore<K, V>` type directly. Closing the gap means adding a
169+
`CachePolicy::Car` variant, a `CacheInner::Car(CarCore<K, V>)` variant, and the
170+
usual method / builder / test arms listed in [Adding a new policy](#adding-a-new-policy).
171+
172+
Until that lands, read "implemented policies" and "`DynCache` variants" as two
173+
different sets:
174+
175+
- **Implemented concrete policies:** 18.
176+
- **Runtime-dispatch variants:** 17.
177+
161178
## Type bounds: heavier than `Cache<K, V>`
162179

163180
`Cache<K, V>` requires only what each individual policy implementation
@@ -313,7 +330,8 @@ The dispatcher's runtime cost is small. The **maintenance** cost is
313330
real:
314331

315332
- **17 inner variants** × **~10 `DynCache` methods** = **~170 match
316-
arms** that must stay in sync.
333+
arms** that must stay in sync today. CAR will make this 18 variants
334+
once it is wired into the dispatcher.
317335
- A `Debug` impl, a `default()` (where applicable), and a
318336
`validate_policy` arm per variant.
319337
- A `Cargo.toml` feature flag per variant.
@@ -393,9 +411,9 @@ Distinctness makes `Expiring<Expiring<DynCache>>` structurally
393411
unrepresentable, which prevents the "two clocks, two indexes"
394412
double-wrapping bug at the type level.
395413

396-
The duplication is real: a parallel ~170 arms for the expiring
397-
variant. It is bounded (one type per cross-cutting capability) and
398-
the trade favours type-level safety over deduplication.
414+
The duplication is real: a parallel ~170 arms today, rising with the
415+
dispatcher variant count. It is bounded (one type per cross-cutting
416+
capability) and the trade favours type-level safety over deduplication.
399417

400418
## When not to use `DynCache`
401419

docs/design/concurrency.md

Lines changed: 51 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,48 +23,70 @@ and where the gaps are.
2323

2424
## The dominant pattern: sequential core, concurrent wrapper
2525

26-
Every concurrent type in cachekit follows the same shape:
26+
cachekit's concurrent types all keep the sequential core unaware of locking,
27+
but they do **not** all have the same struct shape. There are three families.
28+
29+
### Cloneable policy handles
30+
31+
Policy-level wrappers are shared handles around a locked policy core:
2732

2833
```text
29-
ConcurrentX<K, V> { inner: Arc<RwLock<X<K, V>>> }
34+
ConcurrentPolicy<K, V> { inner: Arc<RwLock<Policy<K, V>>> }
3035
```
3136

32-
where `X` is the single-threaded core (`LruCore`, `FifoCache`,
33-
`S3FifoCache`, `SlotArena`, `IntrusiveList`, `ClockRing`,
34-
`HashMapStore`, `SlabStore`, `WeightStore`, `HandleStore`,
35-
`FrequencyBuckets`). The wrapper:
37+
This shape is used by:
3638

37-
1. holds the core behind an `Arc<RwLock<…>>`,
38-
2. presents owned/`Arc<V>` returns instead of borrowed `&V`,
39-
3. is `Clone` via `Arc::clone` so callers can hand copies to threads,
40-
4. is `Send + Sync` because the inner core's `Send + Sync` impls
41-
auto-derive through `Arc<RwLock<…>>`.
39+
- `ConcurrentLruCache`[`src/policy/lru.rs`](../../src/policy/lru.rs)
40+
- `ConcurrentFifoCache`[`src/policy/fifo.rs`](../../src/policy/fifo.rs)
41+
- `ConcurrentS3FifoCache`[`src/policy/s3_fifo.rs`](../../src/policy/s3_fifo.rs)
4242

43-
The pattern is verbose but consistent and was chosen for three reasons:
43+
These types implement `Clone` via `Arc::clone`, so callers can hand cheap
44+
handles to threads. They expose owned / `Arc<V>` returns instead of borrowed
45+
`&V` because no reference can safely outlive the lock guard it came from.
4446

45-
- **No `&mut self` in the public API.** Sharing requires interior
46-
mutability; an `RwLock` is the cheapest tool that exposes both shared
47-
reads and exclusive writes through `&self`.
48-
- **The sequential core stays unaware of locking.** Policy code under
49-
[`src/policy/`](../../src/policy) is single-threaded and easier to
50-
reason about. The locking discipline lives in one place per type.
51-
- **The lock is replaceable.** Swapping `parking_lot::RwLock` for a
52-
different primitive (`std::sync::RwLock`, a sharded lock, a seqlock)
53-
is a local change because the inner core has no opinion on it.
47+
### Owning store and data-structure wrappers
5448

55-
The 11 concurrent wrappers shipped today live in:
49+
Store and data-structure wrappers usually own the lock directly:
50+
51+
```text
52+
ConcurrentX<K, V> { inner: RwLock<X<K, V>>, ... }
53+
```
54+
55+
Examples:
5656

57-
- `ConcurrentLruCache`[`src/policy/lru.rs`](../../src/policy/lru.rs)
58-
- `ConcurrentFifoCache`[`src/policy/fifo.rs`](../../src/policy/fifo.rs)
59-
- `ConcurrentS3FifoCache`[`src/policy/s3_fifo.rs`](../../src/policy/s3_fifo.rs)
6057
- `ConcurrentHashMapStore`, `ShardedHashMapStore`[`src/store/hashmap.rs`](../../src/store/hashmap.rs)
6158
- `ConcurrentSlabStore`[`src/store/slab.rs`](../../src/store/slab.rs)
6259
- `ConcurrentWeightStore`[`src/store/weight.rs`](../../src/store/weight.rs)
6360
- `ConcurrentHandleStore`[`src/store/handle.rs`](../../src/store/handle.rs)
64-
- `ConcurrentSlotArena`, `ShardedSlotArena`[`src/ds/slot_arena.rs`](../../src/ds/slot_arena.rs)
61+
- `ConcurrentSlotArena`[`src/ds/slot_arena.rs`](../../src/ds/slot_arena.rs)
6562
- `ConcurrentIntrusiveList`[`src/ds/intrusive_list.rs`](../../src/ds/intrusive_list.rs)
6663
- `ConcurrentClockRing`[`src/ds/clock_ring.rs`](../../src/ds/clock_ring.rs)
64+
65+
These wrappers are not necessarily cloneable handles. If a caller wants shared
66+
ownership, they can wrap the whole type in `Arc<_>`. Keeping the `Arc` out of
67+
the struct avoids an unnecessary refcount on users who only need a single owner.
68+
69+
### Sharded primitives
70+
71+
Sharded types own multiple independently locked shards:
72+
73+
```text
74+
ShardedX<K, V> {
75+
shards: Vec<RwLock<ShardState<K, V>>>,
76+
selector: ShardSelector,
77+
}
78+
```
79+
80+
Examples:
81+
82+
- `ShardedSlotArena`[`src/ds/slot_arena.rs`](../../src/ds/slot_arena.rs)
6783
- `ShardedFrequencyBuckets`[`src/ds/frequency_buckets.rs`](../../src/ds/frequency_buckets.rs)
84+
- `ShardedHashMapStore`[`src/store/hashmap.rs`](../../src/store/hashmap.rs)
85+
86+
The common design is not "`Arc<RwLock<_>>` everywhere"; it is **lock at the
87+
wrapper boundary and keep the sequential core lock-free**. The exact ownership
88+
shape depends on whether the type is intended to be a cloneable cache handle,
89+
an owning concurrent store, or a sharded primitive.
6890

6991
## Why `Concurrent*` does not implement `Cache<K, V>`
7092

@@ -271,8 +293,8 @@ When sharding is **not** what you want:
271293

272294
## Concurrent policy coverage
273295

274-
Of the 17 implemented policies, **3 ship with a `Concurrent*` wrapper
275-
today**: LRU, FIFO, S3-FIFO. The remaining 14 require external locking
296+
Of the 18 implemented policies, **3 ship with a `Concurrent*` wrapper
297+
today**: LRU, FIFO, S3-FIFO. The remaining 15 require external locking
276298
by the caller — typically `Arc<parking_lot::RwLock<CacheCore>>`. The
277299
relevant rustdoc on those policies (e.g. `LfuCache`, `HeapLfuCache`,
278300
`MfuCache`) calls this out.
@@ -282,7 +304,7 @@ mechanical: wrap the sequential core in `Arc<RwLock<…>>`, expose the
282304
`&self` API with `Arc<V>` returns, decide read-lock vs. write-lock per
283305
method, implement `Clone` via `Arc::clone`, and implement
284306
`unsafe impl ConcurrentCache`. The work is bounded; what's missing is
285-
the discipline to do it consistently across all 17 policies.
307+
the discipline to do it consistently across all 18 policies.
286308

287309
## Failure modes
288310

docs/design/metrics.md

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ Two questions this design avoided:
164164
- **"Why not just `AtomicU64` for everything?"** Because counters
165165
on `&mut self` paths (the majority — `insert`, `get`, `evict`)
166166
do not need atomic semantics; the policy already holds exclusive
167-
access. Using `AtomicU64` everywhere would impose memory-fence
168-
cost on the hot path for no concurrency benefit. The split
169-
reserves atomic-ish behaviour (`MetricsCell` + external lock) for
170-
read paths only.
167+
access. However, `MetricsCell` is only sound when `&self` metric
168+
increments are protected by exclusive synchronization or are known
169+
to be single-threaded. It is **not** a substitute for atomics under
170+
shared `RwLock::read` access.
171171

172172
## `MetricsCell`: interior mutability under external lock
173173

@@ -180,18 +180,20 @@ unsafe impl Sync for MetricsCell {}
180180
unsafe impl Send for MetricsCell {}
181181
```
182182

183-
This is the only `unsafe impl Sync` in the metrics surface. The
184-
contract:
185-
186-
- **External synchronization is required.** `MetricsCell` lives
187-
inside a policy struct that is itself behind an `RwLock` in any
188-
concurrent wrapper (see [`concurrency.md`](concurrency.md)). The
189-
read lock serializes concurrent `&self` access; the cell is
190-
manipulated under that lock.
191-
- **The cell is observation-only.** Lost increments are
192-
acceptable; the worst-case outcome is undercounting a metric,
193-
which is a precision issue, not a correctness one. Cache hits and
194-
evictions still behave correctly.
183+
This is the only `unsafe impl Sync` in the metrics surface, so its
184+
contract must be narrow:
185+
186+
- **Exclusive external synchronization is required.** A shared
187+
`RwLock::read` guard does **not** serialize readers, so it is not
188+
sufficient protection for `Cell<u64>`. `MetricsCell` may be used
189+
on single-threaded policy paths, or behind a write lock / mutex,
190+
but not for counters mutated concurrently through read-locked
191+
`&self` methods.
192+
- **Observation-only does not relax Rust's aliasing rules.** It is
193+
acceptable for metrics to be approximate; it is not acceptable for
194+
approximation to be implemented as unsynchronized `Cell` mutation.
195+
Concurrent read-path counters must use `AtomicU64`, take an
196+
exclusive lock, or be disabled for that path.
195197
- **`pub(crate)`.** The type does not escape the crate.
196198
Down-stream code can read counters through the snapshot API but
197199
cannot construct `MetricsCell` itself, which prevents misuse from
@@ -200,14 +202,15 @@ contract:
200202
The alternatives considered and rejected:
201203

202204
- `Mutex<u64>` — cost dominates the counter increment.
203-
- `AtomicU64` — works, but imposes fence cost where no concurrency
204-
exists for the increment itself.
205+
- `AtomicU64` — the correct choice for counters that can be
206+
incremented concurrently through shared references; unnecessary
207+
for single-threaded or exclusively locked counters.
205208
- `RefCell<u64>` — runtime borrow checking with panic on contention;
206209
not desirable on a metrics increment path.
207210

208-
`MetricsCell` is the smallest tool that says "we know about the
209-
sync requirement; trust the external lock; pay no per-increment
210-
cost beyond a `Cell::set`."
211+
`MetricsCell` is the smallest tool for single-threaded or exclusively
212+
locked metric counters. Any policy or wrapper that records metrics
213+
from a read-locked path must not rely on `MetricsCell` for soundness.
211214

212215
## Snapshots: cheap, copyable, optionally serializable
213216

@@ -482,11 +485,11 @@ What it does **not** guarantee:
482485
sequentially. A reader can observe `hits = 100, misses = 99`
483486
while a concurrent writer is mid-update; the next snapshot may
484487
show `hits = 100, misses = 101`. There is no "snapshot epoch."
485-
- **Lossless recording under contention.** `MetricsCell`
486-
increments under a held read lock are safe; multiple read locks
487-
are not serialized against each other. Concurrent `&self`
488-
recorder calls on the same `MetricsCell` can lose increments.
489-
This is the "best-effort observability" caveat.
488+
- **Concurrent `MetricsCell` recording.** `MetricsCell` must not be
489+
incremented from multiple read-locked callers. Shared read locks do
490+
not serialize readers, so those paths must use atomics or acquire an
491+
exclusive lock before recording. Metrics may be best-effort, but
492+
the implementation still has to be data-race-free.
490493
- **Wrap-safe arithmetic in release.** Release profile sets
491494
`overflow-checks = false`. Counters wrap silently. At one billion
492495
events per second, `u64` wraps in ~585 years — practically a
@@ -498,8 +501,8 @@ What it does **not** guarantee:
498501
principles level
499502
- [Cache trait hierarchy](trait-hierarchy.md)`&self` / `&mut self`
500503
split that drives the read-vs-mutate recorder fork
501-
- [Concurrency](concurrency.md) — read/write lock model behind
502-
`MetricsCell`'s soundness
504+
- [Concurrency](concurrency.md) — read/write lock model that
505+
constrains where `MetricsCell` may be used
503506
- [Error model](error-model.md) — panic discipline shared by the
504507
exporter's poisoning behaviour
505508
- [`src/metrics/`](../../src/metrics) — the canonical implementation

docs/design/trait-hierarchy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ The trait surface optimizes for four things, roughly in order:
1818

1919
1. **Code written against the kernel survives a policy swap.** Users
2020
writing `fn warm<C: Cache<K, V>>(c: &mut C, …)` can pick any of
21-
the 17 implemented policies without changing call sites.
21+
the 18 implemented concrete policies without changing call sites.
2222
2. **Optional behaviour is visible only when present.** A policy that
2323
doesn't track frequency should not have a `frequency()` method that
2424
returns garbage or panics. Capability traits exist so this remains

docs/design/weighted-eviction.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ the module documentation.
181181

182182
## Why weight is at the **store** layer, not the policy layer
183183

184-
The 17 implemented policies in `src/policy/` are all weight-unaware.
184+
The 18 implemented policies in `src/policy/` are all weight-unaware.
185185
They count entries and evict by entry. `WeightStore` is below them in
186186
the layering:
187187

0 commit comments

Comments
 (0)