From 6556c7039dad9ca1cb62d87178d3338ff418ff66 Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Wed, 28 Oct 2026 13:19:55 +0300 Subject: [PATCH 1/4] perf: indexed binary heap for LFU/VTTL; fix LFU popitem evicting hot keys LazyHeap was a VecDeque fully re-sorted before front()/pop whenever anything had been inserted, so LFU eviction and VTTL insert/eviction cost O(n log n) per operation (filling an unbounded VTTLCache with 100k entries took ~15s). Replace it with an indexed binary min-heap: - each element stores its buffer position, an insertion sequence number (ties keep insertion order, like the old stable sort) and the priority it was placed with; - push / pop_front / remove are O(log n); - LFU get() increments the frequency in O(1) without moving the buffer; the root is repaired lazily in front(), which is valid because frequencies only grow; - arbitrary key changes on replace are queued in O(1) and applied in front(), so replacing a value during iteration still does not move the buffer (generation-version semantics unchanged); - ordered iteration sorts in place (a sorted buffer is a valid heap). Bug fix: LFUCache.get() incremented the frequency without invalidating the sorted order, so popitem() could evict the most frequently used key: c = LFUCache(10) for k in "abcd": c[k] = k c.popitem() for _ in range(10): c.get("b") c.popitem() # -> ('b', 'b'), expected ('c', 'c') Notes: - VTTL ties (e.g. several entries without ttl) are evicted in insertion order; previously by buffer position. - ~21 extra bytes per LFU/VTTL entry for heap bookkeeping. The frequency is stored as u64 internally; the public API and pickle format still use the same integers. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KRuJihprbupFprYtRYAs92 --- src/internal/lazyheap.rs | 488 ++++++++++++++++++++++++++----------- src/policies/lfupolicy.rs | 43 +++- src/policies/vttlpolicy.rs | 38 ++- 3 files changed, 417 insertions(+), 152 deletions(-) diff --git a/src/internal/lazyheap.rs b/src/internal/lazyheap.rs index b63f05b..34f1ee4 100644 --- a/src/internal/lazyheap.rs +++ b/src/internal/lazyheap.rs @@ -2,47 +2,89 @@ use std::ptr::NonNull; use crate::internal::utils; -/// A collection that defers sorting until an ordered operation is requested. +/// Bookkeeping stored inside every element of a [`LazyHeap`]. /// -/// Unlike a classic binary heap, `LazyHeap` does not maintain a heap -/// invariant after every insertion. Instead it tracks a dirty flag and -/// re-sorts the entire backing buffer the first time an ordered operation is -/// needed. This amortises well when many insertions occur before any removal, -/// because one `O(n log n)` sort is cheaper than repeated `O(log n)` sift-ups. +/// * `pos` — current index in the heap buffer, so removal by cursor is O(log n) +/// instead of a linear scan. +/// * `seq` — insertion sequence number, used as a tie-breaker so elements with +/// equal priority keep insertion order (same behaviour as the old stable sort). +/// * `rec` — the priority the element had when it was last placed. The heap +/// invariant is maintained on `(rec, seq)`, not on the live key. This lets a +/// policy *increase* an element's priority in O(1) (e.g. LFU `get`) without +/// touching the buffer layout; the stale root is repaired lazily in +/// [`LazyHeap::front`]. +#[derive(Clone, Copy)] +pub struct HeapSlot { + seq: u64, + rec: K, + pos: u32, + /// Index in [`LazyHeap::pending`], or [`NOT_PENDING`]. + pidx: u32, +} + +const NOT_PENDING: u32 = u32::MAX; + +impl Default for HeapSlot { + fn default() -> Self { + Self { + pos: 0, + seq: 0, + rec: K::default(), + pidx: NOT_PENDING, + } + } +} + +/// An element that can live in a [`LazyHeap`]. +pub trait HeapNode { + /// Priority type. Smaller is closer to the front. + type Key: Ord + Copy; + + /// The element's *current* priority. + fn heap_key(&self) -> Self::Key; + fn heap_slot(&self) -> &HeapSlot; + fn heap_slot_mut(&mut self) -> &mut HeapSlot; +} + +/// An indexed binary min-heap with lazy priority increases and on-demand sorting. +/// +/// Complexity: +/// * `push`, `pop_front`, `remove`, `update` — O(log n) +/// * `front` — O(1), plus amortised repair work paid for by earlier lazy increases +/// * `sort_by` / `iter` — O(n log n) only when the order is not already sorted +/// +/// A fully sorted buffer is a valid heap, so ordered iteration simply sorts the +/// buffer in place and keeps using it as a heap afterwards. +/// +/// # Lazy increases +/// A caller may increase an element's key through [`Cursor::element_mut`] without +/// telling the heap, as long as it never *decreases* it that way (use +/// [`mark_changed`](Self::mark_changed) for arbitrary changes). Because recorded keys never +/// exceed live keys and the heap is ordered on recorded keys, a root whose +/// recorded key equals its live key is the true minimum. /// /// # Ownership model /// `LazyHeap` is the **sole owner** of every element it holds. Cursors are /// purely non-owning handles and must never be used to free the backing /// allocation. -pub struct LazyHeap { - data: std::collections::VecDeque>, +pub struct LazyHeap { + data: Vec>, + /// Elements whose key changed arbitrarily (possibly decreased) and still need + /// re-positioning. Applied in [`front`](Self::front), so the buffer layout never + /// moves under a live iterator because of a value replacement. + pending: Vec>, + next_seq: u64, is_sorted: bool, _marker: std::marker::PhantomData>, } -impl LazyHeap { - /// Pops and owns the front allocation. Does **not** sort. - #[inline] - fn unlink_front(&mut self) -> Option { - let ptr = self.data.pop_front()?; - // SAFETY: LazyHeap owns the sole Box for every pointer it stores. - Some(*unsafe { Box::from_raw(ptr.as_ptr()) }) - } - - /// Pops and owns the back allocation. Does **not** sort. - #[inline] - fn unlink_back(&mut self) -> Option { - let ptr = self.data.pop_back()?; - // SAFETY: LazyHeap owns the sole Box for every pointer it stores. - Some(*unsafe { Box::from_raw(ptr.as_ptr()) }) - } -} - -impl LazyHeap { +impl LazyHeap { /// Creates a new, empty `LazyHeap`. pub fn new() -> Self { Self { - data: std::collections::VecDeque::new(), + data: Vec::new(), + pending: Vec::new(), + next_seq: 0, is_sorted: true, _marker: std::marker::PhantomData, } @@ -60,55 +102,215 @@ impl LazyHeap { self.data.is_empty() } + #[inline(always)] + unsafe fn node(&self, i: usize) -> &T { + unsafe { self.data.get_unchecked(i).as_ref() } + } + + #[inline(always)] + unsafe fn node_mut(&mut self, i: usize) -> &mut T { + unsafe { self.data.get_unchecked_mut(i).as_mut() } + } + + /// `(rec, seq)` ordering between two buffer indices. + #[inline(always)] + unsafe fn less(&self, a: usize, b: usize) -> bool { + unsafe { + let (sa, sb) = (self.node(a).heap_slot(), self.node(b).heap_slot()); + (sa.rec, sa.seq) < (sb.rec, sb.seq) + } + } + + #[inline(always)] + unsafe fn swap(&mut self, a: usize, b: usize) { + self.data.swap(a, b); + unsafe { + self.node_mut(a).heap_slot_mut().pos = a as u32; + self.node_mut(b).heap_slot_mut().pos = b as u32; + } + } + + /// Returns `true` if the element moved. + unsafe fn sift_up(&mut self, mut i: usize) -> bool { + let start = i; + while i > 0 { + let parent = (i - 1) / 2; + if unsafe { !self.less(i, parent) } { + break; + } + unsafe { self.swap(i, parent) }; + i = parent; + } + i != start + } + + /// Returns `true` if the element moved. + unsafe fn sift_down(&mut self, mut i: usize) -> bool { + let start = i; + let len = self.data.len(); + loop { + let left = 2 * i + 1; + if left >= len { + break; + } + let right = left + 1; + let child = if right < len && unsafe { self.less(right, left) } { + right + } else { + left + }; + if unsafe { !self.less(child, i) } { + break; + } + unsafe { self.swap(i, child) }; + i = child; + } + i != start + } + + /// Moves the element at `i` to its correct place after its recorded key changed. + #[inline] + unsafe fn fix(&mut self, i: usize) -> bool { + unsafe { self.sift_up(i) || self.sift_down(i) } + } + + /// Restores the invariant at the root after lazy increases, so the root is the + /// true minimum. Returns `true` if the layout changed. + fn repair_root(&mut self) -> bool { + let mut moved = false; + while !self.data.is_empty() { + let root = unsafe { self.node_mut(0) }; + let key = root.heap_key(); + let slot = root.heap_slot_mut(); + if slot.rec == key { + break; + } + slot.rec = key; + if !unsafe { self.sift_down(0) } { + break; + } + moved = true; + } + moved + } + /// Inserts `value` into the heap and returns a [`Cursor`] to it. /// - /// The returned cursor is **non-owning**. Store it in an external structure - /// (e.g. a `hashbrown::RawTable`) for later removal via [`remove`](Self::remove). - /// Never reconstruct a `Box` from it. - /// - /// This call marks the heap as unsorted; the next ordered operation - /// triggers a full sort. + /// The returned cursor is **non-owning**. Never reconstruct a `Box` from it. /// /// # Complexity - /// Amortised O(1). + /// O(log n). #[inline] - pub fn push(&mut self, value: T) -> Cursor { + pub fn push(&mut self, mut value: T) -> Cursor { + let pos = self.data.len(); + assert!(pos < NOT_PENDING as usize, "LazyHeap: too many elements"); + let seq = self.next_seq; + self.next_seq = self.next_seq.wrapping_add(1); + + let key = value.heap_key(); + *value.heap_slot_mut() = HeapSlot { + pos: pos as u32, + seq, + rec: key, + pidx: NOT_PENDING, + }; + // SAFETY: Box::into_raw is guaranteed non-null. let ptr = unsafe { NonNull::new_unchecked(Box::into_raw(Box::new(value))) }; - self.data.push_back(ptr); - self.is_sorted = false; + self.data.push(ptr); + + // Appending the largest seq with a key >= the current last element keeps + // a sorted buffer sorted (common for monotonic keys like expiry times). + if self.is_sorted && pos > 0 { + self.is_sorted = unsafe { !self.less(pos, pos - 1) }; + } + unsafe { self.sift_up(pos) }; Cursor(ptr) } - /// Marks the heap's order as invalid without re-sorting immediately. + /// Marks the sorted order as invalid (the heap invariant is still valid). /// - /// Call this after mutating an element's sort key through [`Cursor::as_mut`]. - /// The next ordered operation will then re-sort before proceeding. + /// Call this after a lazy key increase through [`Cursor::element_mut`]. #[inline] pub fn mark_unsorted(&mut self) { self.is_sorted = false; } - /// Sorts the backing buffer with `compare` if it is not already sorted and - /// then returns `true`. + /// Records that the element behind `cursor` had an arbitrary key change + /// (increase or decrease). O(1); the element is re-positioned lazily on the + /// next [`front`](Self::front) / [`pop_front`](Self::pop_front) / sort, so the + /// buffer layout does not move now. + pub fn mark_changed(&mut self, cursor: Cursor) { + self.is_sorted = false; + // SAFETY: cursor belongs to this heap (caller contract). + let slot = unsafe { (*cursor.0.as_ptr()).heap_slot_mut() }; + if slot.pidx == NOT_PENDING { + slot.pidx = self.pending.len() as u32; + self.pending.push(cursor.0); + } + } + + /// Re-positions every pending element. Amortised O(p log n). + fn apply_pending(&mut self) { + while let Some(ptr) = self.pending.pop() { + // SAFETY: pending only holds live elements of this heap; removed + // elements are unlinked from `pending` in `remove_at`. + let i = unsafe { + let node = &mut *ptr.as_ptr(); + let key = node.heap_key(); + let slot = node.heap_slot_mut(); + slot.pidx = NOT_PENDING; + slot.rec = key; + slot.pos as usize + }; + unsafe { self.fix(i) }; + } + } + + /// Unlinks the element at buffer index `i` from `pending`, if it is there. + #[inline] + unsafe fn unlink_pending(&mut self, i: usize) { + let pidx = unsafe { self.node(i).heap_slot().pidx }; + if pidx == NOT_PENDING { + return; + } + self.pending.swap_remove(pidx as usize); + if let Some(moved) = self.pending.get_mut(pidx as usize) { + unsafe { moved.as_mut().heap_slot_mut().pidx = pidx }; + } + unsafe { self.node_mut(i).heap_slot_mut().pidx = NOT_PENDING }; + } + + /// Sorts the backing buffer if it is not already sorted and then returns `true`. /// - /// All ordered operations call this automatically. You can call it - /// manually to amortise the sort cost before a batch of [`front`](Self::front) / - /// [`get`](Self::get) accesses. + /// The order is by the live key, ties broken by insertion order. The `compare` + /// argument is kept for API compatibility and must agree with + /// [`HeapNode::heap_key`]. /// /// # Complexity /// O(n log n) when unsorted; O(1) when already sorted. #[inline] - pub fn sort_by(&mut self, compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> bool { + pub fn sort_by(&mut self, _compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> bool { if self.is_sorted { return false; } - if self.data.len() > 1 { - // SAFETY: every pointer in `self.data` is a live, heap-owned allocation. - unsafe { - self.data - .make_contiguous() - .sort_by(|a, b| compare(a.as_ref(), b.as_ref())); + + // SAFETY: every pointer in `self.data` is a live, heap-owned allocation. + unsafe { + for ptr in self.data.iter_mut() { + let node = ptr.as_mut(); + let key = node.heap_key(); + let slot = node.heap_slot_mut(); + slot.rec = key; + slot.pidx = NOT_PENDING; + } + self.pending.clear(); + self.data.sort_unstable_by(|a, b| { + let (sa, sb) = (a.as_ref().heap_slot(), b.as_ref().heap_slot()); + (sa.rec, sa.seq).cmp(&(sb.rec, sb.seq)) + }); + for (i, ptr) in self.data.iter_mut().enumerate() { + ptr.as_mut().heap_slot_mut().pos = i as u32; } } self.is_sorted = true; @@ -118,113 +320,115 @@ impl LazyHeap { /// Returns a cursor to the smallest (front) element without removing it, /// or `None` if the heap is empty. /// - /// Sorts the heap first if necessary. + /// # Complexity + /// O(1) amortised. #[inline] - pub fn front(&mut self, compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> Option> { - self.sort_by(compare); - self.data.front().copied().map(Cursor) + pub fn front(&mut self, _compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> Option> { + self.apply_pending(); + if self.repair_root() { + self.is_sorted = false; + } + self.data.first().copied().map(Cursor) } - /// Returns a cursor to the largest (back) element without removing it, - /// or `None` if the heap is empty. + /// Returns a cursor to the largest (back) element without removing it. /// - /// Sorts the heap first if necessary. + /// Sorts the heap first if necessary (O(n log n)); rarely needed. #[inline] pub fn back(&mut self, compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> Option> { self.sort_by(compare); - self.data.back().copied().map(Cursor) + self.data.last().copied().map(Cursor) } /// Returns a cursor to the element at position `index`, or `None` if out /// of bounds. /// - /// The index is only meaningful after the heap has been sorted — consider - /// calling [`sort_by`](Self::sort_by) first. + /// The index is only meaningful after the heap has been sorted — call + /// [`sort_by`](Self::sort_by) first. #[inline] pub fn get(&self, index: usize) -> Option> { self.data.get(index).copied().map(Cursor) } + /// Removes the element at buffer index `i` and returns it. + unsafe fn remove_at(&mut self, i: usize) -> T { + unsafe { self.unlink_pending(i) }; + let last = self.data.len() - 1; + if i != last { + unsafe { self.swap(i, last) }; + } + // SAFETY: data is non-empty because `i` was a valid index. + let ptr = unsafe { self.data.pop().unwrap_unchecked() }; + if i < self.data.len() { + unsafe { self.fix(i) }; + } + // Removing the last element of a sorted buffer keeps it sorted. + if i != last { + self.is_sorted = false; + } + // SAFETY: LazyHeap holds the sole Box for this pointer. + *unsafe { Box::from_raw(ptr.as_ptr()) } + } + /// Removes and returns the smallest (front) element, or `None` if empty. /// - /// Sorts the heap first if necessary. - /// /// # Complexity - /// O(n log n) when unsorted; O(n) when already sorted (front removal from - /// a `VecDeque` shifts elements). + /// O(log n). #[inline] pub fn pop_front(&mut self, compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> Option { - self.sort_by(compare); - self.unlink_front() + self.front(compare)?; + Some(unsafe { self.remove_at(0) }) } /// Removes and returns the largest (back) element, or `None` if empty. /// /// Sorts the heap first if necessary. - /// - /// # Complexity - /// O(n log n) when unsorted; O(1) when already sorted. #[inline] pub fn pop_back(&mut self, compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> Option { self.sort_by(compare); - self.unlink_back() + let last = self.data.len().checked_sub(1)?; + Some(unsafe { self.remove_at(last) }) } /// Removes and returns the element identified by `cursor`. /// - /// Sorts the heap first if necessary, then performs a linear scan to - /// locate the element by pointer identity. - /// /// # Complexity - /// O(n log n) when unsorted; O(n) when already sorted. + /// O(log n). pub fn remove( &mut self, cursor: Cursor, - compare: impl Fn(&T, &T) -> std::cmp::Ordering, + _compare: impl Fn(&T, &T) -> std::cmp::Ordering, ) -> T { debug_assert!(!self.data.is_empty()); - - // Fast path: single element — no need to sort or scan. - if self.data.len() == 1 { - return self.unlink_back().unwrap(); - } - - self.sort_by(compare); - - let index = self - .data - .iter() - .position(|ptr| cursor.0 == *ptr) - .expect("cursor does not belong to this LazyHeap"); - - // SAFETY: `index` was just returned by `position`, so it is in bounds. - // LazyHeap holds the sole Box for this pointer; the cursor is non-owning. - let ptr = unsafe { self.data.remove(index).unwrap_unchecked() }; - *unsafe { Box::from_raw(ptr.as_ptr()) } + // SAFETY: cursor belongs to this heap (caller contract). + let i = unsafe { cursor.element().heap_slot().pos as usize }; + debug_assert!(std::ptr::eq(self.data[i].as_ptr(), cursor.0.as_ptr())); + unsafe { self.remove_at(i) } } /// Returns an iterator that yields a [`Cursor`] for each element in sorted /// order. /// - /// Sorts the heap first if necessary. The returned [`Iter`] holds raw + /// Sorts the heap first if necessary. The returned [`RawIter`] holds raw /// pointers into the backing buffer; do not mutate or drop the heap while /// it is alive. #[inline] pub fn iter(&mut self, compare: impl Fn(&T, &T) -> std::cmp::Ordering) -> RawIter { self.sort_by(compare); - let (a, b) = self.data.as_slices(); RawIter { - first: utils::RawSliceIter::new(a), - second: utils::RawSliceIter::new(b), + first: utils::RawSliceIter::new(self.data.as_slice()), + second: utils::RawSliceIter::new(&[]), } } /// Removes all elements, dropping each one. - /// - /// The heap is empty and considered sorted after this call. #[inline] pub fn clear(&mut self) { - while self.unlink_back().is_some() {} + self.pending.clear(); + while let Some(ptr) = self.data.pop() { + // SAFETY: LazyHeap owns the sole Box for every pointer it stores. + drop(unsafe { Box::from_raw(ptr.as_ptr()) }); + } self.is_sorted = true; } @@ -236,43 +440,48 @@ impl LazyHeap { } } -impl Default for LazyHeap { +impl Default for LazyHeap { fn default() -> Self { Self::new() } } -impl Drop for LazyHeap { +impl Drop for LazyHeap { fn drop(&mut self) { - struct DropGuard<'a, T>(&'a mut LazyHeap); + struct DropGuard<'a, T: HeapNode>(&'a mut LazyHeap); - impl<'a, T> Drop for DropGuard<'a, T> { + impl<'a, T: HeapNode> Drop for DropGuard<'a, T> { fn drop(&mut self) { - // Continue the same loop we do below. This only runs when a destructor has - // panicked. If another one panics this will abort. - while self.0.unlink_back().is_some() {} + // Only runs when a destructor panicked; keep freeing the rest. + while let Some(ptr) = self.0.data.pop() { + drop(unsafe { Box::from_raw(ptr.as_ptr()) }); + } } } - // Wrap self so that if a destructor panics, we can try to keep looping let guard = DropGuard(self); - while guard.0.unlink_back().is_some() {} + while let Some(ptr) = guard.0.data.pop() { + drop(unsafe { Box::from_raw(ptr.as_ptr()) }); + } std::mem::forget(guard); } } /// A non-owning, pointer-sized handle to an element stored in a [`LazyHeap`]. /// -/// Think of `Cursor` as a stable address you can cache in an external data -/// structure (e.g. `hashbrown::raw::RawTable`) and later hand back to -/// [`LazyHeap::remove`] for cheap lookup and removal. It carries **no -/// ownership**: every allocation is owned exclusively by the heap that -/// produced the cursor. -/// /// Using a stale cursor is undefined behaviour. -#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] +#[derive(PartialEq, Eq, Hash, Debug)] pub struct Cursor(NonNull); +impl Clone for Cursor { + #[inline(always)] + fn clone(&self) -> Self { + *self + } +} + +impl Copy for Cursor {} + impl Cursor { /// Returns a shared reference to the value this cursor points to. /// @@ -280,29 +489,25 @@ impl Cursor { /// The cursor must be valid (see the [type-level docs](Self)). #[inline] pub unsafe fn element(&self) -> &T { - self.0.as_ref() + unsafe { self.0.as_ref() } } /// Returns a mutable reference to the value this cursor points to. /// - /// If the mutation changes any field that affects sort order, you **must** - /// call [`LazyHeap::invalidate`] afterwards so the heap re-sorts before - /// the next ordered operation. + /// The heap key may only be *increased* through this reference (followed by + /// [`LazyHeap::mark_unsorted`]); any other key change must be followed by + /// [`LazyHeap::mark_changed`]. /// /// # Safety /// - The cursor must be valid (see the [type-level docs](Self)). /// - No other reference to the same element may be alive simultaneously. #[inline] pub unsafe fn element_mut(&mut self) -> &mut T { - self.0.as_mut() + unsafe { self.0.as_mut() } } /// Returns the raw pointer underlying this cursor. /// - /// Prefer [`as_ref`](Self::as_ref) or [`as_mut`](Self::as_mut) for - /// element access. This exists for interoperability with APIs that require - /// a raw pointer (e.g. hashing into a `RawTable` by address). - /// /// **Never** reconstruct a `Box` from this pointer — doing so transfers /// ownership out of the heap and causes a double-free. #[inline] @@ -311,10 +516,10 @@ impl Cursor { } } -/// Raw iterator for [`VecDeque`] which doesn't have lifetime. +/// Raw iterator over the heap buffer which doesn't carry a lifetime. /// /// # Safety -/// You should track changes of [`VecDeque`] yourself. +/// You should track changes of the heap yourself. pub struct RawIter { first: utils::RawSliceIter>, second: utils::RawSliceIter>, @@ -326,16 +531,11 @@ impl Iterator for RawIter { #[inline] fn next(&mut self) -> Option { match self.first.next() { - Some(val) => Some( - // SAFETY: `val` is a valid `NonNull>` pointing into the - // first slice of the `VecDeque`. The pointee is `Copy` and remains - // valid as long as the `VecDeque` is alive and unmodified, which the - // caller is required to uphold per this type's safety contract. - Cursor(unsafe { val.read() }), - ), + // SAFETY: `val` points into the heap buffer, which the caller keeps + // alive and unmodified per this type's safety contract. + Some(val) => Some(Cursor(unsafe { val.read() })), None => { std::mem::swap(&mut self.first, &mut self.second); - // SAFETY: same as above. self.first.next().map(|val| Cursor(unsafe { val.read() })) } } @@ -360,9 +560,9 @@ impl Clone for RawIter { } } -unsafe impl Send for LazyHeap {} -unsafe impl Sync for LazyHeap {} -unsafe impl Send for RawIter {} -unsafe impl Sync for RawIter {} -unsafe impl Send for Cursor {} -unsafe impl Sync for Cursor {} +unsafe impl Send for LazyHeap {} +unsafe impl Sync for LazyHeap {} +unsafe impl Send for RawIter {} +unsafe impl Sync for RawIter {} +unsafe impl Send for Cursor {} +unsafe impl Sync for Cursor {} diff --git a/src/policies/lfupolicy.rs b/src/policies/lfupolicy.rs index 0742d4f..0fae1d5 100644 --- a/src/policies/lfupolicy.rs +++ b/src/policies/lfupolicy.rs @@ -1,6 +1,7 @@ use crate::hashbrown; use crate::internal::alias; use crate::internal::lazyheap; +use crate::internal::lazyheap::HeapNode; use crate::internal::pickle::Builder; use crate::internal::utils; use crate::policies::traits; @@ -16,9 +17,9 @@ macro_rules! compare_fn { }; } -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord)] #[repr(transparent)] -pub struct Frequency(u128); +pub struct Frequency(u64); impl Frequency { #[inline(always)] @@ -33,6 +34,26 @@ pub struct FrequencyHandle { value: alias::PyObject, size: usize, frequency: Frequency, + slot: lazyheap::HeapSlot, +} + +impl lazyheap::HeapNode for FrequencyHandle { + type Key = u64; + + #[inline(always)] + fn heap_key(&self) -> u64 { + self.frequency.0 + } + + #[inline(always)] + fn heap_slot(&self) -> &lazyheap::HeapSlot { + &self.slot + } + + #[inline(always)] + fn heap_slot_mut(&mut self) -> &mut lazyheap::HeapSlot { + &mut self.slot + } } impl FrequencyHandle { @@ -70,14 +91,15 @@ impl FrequencyHandle { key, value, size, - frequency: Frequency(frequency), + frequency: Frequency(u64::try_from(frequency).unwrap_or(u64::MAX)), + slot: Default::default(), }) } /// Returns the frequency. #[inline] pub fn frequency(&self) -> u128 { - self.frequency.0 + self.frequency.0 as u128 } /// Consumes `self` and returns the [`utils::PrecomputedHashObject`]. @@ -114,6 +136,7 @@ impl FrequencyHandle { value: self.value.clone_ref(py), size: self.size, frequency: self.frequency, + slot: Default::default(), } } } @@ -163,10 +186,14 @@ impl traits::OccupiedExt for Occupied<'_> { .saturating_sub(cursor.element().size()) .saturating_add(new.size()); + let mut new = new; + *new.heap_slot_mut() = *cursor.element().heap_slot(); let old = std::mem::replace(cursor.element_mut(), new); cursor.element_mut().frequency.increment(); - self.policy.heap.mark_unsorted(); + + // The key may move either way; re-position lazily (no layout change now). + self.policy.heap.mark_changed(*cursor); old } @@ -332,8 +359,10 @@ impl PolicyExt for LFUPolicy { match cursor { Some(cursor) => unsafe { - // increment frequency + // Lazy priority increase: O(1), no buffer movement. The heap repairs + // its root on the next eviction; ordered iteration re-sorts. cursor.element_mut().frequency.increment(); + self.heap.mark_unsorted(); Ok(Some(cursor.element())) }, @@ -494,7 +523,7 @@ impl PolicyExt for LFUPolicy { let mut tuple = list.begin_tuple(3)?; tuple.push(handle.key.as_ref())?; tuple.push(handle.value())?; - tuple.push(handle.frequency.0)?; + tuple.push(handle.frequency.0 as u128)?; tuple.end()?; } diff --git a/src/policies/vttlpolicy.rs b/src/policies/vttlpolicy.rs index 3b923f5..c66c2d0 100644 --- a/src/policies/vttlpolicy.rs +++ b/src/policies/vttlpolicy.rs @@ -7,6 +7,7 @@ use crate::policies::traits; use crate::policies::traits::HandleExt; use crate::policies::traits::PolicyExt; +use crate::internal::lazyheap::HeapNode; pub use crate::policies::common::Shared; use crate::policies::traits::SharedExt; @@ -33,6 +34,36 @@ pub struct ExpiringHandle { size: usize, /// Configured ttl for handle. `None` means has no ttl. expires_at: Option, + /// Heap bookkeeping (position, insertion order, recorded priority). + slot: lazyheap::HeapSlot, +} + +impl lazyheap::HeapNode for ExpiringHandle { + /// Expiry as nanoseconds since the Unix epoch (earliest first); `None` sorts + /// last as `u64::MAX`. Representable until year 2554. + type Key = u64; + + #[inline(always)] + fn heap_key(&self) -> u64 { + match self.expires_at { + None => u64::MAX, + Some(t) => t.duration_since(std::time::UNIX_EPOCH).map_or(0, |d| { + u64::try_from(d.as_nanos()) + .unwrap_or(u64::MAX - 1) + .min(u64::MAX - 1) + }), + } + } + + #[inline(always)] + fn heap_slot(&self) -> &lazyheap::HeapSlot { + &self.slot + } + + #[inline(always)] + fn heap_slot_mut(&mut self) -> &mut lazyheap::HeapSlot { + &mut self.slot + } } impl ExpiringHandle { @@ -72,6 +103,7 @@ impl ExpiringHandle { value, size, expires_at: expires_at.map(Into::into), + slot: Default::default(), }) } @@ -119,6 +151,7 @@ impl ExpiringHandle { value: self.value.clone_ref(py), size: self.size, expires_at: self.expires_at, + slot: Default::default(), } } } @@ -168,9 +201,12 @@ impl traits::OccupiedExt for Occupied<'_> { .saturating_sub(cursor.element().size()) .saturating_add(new.size()); + let mut new = new; + *new.heap_slot_mut() = *cursor.element().heap_slot(); let old = std::mem::replace(cursor.element_mut(), new); - self.policy.heap.mark_unsorted(); + // The key may move either way; re-position lazily (no layout change now). + self.policy.heap.mark_changed(*cursor); old } } From 87c2d386c213c964625f1c329f6dd3be937ed0f4 Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Wed, 28 Oct 2026 14:05:26 +0300 Subject: [PATCH 2/4] test: fix race in TTLCache test_sweep_interval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Entries expired at t≈3s and the sweeper woke via Event.wait(3) at the same moment. Event.wait() may return a few ms early (Windows timer resolution is ~15.6 ms), so the sweep could run just before expiry; the next one was at t≈6s, after the assert at 3.5s. Seen as `assert 3 == 0` on windows-latest. Use ttl=1 with sweep_interval=1 and assert at 2.5s, so at least one full sweep runs after expiry. Also makes the test a second faster. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KRuJihprbupFprYtRYAs92 --- tests/test_impls.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_impls.py b/tests/test_impls.py index 20141c5..6d1ff19 100644 --- a/tests/test_impls.py +++ b/tests/test_impls.py @@ -1550,11 +1550,14 @@ def test_items_with_expire(self): assert isinstance(ttl, float) def test_sweep_interval(self): - obj = cachebox.TTLCache(10, 3, {1: 1, 2: 2, 3: 3}, sweep_interval=3) + # Entries expire at t≈1s and the sweeper wakes every second. The sweep at + # t≈1s races with expiry (Event.wait() may return a few ms early, notably on + # Windows), so assert only after the next sweep at t≈2s has surely run. + obj = cachebox.TTLCache(10, 1, {1: 1, 2: 2, 3: 3}, sweep_interval=1) # __len__ doesn't call expire itself assert len(obj) == 3 - time.sleep(3.5) + time.sleep(2.5) assert len(obj) == 0 From f12fb3d6a70f91fde24738303ab0e65efc71c6c8 Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Wed, 28 Oct 2026 14:13:13 +0300 Subject: [PATCH 3/4] test: LFU/VTTL eviction-order regressions and model-based tests - LFU: popitem()/drain() after reads only (the bug fixed in the previous commit), popitem() agreeing with least_frequently_used(), frequency reset on replace, replace during iteration yielding each key once, eviction of the coldest keys in a large cache. - VTTL: popitem() following expiry after TTL changes, entries without ttl evicted in insertion order. - Hypothesis state machines comparing LFUCache and VTTLCache with pure-Python reference models (eviction, popitem, least_frequently_used, ordered iteration). The LFU regressions and the LFU model test fail on main. Co-Authored-By: Claude Opus 5.5 --- tests/test_impls.py | 92 +++++++++++++++++++++ tests/test_policy_model.py | 163 +++++++++++++++++++++++++++++++++++++ 2 files changed, 255 insertions(+) create mode 100644 tests/test_policy_model.py diff --git a/tests/test_impls.py b/tests/test_impls.py index 6d1ff19..4900179 100644 --- a/tests/test_impls.py +++ b/tests/test_impls.py @@ -1013,6 +1013,78 @@ def test_generation_version_on_least_frequently_used(self): for _ in c: c.least_frequently_used() + def test_popitem_after_reads_only(self): + """Reads between two popitem() calls must be taken into account. + + Regression: get() incremented the frequency without invalidating the + order established by the previous popitem(), so the second popitem() + evicted the most frequently used key. + """ + c = self.create_cache(10) + for k in "abcd": + c[k] = k + assert c.popitem() == ("a", "a") + self._hit(c, "b", 10) + assert c.popitem() == ("c", "c") + assert c.popitem() == ("d", "d") + assert c.popitem() == ("b", "b") + + def test_popitem_agrees_with_least_frequently_used(self): + c = self.create_cache(10, {i: i for i in range(8)}) + for step in range(8): + for k in list(c.keys())[: step % 3 + 1]: + self._hit(c, k, step + 1) + expected = c.least_frequently_used() + key, _ = c.popitem() + assert key == expected + + def test_drain_after_reads_only(self): + c = self.create_cache(10) + for k in "abcde": + c[k] = k + c.popitem() # "a" + self._hit(c, "b", 5) + self._hit(c, "c", 3) + assert c.drain(2) == 2 + assert sorted(c.keys()) == ["b", "c"] + + def test_replace_lowers_priority_for_eviction(self): + # Replacing a value resets the frequency (as before), so a hot key + # that was just replaced becomes the eviction candidate again. + c = self.create_cache(3) + c["hot"] = 0 + c["warm"] = 0 + c["cold"] = 0 + self._hit(c, "hot", 10) + self._hit(c, "warm", 3) + self._hit(c, "cold", 2) + c.popitem() # "cold" — establishes the order + c["hot"] = 1 # frequency reset + assert c.least_frequently_used() == "hot" + assert c.popitem()[0] == "hot" + + def test_replace_during_iteration_yields_each_key_once(self): + c = self.create_cache(10, {i: i for i in range(6)}) + self._hit(c, 0, 5) + self._hit(c, 3, 2) + seen = [] + for k in c: + c[k] = "new" # changes priorities but must not reorder the live iterator + seen.append(k) + assert sorted(seen) == list(range(6)) + + def test_large_cache_evicts_coldest(self): + n = 2000 + c = self.create_cache(n) + for i in range(n): + c[i] = i + for i in range(n): + self._hit(c, i, 1 + (i % 7)) + cold = {i for i in range(n) if i % 7 == 0} + assert c.drain(len(cold)) == len(cold) + assert not cold & set(c.keys()) + assert len(c) == n - len(cold) + class TestTTLCache( mixins.InitializeMixin, @@ -1810,6 +1882,26 @@ def test_popitem_on_empty_raises(self): with pytest.raises(KeyError): c.popitem() + def test_popitem_follows_expiry_after_ttl_changes(self): + c = cachebox.VTTLCache(10) + c.insert("a", 1, ttl=100) + c.insert("b", 2, ttl=200) + c.insert("c", 3, ttl=300) + assert c.popitem()[0] == "a" # establishes the order + c.insert("c", 3, ttl=50) # moved to the front + c.insert("b", 2, ttl=None) # moved to the back + c.insert("d", 4, ttl=150) + assert [c.popitem()[0] for _ in range(3)] == ["c", "d", "b"] + + def test_entries_without_ttl_evicted_in_insertion_order(self): + c = cachebox.VTTLCache(3) + c.insert("x", 1, ttl=None) + c.insert("y", 2, ttl=None) + c.insert("z", 3, ttl=None) + c.insert("w", 4, ttl=None) # evicts the oldest permanent entry + assert sorted(c.keys()) == ["w", "y", "z"] + assert [c.popitem()[0] for _ in range(3)] == ["y", "z", "w"] + def test_expire_removes_stale_items(self): c = self.create_cache() c.insert("stale", "s", ttl=0.1) diff --git a/tests/test_policy_model.py b/tests/test_policy_model.py new file mode 100644 index 0000000..39d2b57 --- /dev/null +++ b/tests/test_policy_model.py @@ -0,0 +1,163 @@ +"""Differential tests: LFUCache and VTTLCache against small pure-Python models. + +Hypothesis drives random sequences of operations and checks eviction order, +``popitem``, ``least_frequently_used`` and ordered iteration after every step. +LFU membership is checked with ``peek`` because ``key in LFUCache`` counts as +an access. +""" + +import time + +from hypothesis import assume, settings +from hypothesis import strategies as st +from hypothesis.stateful import RuleBasedStateMachine, invariant, precondition, rule + +import cachebox + +KEYS = st.integers(min_value=0, max_value=15) +_MISSING = object() + + +class LFUModel(RuleBasedStateMachine): + """Reference: evict the minimal (frequency, insertion order). + + Inserting a new key starts at frequency 0, every read adds 1, and + replacing a value resets the frequency to 1 while keeping insertion order. + """ + + MAXSIZE = 6 + + def __init__(self): + super().__init__() + self.cache = cachebox.LFUCache(self.MAXSIZE) + self.model = {} # key -> [frequency, seq, value] + self.seq = 0 + + def _order(self): + return sorted(self.model, key=lambda k: (self.model[k][0], self.model[k][1])) + + @rule(key=KEYS, value=st.integers()) + def set(self, key, value): + if key in self.model: + self.model[key][0] = 1 + self.model[key][2] = value + else: + if len(self.model) >= self.MAXSIZE: + del self.model[self._order()[0]] + self.model[key] = [0, self.seq, value] + self.seq += 1 + self.cache[key] = value + + @rule(key=KEYS) + def get(self, key): + got = self.cache.get(key, _MISSING) + if key in self.model: + self.model[key][0] += 1 + assert got == self.model[key][2] + else: + assert got is _MISSING + + @precondition(lambda self: self.model) + @rule() + def popitem(self): + expected = self._order()[0] + key, value = self.cache.popitem() + assert key == expected + assert value == self.model.pop(key)[2] + + @rule(key=KEYS) + def delete(self, key): + if key in self.model: + del self.model[key] + del self.cache[key] + + @precondition(lambda self: self.model) + @rule(data=st.data()) + def least_frequently_used(self, data): + n = data.draw(st.integers(min_value=0, max_value=len(self.model) - 1)) + assert self.cache.least_frequently_used(n) == self._order()[n] + + @rule() + def iterate(self): + assert list(self.cache.keys()) == self._order() + + @invariant() + def same_contents(self): + assert len(self.cache) == len(self.model) + for key, (_, _, value) in self.model.items(): + assert self.cache.peek(key, _MISSING) == value + + +class VTTLModel(RuleBasedStateMachine): + """Reference: evict the earliest expiry; entries without ttl last. + + TTLs are whole seconds far in the future so nothing expires during the test. + Live entries never share a TTL, so their expiries differ by at least a + second and clock resolution cannot reorder them; entries without ttl are + ordered by insertion. + """ + + MAXSIZE = 6 + + def __init__(self): + super().__init__() + self.cache = cachebox.VTTLCache(self.MAXSIZE) + self.model = {} # key -> [priority, seq] + self.ttls = {} # key -> ttl, to keep live TTLs distinct + self.seq = 0 + + def _order(self): + return sorted(self.model, key=lambda k: (self.model[k][0], self.model[k][1])) + + @rule(key=KEYS, ttl=st.none() | st.integers(min_value=1_000, max_value=1_000_000)) + def insert(self, key, ttl): + if ttl is not None: + assume(all(t != ttl for k, t in self.ttls.items() if k != key)) + if key not in self.model and len(self.model) >= self.MAXSIZE: + evicted = self._order()[0] + del self.model[evicted] + self.ttls.pop(evicted, None) + self.cache.insert(key, key, ttl) + priority = (1, 0.0) if ttl is None else (0, time.time() + ttl) + self.ttls[key] = ttl + if key in self.model: + self.model[key][0] = priority + else: + self.model[key] = [priority, self.seq] + self.seq += 1 + + @precondition(lambda self: self.model) + @rule() + def popitem(self): + expected = self._order()[0] + key, _ = self.cache.popitem() + assert key == expected + del self.model[key] + self.ttls.pop(key, None) + + @rule(key=KEYS) + def delete(self, key): + if key in self.model: + del self.model[key] + del self.cache[key] + self.ttls.pop(key, None) + + @rule() + def iterate(self): + assert list(self.cache.keys()) == self._order() + + @invariant() + def same_contents(self): + assert len(self.cache) == len(self.model) + for key in self.model: + # VTTLCache has no peek(); membership does not affect its order. + assert key in self.cache + + +_settings = settings(max_examples=200, stateful_step_count=60, deadline=None) + +TestLFUModel = LFUModel.TestCase +TestLFUModel.settings = _settings + +TestVTTLModel = VTTLModel.TestCase +TestVTTLModel.settings = _settings From 4266079d4a1b5c2a151ca53fc309c4bb0fb6c0cc Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Wed, 28 Oct 2026 16:02:36 +0300 Subject: [PATCH 4/4] test: skip LFU replace-during-iteration test on free-threaded builds On Py_GIL_DISABLED builds LFU replace() bumps the generation version by design, so iterating while replacing raises RuntimeError (the same behaviour test_generation_version_on_insert expects). Skip the new test there, as it checks the GIL build's guarantee only. --- tests/test_impls.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_impls.py b/tests/test_impls.py index 4900179..34e6943 100644 --- a/tests/test_impls.py +++ b/tests/test_impls.py @@ -1,5 +1,6 @@ import subprocess import sys +import sysconfig import time import typing from datetime import datetime, timedelta, timezone @@ -1063,6 +1064,10 @@ def test_replace_lowers_priority_for_eviction(self): assert c.least_frequently_used() == "hot" assert c.popitem()[0] == "hot" + @pytest.mark.skipif( + bool(sysconfig.get_config_var("Py_GIL_DISABLED")), + reason="free-threaded builds invalidate iterators on replace by design", + ) def test_replace_during_iteration_yields_each_key_once(self): c = self.create_cache(10, {i: i for i in range(6)}) self._hit(c, 0, 5)