perf(layout): don't pin id-keyed source images in fit_image's cache

src/adaptive_layout.py

@@ -28,6 +28,7 @@
 """
 
 import logging
+import weakref
 from collections import OrderedDict
 from dataclasses import dataclass
 from typing import Any, Dict, List, Optional, Sequence, Tuple, Union
@@ -332,9 +333,10 @@ def __init__(self, width: int, height: int, font_manager: Any,
         # a plugin fitting changing text (a live game clock, a ticker) on a
         # 24/7 service would otherwise grow this without bound.
         self._fit_cache: "OrderedDict[Any, FitResult]" = OrderedDict()
-        # LRU-bounded (images are big). Entries hold a strong reference to
-        # the source image when keyed by id() so the id can't be recycled
-        # out from under the cache.
+        # LRU-bounded (images are big). An id()-keyed entry watches its
+        # source image through a weak reference and is dropped when the
+        # source is freed (see fit_image), so the id can't be recycled out
+        # from under the cache and the cache never keeps the source alive.
         self._image_cache: "OrderedDict[Any, Tuple[Any, Any]]" = OrderedDict()
 
     _IMAGE_CACHE_MAX = 64
@@ -536,8 +538,15 @@ def fit_image(self, img: Any, box: Union[Region, Tuple[int, int]], *,
         cached per (image, box size, options) for this panel size.
 
         Prefer a stable ``cache_key`` (e.g. "logo:KC") for images that get
-        reloaded — the default id()-based key is safe (the entry pins the
-        source image) but misses across reloads of the same content.
+        reloaded — the default id()-based key misses across reloads of the
+        same content.
+
+        An id()-keyed entry lives only as long as its source image: it holds
+        a weak reference and is dropped when the source is freed. It used to
+        pin the source instead, so a plugin passing a freshly loaded image
+        each frame (``draw_image(Image.open(path), box)``, the documented
+        one-liner) never hit and kept the last 64 sources alive — ~64MB for
+        500x500 RGBA team logos, the median size under assets/sports.
         """
         from src.adaptive_images import fit_image as _fit_image
 
@@ -547,18 +556,31 @@ def fit_image(self, img: Any, box: Union[Region, Tuple[int, int]], *,
         key = ("image", identity, img.size, box_w, box_h, mode,
                crop_to_ink, anchor, resample_name, upscale)
 
-        cached = self._image_cache.get(key)
-        if cached is not None:
-            self._image_cache.move_to_end(key)
+        cache = self._image_cache
+        cached = cache.get(key)
+        # An id()-keyed hit must still be this very image; the callback below
+        # normally removes a dead source's entry before its id can recur.
+        if cached is not None and (cache_key is not None or cached[1]() is img):
+            cache.move_to_end(key)
             return cached[0]
 
         result = _fit_image(img, (box_w, box_h), mode=mode,
                             crop_to_ink=crop_to_ink, anchor=anchor,
                             resample=resample, upscale=upscale)
-        # Pin the source only for id()-keyed entries (see docstring).
-        self._image_cache[key] = (result, img if cache_key is None else None)
-        while len(self._image_cache) > self._IMAGE_CACHE_MAX:
-            self._image_cache.popitem(last=False)
+        source = None
+        if cache_key is None:
+            def _forget(ref: Any, key: Any = key) -> None:
+                entry = cache.get(key)
+                if entry is not None and entry[1] is ref:
+                    cache.pop(key, None)
+            try:
+                source = weakref.ref(img, _forget)
+            except TypeError:
+                # Not weak-referenceable: pin it, as before.
+                source = lambda img=img: img  # noqa: E731
+        cache[key] = (result, source)
+        while len(cache) > self._IMAGE_CACHE_MAX:
+            cache.popitem(last=False)
         return result
 
     # ---- text utilities ------------------------------------------------

test/test_adaptive_images.py

@@ -145,13 +145,49 @@ def test_id_keyed_default(self, ctx):
         a = ctx.fit_image(img, (20, 20))
         assert ctx.fit_image(img, (20, 20)) is a
 
-    def test_id_safety_pins_source(self, ctx):
-        # id()-keyed entries must pin the source image so a recycled id
-        # can't alias a dead image's cache entry.
+    def test_id_keyed_entry_does_not_pin_source(self, ctx):
+        import gc
+        import weakref
+
         img = _solid(10, 10)
         ctx.fit_image(img, (20, 20))
-        pinned = [entry[1] for entry in ctx._image_cache.values()]
-        assert img in pinned
+        assert len(ctx._image_cache) == 1
+        watch = weakref.ref(img)
+        del img
+        gc.collect()
+        assert watch() is None  # the cache did not keep it alive
+        assert len(ctx._image_cache) == 0  # and its entry went with it
+
+    def test_fresh_image_each_frame_holds_nothing(self, ctx):
+        # draw_image(Image.open(path), box) every frame: the old pinning
+        # filled all 64 slots with dead-weight sources.
+        for _ in range(ctx._IMAGE_CACHE_MAX * 2):
+            ctx.fit_image(_solid(50, 50), (20, 20))
+        assert len(ctx._image_cache) == 0
+
+    def test_recycled_id_does_not_alias(self, ctx):
+        # A same-size image at a recycled address must not get the dead
+        # image's fit, even if the entry somehow outlived its source.
+        red = _solid(10, 10, (255, 0, 0, 255))
+        first = ctx.fit_image(red, (20, 20))
+        key = next(iter(ctx._image_cache))
+        entry = ctx._image_cache[key]
+        ctx._image_cache[key] = (entry[0], lambda: None)  # source "gone"
+        again = ctx.fit_image(red, (20, 20))
+        assert again is not first  # refit, not a stale hit
+        assert ctx.fit_image(red, (20, 20)) is again
+
+    def test_unweakrefable_source_is_pinned(self, ctx, monkeypatch):
+        import src.adaptive_layout as layout_mod
+
+        def no_weakref(*_args, **_kwargs):
+            raise TypeError("cannot create weak reference")
+
+        monkeypatch.setattr(layout_mod.weakref, "ref", no_weakref)
+        img = _solid(10, 10)
+        a = ctx.fit_image(img, (20, 20))
+        assert ctx.fit_image(img, (20, 20)) is a
+        assert next(iter(ctx._image_cache.values()))[1]() is img
 
     def test_cache_key_entries_do_not_pin(self, ctx):
         img = _solid(10, 10)