diff --git a/architecture/performance.md b/architecture/performance.md index 0103e72..118c130 100644 --- a/architecture/performance.md +++ b/architecture/performance.md @@ -53,19 +53,25 @@ deliberate — there is no interpreted fallback to inherit shared behaviour from ## Inlined memo hits -Two lookups are hand-inlined at their call site, with the method called only on -a miss: +Four lookups are hand-inlined across three call sites, with the method called +only on a miss: | Call site | Inlines | Method still owns | |---|---|---| | `Container.resolve_provider` | `providers_registry._resolvers.get(pid)` | the cycle guard and memo write, on a miss | | `_compile_cached_factory`'s `resolve` | `cache_registry._items.get(pid)` | `setdefault`, which is what makes concurrent first-resolvers share one `CacheItem` | +| `_compile_alias`'s `resolve` | `providers_registry._providers.get(source_type)` and `._resolvers.get(source.provider_id)` | `_find_source`'s error, and `resolver_for`'s cycle guard and memo write, on a miss | -In both cases the method being inlined *opens with exactly that lookup and +In each case the method being inlined *opens with exactly that lookup and returns*, so the inline is not a reimplementation that can drift — it is the -method's own fast path, hoisted past its frame. Both keep calling the real method -on a miss, so the miss-path invariants (cycle detection, single shared `CacheItem`) -are untouched. +method's own fast path, hoisted past its frame. All three keep calling the real +method on a miss, so the miss-path invariants (cycle detection, single shared +`CacheItem`, the dangling-source error) are untouched. + +The alias case inlines two lookups rather than one, because the hop is two indirections deep: without them an +alias costs four Python frames (`_find_source`, `find_provider`, `resolve_provider`, then the source's +resolver) where every `Factory` dependency costs one. +`tests/test_resolver_compiler.py::test_alias_hop_costs_exactly_one_resolver_frame` holds it at one. The warm cached resolve also returns before `CacheItem.get_or_create`, having already made the same `is UNSET` sentinel check that method opens with. diff --git a/architecture/providers.md b/architecture/providers.md index 6007639..57301c0 100644 --- a/architecture/providers.md +++ b/architecture/providers.md @@ -165,10 +165,12 @@ as nullable would silently swallow the unset-context signal — so it keeps dire providers.Alias(ConcreteDatabase, bound_type=DatabaseProtocol) ``` -The compiled `Alias` resolver forwards to `container.resolve_provider(source_provider)` after its own override -guard — it holds no cache of its own — wrapping a scope/resolution error with the alias's own step; `Alias` also -accepts an optional `bound_type` override. See [docs/providers/alias.md](../docs/providers/alias.md) for the -user-facing rationale and caching implications. +The compiled `Alias` resolver calls its source's compiled resolver directly, after its own override guard — it +holds no cache of its own — wrapping a scope/resolution error with the alias's own step. The source lookup and +the source's resolver-memo read are inlined into the closure (see +[performance.md](performance.md#inlined-memo-hits)); nothing is cached there, so a source registered after the +alias first resolves is picked up on the next one. `Alias` also accepts an optional `bound_type` override. See +[docs/providers/alias.md](../docs/providers/alias.md) for the user-facing rationale and caching implications. `Alias` overrides the `redirect_target(container)` node hook to return its source provider (`None` when the source type is unregistered), marking the alias as a transparent redirect. `DependencyGraph.terminal_scope` diff --git a/docs/providers/alias.md b/docs/providers/alias.md index 78c095d..9498ed5 100644 --- a/docs/providers/alias.md +++ b/docs/providers/alias.md @@ -2,7 +2,7 @@ `Alias` lets one type resolve to whatever provider already handles a different type. The most common use is binding an abstract base or `Protocol` to a concrete implementation that is already registered, without registering the implementation twice. -Resolving the alias delegates straight back through the container, so overrides and caching on the source provider apply transparently. +Resolving the alias calls the source's resolver directly, so overrides and caching on the source provider apply transparently. ## Parameters diff --git a/modern_di/resolver_compiler.py b/modern_di/resolver_compiler.py index 6b17506..a864ea9 100644 --- a/modern_di/resolver_compiler.py +++ b/modern_di/resolver_compiler.py @@ -276,12 +276,14 @@ def resolve(container: "Container") -> typing.Any: def _compile_alias(a: "Alias[typing.Any]") -> "typing.Callable[[Container], typing.Any]": - """Forward to the alias's source resolver, wrapping scope/resolution errors with its own step. + """Call the source's compiled resolver directly, wrapping scope/resolution errors with its own step. - A single try/except covers both the dangling-source lookup and the forwarded resolve, so a - missing source and a source's own scope error each carry this alias's resolution step. + The source lookup and its resolver memo read are inlined, and nothing is cached: a source + registered later is picked up on the next resolve. A single try/except covers the + dangling-source lookup and the forwarded resolve, so both carry this alias's resolution step. """ pid = a.provider_id + source_type = a._source_type resolution_step = a._resolution_step find_source = a._find_source @@ -292,7 +294,14 @@ def resolve(container: "Container") -> typing.Any: if override is not types.UNSET: return override try: - return container.resolve_provider(find_source(container)) + registry = container.providers_registry + source = registry._providers.get(source_type) + if source is None: + source = find_source(container) # raises AliasSourceNotRegisteredError + source_resolver = registry._resolvers.get(source.provider_id) + if source_resolver is None: + source_resolver = registry.resolver_for(source) + return source_resolver(container) except _STEP_ERRORS as exc: exc.prepend_step(resolution_step()) raise diff --git a/planning/decisions/2026-08-03-alias-binds-nothing.md b/planning/decisions/2026-08-03-alias-binds-nothing.md new file mode 100644 index 0000000..6e66ff2 --- /dev/null +++ b/planning/decisions/2026-08-03-alias-binds-nothing.md @@ -0,0 +1,79 @@ +--- +summary: The alias hop inlines its source lookup but caches nothing and captures nothing — binding the source resolver into the closure measured faster still, but buys a new invalidation invariant, and capturing the registry made it the one resolver forming a cycle with the memo that holds it. +--- + +# The alias hop inlines, but binds nothing + +**Decision:** `_compile_alias`'s closure reads `container.providers_registry` per +resolve and inlines both the source lookup and the source's resolver-memo read. +It holds no reference to the source, its resolver, or the registry. + +## Context + +`Alias` was the one compiled closure that did not reach its dependency's resolver +by direct reference. It called `Alias._find_source` → `find_provider`, then +`Container.resolve_provider` — four Python frames per hop where a `Factory` +dependency costs one. Three shapes were on the table: + +- **Eager bind.** Resolve the source at compile time and close over its resolver. + Measured alias resolve 305.5 → 192.0 ns (**-36%**), reproduced by two + independent verifiers. +- **Lazy bind.** Bind on the first non-overridden resolve, behind a + `bound is None` branch. Same steady-state cost as eager, without eager's + compile-time reach. +- **Inline only.** Read `_providers` and `_resolvers` per resolve, call the + source's resolver directly, cache nothing. Two dict `get`s per hop where a bind + has none. + +## Decision & rationale + +Inline-only ships, at **~322 → ~252 ns (-22%)** and 4 frames → 1. It gives up +roughly a third of the available win. + +**A bind buys an invalidation invariant; the inline buys none.** Both bind +variants are only sound because `ProvidersRegistry._invalidate()` clears +`_resolvers`, so a stale binding dies with the closure holding it. That is true +today, and it is a *second* place the invariant has to hold — stated, defended, +and re-checked by anyone who later touches memo publication. The inline re-reads +the live registry and cannot go stale by construction. The design principle here +is the conservative one: a bounded win does not buy a permanent cross-cutting +invariant, the same reasoning that dropped the +[warm-singleton memo swap](2026-07-18-warm-singleton-memo-swap-dropped.md). + +**Eager bind additionally escapes the override front-guard.** It compiles the +alias's whole source subtree even when the alias is overridden and the source is +never touched — the `modern-di-pytest` mock pattern. Shown structurally: +`len(_resolvers)` after resolving an overridden alias goes from 1 to 1+depth (11 +at depth 10), cold cost +404%. It also raises `TypeError` eagerly for a source +whose provider type `compile_resolver` does not know, and drops the maximum pure +alias chain from 494 to 329 hops. Lazy bind avoids all of this; only the +invariant argument above rules it out. + +**Capturing the registry was declined on the same grounds one level down.** The +first shipped form took the registry as a compile-time parameter, saving one +attribute load per hop. Since the registry memoizes the closure in `_resolvers`, +that made `_compile_alias`'s `resolve` the only compiled resolver forming +`registry → _resolvers → closure → cell → registry` — a registry with an alias in +it could then be freed only by cyclic GC, never by refcounting. Not a leak, but +the repo already took the opposite position for containers +(`Container.__init__`'s `scope: self` note, and `64b7cec`), and registries are +per-root-container, not per-process — a suite building a container per test +builds one per test. Reading the registry off the `container` argument removed +the cycle and measured **free** (250 → 249 ns, inside noise), which also puts the +alias back in the shape every other closure in the module already uses. + +Pinned by `test_alias_hop_costs_exactly_one_resolver_frame`, +`test_no_compiled_resolver_closes_over_its_registry`, +`test_overridden_alias_compiles_nothing_of_its_source`, and +`test_alias_picks_up_a_source_registered_after_a_failed_resolve` — that last one +catches only a *negative* cache; a success-path cache is undetectable by +construction, because a registered type's provider can never be replaced and any +registration clears `_resolvers`. + +## Revisit trigger + +An alias hop shows up hot in a profile from a real integration, **and** the +`_invalidate()`-clears-`_resolvers` invariant has acquired an explicit owner and +test of its own — at which point lazy bind (never eager) is worth the remaining +~60 ns. A second compiled closure needing the registry at resolve time would +reopen the capture question separately. diff --git a/planning/deferred/2026-08-01-alias-source-binding.md b/planning/deferred/2026-08-01-alias-source-binding.md deleted file mode 100644 index 6d46eb5..0000000 --- a/planning/deferred/2026-08-01-alias-source-binding.md +++ /dev/null @@ -1,51 +0,0 @@ ---- -summary: Binding an Alias to its source's compiled resolver at compile time measures -36% on every alias resolve (~100 ns and 3 frames per hop); the memo-publication blocker is cleared, leaving eager compilation escaping the override front-guard as the sole prerequisite. ---- - -# Bind an `Alias` to its source's compiled resolver - -`_compile_alias` is the only compiled closure that does not use the -`registry.resolver_for(p)` direct-reference pattern every `Factory` closure uses. -It calls `a._find_source(container)` per resolve and re-enters -`Container.resolve_provider`, paying three Python frames and two dict lookups on -every alias hop. Resolving the source once at compile time and closing over its -resolver would reduce the body to `source_resolver(container)` inside the -existing try/except that prepends the alias's resolution step. - -## Why it is open - -The prize is real and was reproduced by two independent verifiers: alias resolve -305.5 → 192.0 ns (**-36%**), roughly **-100 ns and -3 frames per alias hop**. A -control that overrides the alias itself stayed flat, as it must. - -Two blockers, and **neither is alias-specific** — which is why this is deferred -rather than rejected. - -**Eager compilation escapes the override front-guard.** Binding at compile time -makes alias compilation recursive and eager: it compiles the alias's entire -source subtree *even when the alias is overridden and the source is never -touched* — which is exactly the `modern-di-pytest` mock pattern. Shown -structurally rather than by timing: `len(providers_registry._resolvers)` after -resolving an overridden alias goes from 1 to 1+depth (11 at depth 10), and cold -cost from 6652 to 33539 ns (**+404%**). It also raises `TypeError` eagerly for a -source whose provider type `compile_resolver` does not know, and drops the -maximum pure-alias chain from 494 to 329 hops (`RecursionError` at compile time). -Any future attempt must bind **lazily on first miss**, or bind only after the -override front-guard has been cleared. - -**~~The resolver-memo publication race.~~ Cleared.** The licensing invariant — -that a source registered after the alias compiles is picked up via -`_invalidate()` — was false, because `resolver_for` and `plan_for` published -their memos without checking whether a mutation had landed while they built. -Both are now generation-checked; see -[`architecture/concurrency.md`](../../architecture/concurrency.md). This blocker -no longer applies, and the first one below is the only remaining prerequisite. - -A working prototype is preserved at `prototype-alias-bind.diff` in this session's -workflow transcript directory. - -## Revisit trigger - -Alias source binding can be made lazy enough not to compile a subtree behind an -active override. That is now the sole remaining prerequisite — resolver-memo -publication became generation-checked, closing the other one. diff --git a/tests/providers/test_alias.py b/tests/providers/test_alias.py index 25a8500..6abd7dd 100644 --- a/tests/providers/test_alias.py +++ b/tests/providers/test_alias.py @@ -479,3 +479,59 @@ class G(Group): container = Container(scope=_BelowApp.ROOT, groups=[G]) container.validate() assert container.providers_registry.is_validated() is True + + +def test_alias_resolves_from_a_closed_container_with_warning() -> None: + # The alias hop itself carries no closed-container check; the entry `resolve_provider` + # reopens, exactly as it does for a context provider or `container_provider`. + container = Container(groups=[MyGroup]) + container.open() + container.resolve(AbstractRepository) + container.close_sync() + + with pytest.warns(exceptions.ContainerClosedWarning): + assert isinstance(container.resolve(AbstractRepository), PostgresRepository) + + +def test_alias_picks_up_a_source_registered_after_a_failed_resolve() -> None: + # The compiled alias caches nothing, so a source registered after a failed resolve is picked up next. + # Catches only a negative cache (remembering the miss) -- a positive one would pass too, since a + # registered provider can't be replaced (DuplicateProviderTypeError) and registering anything clears `_resolvers`. + class Late: ... + + class LateIface: ... + + class G(Group): + iface = providers.Alias(source_type=Late, bound_type=LateIface) + + container = Container(groups=[G]) + container.open() + + with pytest.raises(AliasSourceNotRegisteredError): + container.resolve(LateIface) + + container.add_providers(providers.Factory(creator=Late)) + + assert isinstance(container.resolve(LateIface), Late) + + +def test_mutual_alias_cycle_raises_circular_dependency_at_runtime() -> None: + # A pure-alias cycle has no factory node to trip the static guard, so it recurses at + # runtime until `resolve_provider` converts the RecursionError. The chain is rooted at + # the provider the caller asked for. + class First: ... + + class Second: ... + + class G(Group): + first = providers.Alias(source_type=Second, bound_type=First) + second = providers.Alias(source_type=First, bound_type=Second) + + container = Container(groups=[G]) + container.open() + + # Asserted via `match=` rather than after the block: a RecursionError tears down the + # trace function, so below 3.12 -- where coverage traces instead of using + # sys.monitoring -- any line following it here runs unrecorded and fails the gate. + with pytest.raises(CircularDependencyError, match=r"(?s)First.*Second"): + container.resolve(First) diff --git a/tests/test_resolver_compiler.py b/tests/test_resolver_compiler.py index 0c68a3e..ac39db7 100644 --- a/tests/test_resolver_compiler.py +++ b/tests/test_resolver_compiler.py @@ -308,6 +308,80 @@ def test_resolve_costs_exactly_one_resolver_frame_per_node() -> None: ) +def test_alias_hop_costs_exactly_one_resolver_frame() -> None: + # An alias forwards to its source's compiled resolver by direct reference, like every + # Factory dependency. Routing through `_find_source` + `find_provider` + + # `resolve_provider` instead costs four frames per hop -- see architecture/performance.md. + class _Source: ... + + class _Iface: ... + + class _Direct(Group): + source = providers.Factory(creator=_Source, scope=Scope.APP) + + class _Aliased(Group): + source = providers.Factory(creator=_Source, scope=Scope.APP) + iface = providers.Alias(source_type=_Source, bound_type=_Iface) + + direct = Container(scope=Scope.APP, groups=[_Direct]) + aliased = Container(scope=Scope.APP, groups=[_Aliased]) + direct.resolve_provider(_Direct.source) # compile before measuring + aliased.resolve_provider(_Aliased.iface) + + without_alias = _count_python_calls(lambda: direct.resolve_provider(_Direct.source)) + with_alias = _count_python_calls(lambda: aliased.resolve_provider(_Aliased.iface)) + + assert (with_alias - without_alias) == 1, ( + f"an alias hop costs {with_alias - without_alias} Python calls, expected 1 (its own " + f"resolver). Looking the source up per resolve costs four -- see architecture/performance.md." + ) + + +def test_overridden_alias_compiles_nothing_of_its_source() -> None: + # The override front-guard runs before the source is ever looked up, so the mock pattern + # never pays to compile a subtree it will not touch. + class _Source: ... + + class _Iface: ... + + class _G(Group): + source = providers.Factory(creator=_Source, scope=Scope.APP) + iface = providers.Alias(source_type=_Source, bound_type=_Iface) + + container = Container(scope=Scope.APP, groups=[_G]) + sentinel = object() + container.override(_G.iface, sentinel) + + assert container.resolve(_Iface) is sentinel + assert list(container.providers_registry._resolvers) == [_G.iface.provider_id] + + +def test_no_compiled_resolver_closes_over_its_registry() -> None: + # A resolver that captures its registry forms a cycle with the memo holding it, so the + # registry is reclaimable only by cyclic GC. Every closure reads its registries off the + # container argument instead. + class _Src: ... + + class _Iface: ... + + class _G(Group): + source = providers.Factory(creator=_Src, scope=Scope.APP) + iface = providers.Alias(source_type=_Src, bound_type=_Iface) + + container = Container(scope=Scope.APP, groups=[_G]) + container.resolve(_Iface) + registry = container.providers_registry + + capturing = [ + fn.__qualname__ + for fn in typing.cast("tuple[_pytypes.FunctionType, ...]", tuple(registry._resolvers.values())) + for cell in (fn.__closure__ or ()) + if cell.cell_contents is registry + ] + + assert capturing == [] + + def test_cached_resolver_has_no_cell_on_the_warm_path() -> None: # The cold-miss thunk must not close over `target`: a closure promotes it to a cell, so # MAKE_CELL runs in the resolver's prologue on every call -- including the warm hit that