From 0f2612b6423836e41c8810bbfbf459fa3b727bcb Mon Sep 17 00:00:00 2001 From: Hayden <154503486+groupthinking@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:57:11 -0500 Subject: [PATCH] perf: bound L1 cache access-history retention InMemoryCacheLayer.access_patterns appended one float per cache hit and never trimmed, and _evict_if_needed() removed the cache entry without releasing the matching history. delete() already released it, so eviction was the sole path that orphaned a history with no resident key left to ever trigger its cleanup. The retained bytes are invisible to stats.total_size_bytes, so the max_size_bytes LRU budget could neither see nor reclaim them. Under a 200-entry cache serving 200k reads across 5k churn keys the layer reports 2,200 bytes while actually holding 7.1 MiB of timestamps. Bound each key's history to ACCESS_HISTORY_WINDOW=64 via a deque and pop it on eviction. _calculate_adaptive_ttl() reads only accesses[0], accesses[-1] and len(accesses), all O(1) on a deque, so it needs no change. Retained history drops 7,461,354 -> 587,664 bytes with hit rate, total_size_bytes, resident entry count and wall time all unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../backend/services/intelligent_cache.py | 23 ++- tests/unit/test_intelligent_cache.py | 164 ++++++++++++++++++ 2 files changed, 185 insertions(+), 2 deletions(-) diff --git a/src/youtube_extension/backend/services/intelligent_cache.py b/src/youtube_extension/backend/services/intelligent_cache.py index a92cbe171..0e9b9f867 100644 --- a/src/youtube_extension/backend/services/intelligent_cache.py +++ b/src/youtube_extension/backend/services/intelligent_cache.py @@ -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 @@ -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): @@ -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(). + 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""" @@ -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 diff --git a/tests/unit/test_intelligent_cache.py b/tests/unit/test_intelligent_cache.py index b59b3be50..c4dda0a7c 100644 --- a/tests/unit/test_intelligent_cache.py +++ b/tests/unit/test_intelligent_cache.py @@ -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]