perf: create cache items inline, inline the sync close loop, merge build and create (#617)

modern_di/cache.py

@@ -4,11 +4,10 @@
 import typing
 
 from modern_di import exceptions, types
-from modern_di.providers import CacheSettings, Factory
+from modern_di.providers import CacheSettings
 
 
 _T = typing.TypeVar("_T")
-_R = typing.TypeVar("_R")
 _V = typing.TypeVar("_V")
 
 
@@ -24,13 +23,8 @@ def clear(self) -> None:
             self.cache = types.UNSET
             self.finalized = False
 
-    def get_or_create(
-        self,
-        build: typing.Callable[[_T], _R],
-        target: _T,
-        create: typing.Callable[[_R], _V],
-    ) -> tuple[_V, bool]:
-        """Return the memoized singleton, or ``create(build(target))`` it once under this item's lock.
+    def get_or_create(self, make: typing.Callable[[_T], _V], target: _T) -> tuple[_V, bool]:
+        """Return the memoized singleton, or ``make(target)`` it once under this item's lock.
 
         A hit never takes the lock. A miss builds and creates under it, so concurrent misses
         build the value and its dependencies once. `created` is True only for the caller that built.
@@ -40,42 +34,10 @@ def get_or_create(
         with self.lock:
             if self.cache is not types.UNSET:
                 return self.cache, False
-            value = create(build(target))
+            value = make(target)
             self.cache = value
             return value, True
 
-    def _pending_finalizer(self) -> typing.Callable[[typing.Any], object] | None:
-        """Return the finalizer still owed to the cached value, or None when nothing is owed."""
-        return None if self.cache is types.UNSET or self.finalized else self.settings.finalizer
-
-    def close_sync(self) -> None:
-        if (finalizer := self._pending_finalizer()) is not None:
-            if self.settings._is_async_finalizer:  # noqa: SLF001
-                raise exceptions.AsyncFinalizerInSyncCloseError(instance_type=type(self.cache))
-            try:
-                result = finalizer(self.cache)
-            except Exception:
-                self.clear()
-                raise
-            if result is not None and inspect.isawaitable(result):
-                if inspect.iscoroutine(result):
-                    result.close()  # suppress "never awaited" warning
-                raise exceptions.AsyncFinalizerInSyncCloseError(instance_type=type(self.cache))
-            self.finalized = True
-
-        self.clear()
-
-
-def fetch_cache_item(items: dict[int, CacheItem], provider: Factory[typing.Any]) -> CacheItem:
-    """Return the cache item in ``items`` for a cached ``provider``, creating it on first use."""
-    # Get before setdefault: a bare setdefault builds a throwaway CacheItem on every hit.
-    provider_id = provider._provider_id  # noqa: SLF001
-    item = items.get(provider_id)
-    if item is not None:
-        return item
-    settings = typing.cast("CacheSettings[typing.Any]", provider._cache_settings)  # noqa: SLF001
-    return items.setdefault(provider_id, CacheItem(settings=settings))
-
 
 def cached_count(items: dict[int, CacheItem]) -> int:
     """Return how many of ``items`` hold a value."""
@@ -107,13 +69,26 @@ def close_sync(creation_order: list[CacheItem]) -> None:
     finalizer_errors: list[Exception] = []
     remaining: list[CacheItem] = []
     for cache_item in reversed(creation_order):
-        try:
-            cache_item.close_sync()
-        except exceptions.AsyncFinalizerInSyncCloseError as e:
-            finalizer_errors.append(e)
-            remaining.append(cache_item)
-        except Exception as e:  # noqa: BLE001
-            finalizer_errors.append(e)
+        finalizer = cache_item.settings.finalizer
+        value = cache_item.cache
+        if finalizer is not None and value is not types.UNSET and not cache_item.finalized:
+            if cache_item.settings._is_async_finalizer:  # noqa: SLF001
+                finalizer_errors.append(exceptions.AsyncFinalizerInSyncCloseError(instance_type=type(value)))
+                remaining.append(cache_item)
+                continue
+            try:
+                result = finalizer(value)
+            except Exception as e:  # noqa: BLE001
+                finalizer_errors.append(e)
+            else:
+                if result is not None and inspect.isawaitable(result):
+                    if inspect.iscoroutine(result):
+                        result.close()  # suppress "never awaited" warning
+                    finalizer_errors.append(exceptions.AsyncFinalizerInSyncCloseError(instance_type=type(value)))
+                    remaining.append(cache_item)
+                    continue
+                cache_item.finalized = True
+        cache_item.clear()
     remaining.reverse()
     creation_order[:] = remaining
     if finalizer_errors:

modern_di/resolver_compiler.py

@@ -21,7 +21,7 @@
 import typing
 
 from modern_di import exceptions, types
-from modern_di.cache import fetch_cache_item
+from modern_di.cache import CacheItem
 from modern_di.dependency_graph import redirect_hops, terminal_chain
 from modern_di.providers.abstract import AbstractProvider
 from modern_di.providers.alias import Alias
@@ -101,21 +101,20 @@ def compile_resolver(provider: "AbstractProvider[typing.Any]", registry: "Provid
 _TRANSIENT = "def resolve(container):\n" + _NAVIGATE + _BUILD_ARGUMENTS + _CALL_CREATOR
 
 _CACHED = (
-    "def build(target):\n"
+    "def make(target):\n"
     + _BUILD_ARGUMENTS
-    + "    return {built}\n\ndef create(built):\n"
     + _CALL_CREATOR
     + "\ndef resolve(container):\n"
     + _NAVIGATE
     + """\
     cache_items = target._cache_items
     cache_item = cache_items.get(pid)
     if cache_item is None:
-        cache_item = fetch_cache_item(cache_items, provider)
+        cache_item = cache_items.setdefault(pid, CacheItem(settings=cache_settings))
     cached = cache_item.cache
     if cached is not UNSET:
         return cached
-    value, created = cache_item.get_or_create(build, target, create)
+    value, created = cache_item.get_or_create(make, target)
     if created:
         target._creation_order.append(cache_item)
     return value
@@ -129,23 +128,18 @@ def _source(arity: int, names: tuple[str, ...] | None, static: bool, cached: boo
         build = [f"        a{i} = r{i}(target)" for i in range(arity)] or ["        pass"]
         call_offset = 0
         args = ", ".join(f"a{i}" for i in range(arity))
-        built = "(" + "".join(f"a{i}, " for i in range(arity)) + ")"
-        star = "*"
     else:
         calls = [f"            {name!r}: r{i}(target)," for i, name in enumerate(names)]
         build = ["        kwargs = {", *calls, "        }"]
         call_offset = 1
         if static:
             build.append("        kwargs.update(static)")
-        args, built, star = "**kwargs", "kwargs", "**"
-    if cached:
-        template, args = _CACHED, f"{star}built"
-    else:
-        template = _TRANSIENT
+        args = "**kwargs"
+    template = _CACHED if cached else _TRANSIENT
     # `{build}` must be the first multi-line placeholder: the lines before it are counted as-is.
     build_line = template[: template.index("{build}")].count("\n") + 1
     arg_lines = {build_line + call_offset + i: i for i in range(arity)}
-    return template.format(build="\n".join(build), built=built, args=args), arg_lines
+    return template.format(build="\n".join(build), args=args), arg_lines
 
 
 _shape_ids = itertools.count()
@@ -178,7 +172,7 @@ def _navigate(
 
 _FACTORY_GLOBALS: dict[str, typing.Any] = {
     "UNSET": types.UNSET,
-    "fetch_cache_item": fetch_cache_item,
+    "CacheItem": CacheItem,
     "_navigate": _navigate,
     "ResolutionError": exceptions.ResolutionError,
     "CreatorCallError": exceptions.CreatorCallError,
@@ -201,6 +195,7 @@ def _compile_factory(f: "Factory[typing.Any]", registry: "ProvidersRegistry") ->
     )
     namespace = _FACTORY_GLOBALS.copy()
     namespace["provider"] = f
+    namespace["cache_settings"] = f.cache_settings
     namespace["pid"] = f.provider_id
     namespace["scope"] = f.scope
     namespace["creator"] = f._creator

tests/helpers.py

@@ -1,10 +1,11 @@
 import typing
 
 from modern_di import Container
-from modern_di.cache import CacheItem, fetch_cache_item
-from modern_di.providers import Factory
+from modern_di.cache import CacheItem
+from modern_di.providers import CacheSettings, Factory
 
 
 def cache_item(container: Container, provider: Factory[typing.Any]) -> CacheItem:
     """Return `container`'s cache item for the cached `provider`, creating it if needed."""
-    return fetch_cache_item(container._cache_items, provider)
+    settings = typing.cast("CacheSettings[typing.Any]", provider.cache_settings)
+    return container._cache_items.setdefault(provider.provider_id, CacheItem(settings=settings))

tests/test_cache.py

@@ -4,9 +4,11 @@
 
 import pytest
 
-from modern_di.cache import CacheItem, close_async, fetch_cache_item
+from modern_di import Container, Group, Scope
+from modern_di.cache import CacheItem, close_async
 from modern_di.providers import CacheSettings, Factory
 from modern_di.types import UNSET
+from tests.helpers import cache_item
 
 
 def _item() -> CacheItem:
@@ -42,7 +44,7 @@ def create(kwargs: dict[str, typing.Any]) -> tuple[str, dict[str, typing.Any]]:
         calls["create"] += 1
         return ("made", kwargs)
 
-    value, created = item.get_or_create(build=build, target=None, create=create)
+    value, created = item.get_or_create(lambda target: create(build(target)), None)
 
     assert created is True
     assert value == ("made", {"x": 1})
@@ -62,7 +64,7 @@ def create(_: object) -> str:
         msg = "create must not run on a cache hit"
         raise AssertionError(msg)
 
-    value, created = item.get_or_create(build=build, target=None, create=create)
+    value, created = item.get_or_create(lambda target: create(build(target)), None)
 
     assert created is False
     assert value == "cached"
@@ -93,7 +95,7 @@ def create(_: object) -> str:
         msg = "create must not run when another thread already stored the value"
         raise AssertionError(msg)
 
-    value, created = item.get_or_create(build=build, target=None, create=create)
+    value, created = item.get_or_create(lambda target: create(build(target)), None)
 
     assert created is False
     assert value == "won-the-race"
@@ -102,33 +104,42 @@ def create(_: object) -> str:
 def test_get_or_create_releases_the_item_lock() -> None:
     item = _item()
 
-    value, created = item.get_or_create(build=lambda _: 0, target=None, create=lambda _: "v")
+    value, created = item.get_or_create(lambda _: "v", None)
     assert (value, created) == ("v", True)
     assert _acquirable_from_another_thread(item)
 
 
 def test_each_cache_item_owns_its_lock() -> None:
-    cache_items: dict[int, CacheItem] = {}
-    first = fetch_cache_item(cache_items, Factory(creator=lambda: 1, bound_type=int, cache=True))
-    second = fetch_cache_item(cache_items, Factory(creator=lambda: "", bound_type=str, cache=True))
+    container = Container(scope=Scope.APP)
+    first = cache_item(container, Factory(creator=lambda: 1, bound_type=int, cache=True))
+    second = cache_item(container, Factory(creator=lambda: "", bound_type=str, cache=True))
     assert first.lock is not second.lock
 
 
 @pytest.mark.thread_race
-def test_concurrent_fetches_of_one_provider_share_one_item() -> None:
+def test_concurrent_first_resolves_of_one_provider_build_once() -> None:
     n = 8
-    cache_items: dict[int, CacheItem] = {}
-    provider = Factory(creator=lambda: 1, bound_type=int, cache=True)
+    built: list[object] = []
+
+    def make() -> object:
+        built.append(value := object())
+        return value
+
+    class G(Group):
+        cached = Factory(creator=make, bound_type=object, cache=True)
+
+    container = Container(scope=Scope.APP, groups=[G])
     barrier = threading.Barrier(n, timeout=5)
 
-    def fetch() -> CacheItem:
+    def resolve() -> object:
         barrier.wait()
-        return fetch_cache_item(cache_items, provider)
+        return container.resolve(object)
 
     with ThreadPoolExecutor(max_workers=n) as pool:
-        items = [f.result(timeout=5) for f in [pool.submit(fetch) for _ in range(n)]]
+        values = [f.result(timeout=5) for f in [pool.submit(resolve) for _ in range(n)]]
 
-    assert all(item is items[0] for item in items)
+    assert len(built) == 1
+    assert all(value is built[0] for value in values)
 
 
 async def test_close_async_runs_only_owed_finalizers_and_empties_the_order() -> None: