From fe1cbcdae1aad781022f94b32436a0aa5e5325f6 Mon Sep 17 00:00:00 2001 From: magi Date: Sun, 6 Sep 2026 19:56:03 +0530 Subject: [PATCH] fix(cache): route_cache and depends.py's per-handler caches now bounded Router.route_cache had a CachedRoute.is_valid()/ttl meant to gate cache freshness, but nothing ever called it, and nothing ever pruned the cache by size either -- a dynamic route like /users/{id} got one permanent CachedRoute per distinct id ever requested. depends.py's _sig_cache, _type_hints_cache, and _plan_cache (keyed by id(func)) had the same shape of problem: cleanup_caches() existed to trim them but was deleted as dead code in #9 since nothing called it either. Both now prune on write, mirroring Velocix._prune_response_cache's sweep-then-cap: route_cache sweeps expired entries first (finally giving is_valid()/ttl a job), then drops oldest by created_at if still over _ROUTE_CACHE_MAX_SIZE (1024, per method). The depends.py caches have no expiry concept (identity-keyed, not time-keyed), so they just drop the oldest entries via dict's insertion order once over _CACHE_MAX_SIZE (1000). Added tests/test_cache_eviction.py: resolves/handlers well past each cap and asserts the caches stay bounded. Fixes #8. --- tests/test_cache_eviction.py | 60 ++++++++++++++++++++++++++++++++++++ velocix/core/depends.py | 20 ++++++++++++ velocix/core/router.py | 27 ++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 tests/test_cache_eviction.py diff --git a/tests/test_cache_eviction.py b/tests/test_cache_eviction.py new file mode 100644 index 0000000..0916a7f --- /dev/null +++ b/tests/test_cache_eviction.py @@ -0,0 +1,60 @@ +"""Regression tests for issue #8: route_cache and the depends.py per-handler +caches used to grow without bound (a CachedRoute per distinct concrete +dynamic path, or a cache entry per distinct handler function, forever). +Both now prune themselves on write, mirroring Velocix._prune_response_cache. +""" + +from velocix import Router +from velocix.core.depends import ( + _CACHE_MAX_SIZE, + _plan_cache, + _sig_cache, + _type_hints_cache, + get_plan_and_needs_request, +) + + +def test_router_dynamic_cache_stays_bounded(): + router = Router() + + def handler(user_id: int): + return {} + + router.add_route("GET", "/users/{user_id}", handler) + + # Every distinct id is its own concrete path -> its own CachedRoute. + for user_id in range(router._ROUTE_CACHE_MAX_SIZE + 500): + router.resolve("GET", f"/users/{user_id}") + + assert len(router.route_cache["GET"]) <= router._ROUTE_CACHE_MAX_SIZE + + +def test_router_static_cache_unaffected_by_pruning(): + """Static routes are registration-bounded already; pruning shouldn't + make a normal, small app lose its cached static routes.""" + router = Router() + + def handler(): + return {} + + router.add_route("GET", "/health", handler) + router.resolve("GET", "/health") + router.resolve("GET", "/health") + assert "/health" in router.route_cache["GET"] + + +def test_depends_caches_stay_bounded(): + # Distinct closures -> distinct id()s -> distinct cache entries. + handlers = [] + for i in range(_CACHE_MAX_SIZE + 500): + def handler(x: int = i): + return x + + handlers.append(handler) + + for handler in handlers: + get_plan_and_needs_request(handler) + + assert len(_sig_cache) <= _CACHE_MAX_SIZE + assert len(_type_hints_cache) <= _CACHE_MAX_SIZE + assert len(_plan_cache) <= _CACHE_MAX_SIZE diff --git a/velocix/core/depends.py b/velocix/core/depends.py index 104e5f6..62bdc11 100644 --- a/velocix/core/depends.py +++ b/velocix/core/depends.py @@ -84,6 +84,23 @@ def _extract_marker(annotation: Any) -> tuple[Any, Any] | None: ] _plan_cache: dict[int, tuple[Callable[..., Any], PlanEntry]] = {} +# These are keyed by id(func), so their natural upper bound is "every distinct +# handler/dependency function the app ever passes through here" -- fixed for a +# typical app, but unbounded for one that builds fresh closures per request. +# Trim on write, same sweep-then-cap shape as Velocix._prune_response_cache: +# no expiry concept here (identity-keyed, not time-keyed), so instead of an +# expired-first sweep this just drops the oldest entries once over the cap, +# relying on dict's guaranteed insertion order. +_CACHE_MAX_SIZE = 1000 + + +def _trim_cache(cache: dict[int, Any]) -> None: + if len(cache) <= _CACHE_MAX_SIZE: + return + excess = len(cache) - _CACHE_MAX_SIZE + for key in list(cache.keys())[:excess]: + del cache[key] + class Depends: """ @@ -125,6 +142,7 @@ def _get_signature(func: Callable[..., Any]) -> inspect.Signature: entry = _sig_cache.get(func_id) if entry is None or entry[0] is not func: _sig_cache[func_id] = (func, inspect.signature(func)) + _trim_cache(_sig_cache) entry = _sig_cache[func_id] return entry[1] @@ -143,6 +161,7 @@ def _get_type_hints_cached(func: Callable[..., Any]) -> dict[str, Any]: except Exception: hints = {} _type_hints_cache[func_id] = (func, hints) + _trim_cache(_type_hints_cache) entry = _type_hints_cache[func_id] return entry[1] @@ -323,6 +342,7 @@ def _build_resolution_plan(handler: Callable[..., Any]) -> tuple[tuple[str, str, (plan_tuple, needs_request, cache_ttl, call_mode, status_code, response_model, response_class), ) _plan_cache[func_id] = entry + _trim_cache(_plan_cache) return entry[1][0] diff --git a/velocix/core/router.py b/velocix/core/router.py index 7e370b4..c0ec736 100644 --- a/velocix/core/router.py +++ b/velocix/core/router.py @@ -50,6 +50,13 @@ class RouteNode: class Router: """Ultra-high performance router with advanced caching and optimization""" + # Per-method cap on route_cache. Static routes are bounded by however many + # routes the app registers, but a dynamic route like /users/{id} gets one + # CachedRoute per distinct concrete path ever resolved -- unbounded for a + # high-cardinality path param. Pruned on every dynamic-route cache write, + # same sweep-then-cap shape as Velocix._prune_response_cache. + _ROUTE_CACHE_MAX_SIZE: int = 1024 + def __init__(self, *, metrics_enabled: bool = False): # Per-route metrics (hit counts, avg response time) are opt-in: the # counter mutation on every dynamic-route cache hit and the clock @@ -296,6 +303,25 @@ def url_path_for(self, name: str, /, **path_params: Any) -> str: raise NoMatchFound(name, path_params) return quote(template.format(**path_params)) + def _prune_route_cache(self, method: str) -> None: + """Evict expired entries first, then oldest entries if still over size. + + Pattern from Velocix._prune_response_cache: sweep expired (via + CachedRoute.is_valid()/ttl), then drop oldest by created_at until + under the cap. + """ + cache = self.route_cache[method] + if len(cache) <= self._ROUTE_CACHE_MAX_SIZE: + return + expired = [k for k, v in cache.items() if not v.is_valid()] + for k in expired: + del cache[k] + if len(cache) <= self._ROUTE_CACHE_MAX_SIZE: + return + oldest = sorted(cache, key=lambda k: cache[k].created_at) + for k in oldest[: len(cache) - self._ROUTE_CACHE_MAX_SIZE]: + del cache[k] + def resolve(self, method: str, path: str) -> tuple[Callable, dict[str, str]]: """Ultra-fast route resolution with caching""" # Check cache first: no key allocation, no clock reads on the hot path. @@ -369,6 +395,7 @@ def resolve(self, method: str, path: str) -> tuple[Callable, dict[str, str]]: version=self._routes_version, metrics=RouteMetrics(hit_count=1) if self.metrics_enabled else None, ) + self._prune_route_cache(method) return handler, params