Repository navigation
perf: indexed binary heap for LFU/VTTL; fix LFU popitem evicting hot … - #112
Merged
Merged
Conversation
…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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KRuJihprbupFprYtRYAs92
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRuJihprbupFprYtRYAs92
- 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 <noreply@anthropic.com>
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.
Contributor
Author
|
fixed tests for free-threaded 3.14t & 3.15t |
Owner
|
I will release a new version in about 2-3 hours |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…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:
Bug fix: LFUCache.get() incremented the frequency without invalidating the sorted order, so popitem() could evict the most frequently used key:
Notes: