Skip to content

perf: bound L1 cache access-history retention #1295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
groupthinking wants to merge 1 commit into from
Closed

perf: bound L1 cache access-history retention #1295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/youtube_extension/backend/services/intelligent_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import statistics
import threading
import time
from collections import OrderedDict, defaultdict
from collections import OrderedDict, defaultdict, deque
from dataclasses import asdict, dataclass

# pickle removed for security
Expand Down Expand Up @@ -56,6 +56,16 @@ def _resolve_tag_write_limit(max_connections: int) -> int:
"""
return max(1, min(TAG_WRITE_CONCURRENCY, max_connections - TAG_WRITE_POOL_RESERVE))


# Hit timestamps retained per key to drive _calculate_adaptive_ttl(). That
# consumer reads only the first element, the last element and the length, so the
# window only has to be long enough for the ratio between them to be a stable
# frequency estimate rather than a two-sample artefact. Bounding it keeps L1's
# footprint proportional to the number of resident keys instead of to the total
# number of reads the process has ever served.
ACCESS_HISTORY_WINDOW = 64


class DateTimeEncoder(json.JSONEncoder):
"""JSON encoder that handles datetime objects"""
def default(self, obj):
Expand Down Expand Up @@ -138,7 +148,11 @@ def __init__(self, name: str = "L1_Memory", max_size: int = 10000, max_size_byte
super().__init__(name, max_size)
self.max_size_bytes = max_size_bytes
self.cache: OrderedDict[str, CacheEntry] = OrderedDict()
self.access_patterns = defaultdict(list) # Track access patterns for intelligent TTL
# Bounded per key: see ACCESS_HISTORY_WINDOW. Entries are released when
# the key leaves the cache, via delete() or _evict_if_needed().
Comment on lines +151 to +152

@groupthinking groupthinking Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed — this is a valid, merge-blocking gap, not a false positive.

get() releases nothing on the TTL-expiry branch:

if entry.expires_at and datetime.now(timezone.utc) > entry.expires_at:
    del self.cache[key]
    self.stats.miss_count += 1
    return None

delete() and the patched _evict_if_needed() both pop the history, but this path does not — so any key that expires by TTL orphans its access_patterns[key] with no cache entry left to ever trigger cleanup. That is precisely the leak this PR sets out to bound, still open for expiring entries, so the "history footprint proportional to resident keys" guarantee does not hold once TTLs are in play.

Minimal in-scope fix — mirror the eviction path:

if entry.expires_at and datetime.now(timezone.utc) > entry.expires_at:
    del self.cache[key]
    self.access_patterns.pop(key, None)   # release history on expiry, as delete()/_evict_if_needed() do
    self.stats.miss_count += 1
    return None

Suggested regression (parallels test_evicted_key_history_is_released): set() a key with a short ttl, force expires_at into the past, get() it (asserting the None/miss), then assert key not in cache.access_patterns.

Separate, pre-existing observation worth a follow-up rather than folding in here: this same expiry branch also never decrements stats.total_entries / stats.total_size_bytes, so an expired-then-get()'d entry leaves the byte/entry accounting overstated until the next set() of that key. That predates #1295 and is out of its stated scope — flagging it, not asking for it in this PR.

Once the pop and its regression land, this PR is clean; nothing else here is blocking.

Generated by Claude Code

self.access_patterns = defaultdict(
lambda: deque(maxlen=ACCESS_HISTORY_WINDOW)
) # Track access patterns for intelligent TTL

async def get(self, key: str) -> Optional[Any]:
"""Get value from in-memory cache"""
Expand Down Expand Up @@ -256,6 +270,11 @@ async def _evict_if_needed(self, new_size_bytes: int):
entry = self.cache[oldest_key]
del self.cache[oldest_key]

# Release the hit history too. delete() already does this; without
# it here an evicted key's history would be retained forever, with
# no cache entry left to ever trigger its cleanup.
self.access_patterns.pop(oldest_key, None)

self.stats.total_entries -= 1
self.stats.total_size_bytes -= entry.size_bytes
self.stats.eviction_count += 1
Expand Down
164 changes: 164 additions & 0 deletions tests/unit/test_intelligent_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1841,3 +1841,167 @@ def test_subsequent_call_uses_ema(self):
layer._update_avg_access_time(20.0)
# EMA: 0.1 * 20 + 0.9 * 10 = 11.0
assert layer.stats.avg_access_time_ms == pytest.approx(11.0, rel=0.01)


# ---------------------------------------------------------------------------
# InMemoryCacheLayer — access-history retention is bounded
# ---------------------------------------------------------------------------


class TestAccessHistoryRetentionIsBounded:
"""A key that is read repeatedly must not accumulate one timestamp per hit
forever. Before this was bounded, ``access_patterns[key]`` was a plain list
appended to on every cache hit and never trimmed, so a hot key's history
grew without limit for the lifetime of the process."""

async def test_hot_key_history_stays_within_window(self):
from youtube_extension.backend.services.intelligent_cache import (
ACCESS_HISTORY_WINDOW,
)

layer = InMemoryCacheLayer("hot", max_size=100, max_size_bytes=1024 * 1024)
await layer.set("hot", "value")

hits = ACCESS_HISTORY_WINDOW * 20
for _ in range(hits):
await layer.get("hot")

retained = len(layer.access_patterns["hot"])
assert retained <= ACCESS_HISTORY_WINDOW, (
f"history for a single key grew to {retained} entries after {hits} "
f"hits; expected it to stay within ACCESS_HISTORY_WINDOW="
f"{ACCESS_HISTORY_WINDOW}"
)

async def test_window_retains_the_most_recent_timestamps(self):
"""Bounding must drop the oldest samples, not the newest, so the
retained window still describes current behaviour."""
from youtube_extension.backend.services.intelligent_cache import (
ACCESS_HISTORY_WINDOW,
)

layer = InMemoryCacheLayer("recent", max_size=100, max_size_bytes=1024 * 1024)
await layer.set("k", "value")

for _ in range(ACCESS_HISTORY_WINDOW):
await layer.get("k")
boundary = time.time()
for _ in range(ACCESS_HISTORY_WINDOW):
await layer.get("k")

retained = list(layer.access_patterns["k"])
assert retained == sorted(retained), "retained timestamps lost their ordering"
assert all(ts >= boundary for ts in retained), (
"history retained samples recorded before the most recent "
f"{ACCESS_HISTORY_WINDOW} hits; the window is dropping the wrong end"
)


# ---------------------------------------------------------------------------
# InMemoryCacheLayer — eviction releases access history
# ---------------------------------------------------------------------------


class TestEvictionReleasesAccessHistory:
"""``delete()`` already drops a key's access history (see
``test_delete_cleans_access_patterns``). ``_evict_if_needed`` must do the
same: an evicted key leaves no cache entry behind, so nothing else will ever
reclaim its history."""

async def test_evicted_key_history_is_released(self):
layer = InMemoryCacheLayer("evict_one", max_size=1, max_size_bytes=1024 * 1024)
await layer.set("first", "value")
await layer.get("first")
assert "first" in layer.access_patterns

# Inserting a second key evicts "first" (max_size=1).
await layer.set("second", "value")

assert "first" not in layer.cache
assert "first" not in layer.access_patterns, (
"history for an evicted key was retained; nothing will ever "
"reclaim it because the cache entry is already gone"
)

async def test_eviction_churn_leaves_no_orphaned_histories(self):
"""The accumulating case: many keys cycle through a small cache. Every
key that is evicted must take its history with it, so the number of
tracked histories stays proportional to the number of resident keys."""
layer = InMemoryCacheLayer("churn", max_size=5, max_size_bytes=1024 * 1024)

for i in range(200):
key = f"k{i}"
await layer.set(key, "value")
await layer.get(key)

orphans = set(layer.access_patterns) - set(layer.cache)
assert not orphans, (
f"{len(orphans)} evicted keys still have access history retained "
f"while only {len(layer.cache)} entries are resident; this memory "
"is invisible to stats.total_size_bytes so the max_size_bytes "
"budget can never reclaim it"
)

async def test_resident_keys_keep_their_history(self):
"""Releasing evicted histories must not disturb keys still in the
cache — the fix must be a narrow cleanup, not a blanket clear."""
layer = InMemoryCacheLayer("keep", max_size=3, max_size_bytes=1024 * 1024)

for i in range(10):
await layer.set(f"k{i}", "value")
await layer.get(f"k{i}")

for key in layer.cache:
assert layer.access_patterns.get(key), (
f"resident key {key!r} lost its access history; adaptive TTL "
"would fall back to the base value for a live key"
)


# ---------------------------------------------------------------------------
# Adaptive TTL still consumes the bounded history
# ---------------------------------------------------------------------------


class TestAdaptiveTtlOverBoundedHistory:
"""``_calculate_adaptive_ttl`` reads ``accesses[0]``, ``accesses[-1]`` and
``len(accesses)``. Those must keep working over the bounded container, and
a saturated window must still classify a hot key as high-frequency."""

def _make_system(self, layer):
from youtube_extension.backend.services.intelligent_cache import (
IntelligentCacheSystem,
)

system = IntelligentCacheSystem.__new__(IntelligentCacheSystem)
system.layers = [layer]
system.adaptive_ttl_enabled = True
system.cache_warming_enabled = False
system.auto_invalidation_enabled = False
system.performance_history = []
system.optimization_suggestions = []
return system

async def test_saturated_window_still_yields_high_frequency_ttl(self):
from youtube_extension.backend.services.intelligent_cache import (
ACCESS_HISTORY_WINDOW,
)

layer = InMemoryCacheLayer("ttl", max_size=10, max_size_bytes=1024 * 1024)
await layer.set("k", "value")
# Far more hits than the window holds, all in a tight burst.
for _ in range(ACCESS_HISTORY_WINDOW * 5):
await layer.get("k")

system = self._make_system(layer)
assert system._calculate_adaptive_ttl("k") == 3600 * 4

async def test_indexing_and_len_work_over_bounded_history(self):
layer = InMemoryCacheLayer("idx", max_size=10, max_size_bytes=1024 * 1024)
await layer.set("k", "value")
for _ in range(5):
await layer.get("k")

accesses = layer.access_patterns["k"]
assert len(accesses) == 5
assert accesses[0] <= accesses[-1]
Loading