diff --git a/architecture/performance.md b/architecture/performance.md index 118c130..1908d26 100644 --- a/architecture/performance.md +++ b/architecture/performance.md @@ -120,6 +120,12 @@ dependencies — the common case — skip navigation entirely via an int compare O(1), and holds ancestors only: a `scope: self` entry would make every container a reference cycle, so none could be freed by refcounting. +`ContextProvider.fetch_context_value` carries the same guard, so a request value +read from the request container costs no navigation frame either. It uses a plain +int compare and **not** the compiler's `_navigate`: that helper prepends a +resolution step, and the calling `Factory` closure prepends its own, so the caller +would appear twice in the breadcrumb. + `Scope._next_deeper` is memoized because it is a constant function of an immutable enum member, consulted per child on the default `build_child_container()` path. diff --git a/modern_di/providers/context_provider.py b/modern_di/providers/context_provider.py index 4cbc67a..5b702ef 100644 --- a/modern_di/providers/context_provider.py +++ b/modern_di/providers/context_provider.py @@ -46,7 +46,11 @@ def resolve(self, container: "Container") -> types.T_co: return value # ty: ignore[invalid-return-type] def fetch_context_value(self, container: "Container") -> "types.T_co | types.UnsetType": - container = container.find_container(self.scope) + # Same-scope int compare before the hop, as the compiled Factory closures do: a request + # value read from the request container skips `find_container`'s frame. Not the compiler's + # `_navigate` — that prepends a resolution step, which the caller then prepends again. + if container.scope != self.scope: + container = container.find_container(self.scope) if container.closed: # guarded: `_prepare()` warns and reopens unconditionally container._prepare() # noqa: SLF001 return container.context_registry.find_context(self.context_type) diff --git a/planning/deferred/2026-08-01-context-kwarg-inline.md b/planning/deferred/2026-08-01-context-kwarg-inline.md index 8b8dba0..2fad6e8 100644 --- a/planning/deferred/2026-08-01-context-kwarg-inline.md +++ b/planning/deferred/2026-08-01-context-kwarg-inline.md @@ -1,5 +1,5 @@ --- -summary: The context-kwarg path in the Factory closures still pays ~4 Python frames per kwarg after the override-guard fix shipped; the remaining scope-hop and compile-time-fold parts need a ruling that ContextProvider.scope and context_type are frozen after registration. +summary: Only the compile-time fold is left: parts (i) and (ii) shipped, and (iii) still needs a ruling that ContextProvider.scope and context_type are frozen after registration -- what it is worth on its own has never been measured apart from the full inline. --- # Inline the context-kwarg path in the Factory closures @@ -7,8 +7,9 @@ summary: The context-kwarg path in the Factory closures still pays ~4 Python fra A `Factory` parameter backed by a `ContextProvider` is resolved per resolve through `Factory._resolve_context_value`, which calls `ContextProvider.fetch_context_value`, which calls `find_container` and -`ContextRegistry.find_context` — roughly four Python frames per context kwarg -(five before the override guard shipped), on the path every framework +`ContextRegistry.find_context` — three Python frames per context kwarg when the +resolving container is already at the provider's scope (four before the hop's +int compare shipped, five before the override guard), on the path every framework integration takes for its per-request values. ## Why it is open @@ -22,9 +23,23 @@ together: calls `overrides_registry.fetch_override(...)` unconditionally.~~ **Shipped.** Measured -41.1 ns (-5.98%) on the no-overrides path, with the override-active path also improving slightly (-4.4 ns). It needed no ruling and was never - blocked; only (ii) and (iii) below remain. + blocked. (ii) has since shipped too; only (iii) remains. - **(ii) Give the context hop the same-scope int-compare fast path** the Factory - closures already have, instead of always calling `find_container`. + closures already have, instead of always calling `find_container`. **Shipped + (2026-08-03).** It needed no ruling. Measured **-2.5%** on `g9_context` + (~707 → ~690 ns, four A/B/A runs: -3.05, -1.29, -3.98, -1.94%, all negative, + against 0.4-1.7% baseline drift). That percentage is **not** comparable to the + -9.2% recorded above for the narrow variant: this one is measured on + `g9_context` at ~707 ns, that one on a ~312-329 ns benchmark. In absolute terms + (ii) is worth ~17 ns, which with (i)'s -41 ns puts the pair at roughly -18% of + that original baseline — so the narrow variant's target is already met, and + what (iii) adds on its own has not been measured apart from the full inline. + Since the timing sits close to the harness's own drift, the suite asserts the + *structural* claim instead: `test_same_scope_context_hop_does_not_call_find_container`. + The predicted `_navigate` trap was confirmed and avoided — a plain int compare, + never `_navigate`, which prepends a resolution step the caller then prepends + again; `test_scope_error_through_a_context_kwarg_carries_one_breadcrumb_step` + pins that and was verified to fail against the double-prepending shape. - **(iii) Fold the bindings at compile time**, capturing `cp.context_type`, `cp.scope`, `cp.provider_id` and `absent_disposition(item)` into the closure instead of re-reading them per resolve. @@ -43,12 +58,16 @@ demonstrated counterexample but does **not** by itself license the capture: The submitted prototype also failed the gates: +12 executable statements, coverage 100% → 99% (eight new uncovered lines, five in the cold cached copy), and two *new* ruff violations (`C901` 15>10, `PLR0912` 14>12) on the hot closure. -The obvious `_navigate`-based implementation of (ii) double-prepends the -resolution-step breadcrumb, and CI stays green while it does. +The obvious `_navigate`-based implementation of (ii) double-prepended the +resolution-step breadcrumb while CI stayed green — no longer true since (ii) +shipped with `test_scope_error_through_a_context_kwarg_carries_one_breadcrumb_step`, +which turns that shape red. ## Revisit trigger A maintainer ruling that a `ContextProvider`'s `scope` **and** `context_type` are fixed once it is registered — the same shape as the scope freeze, extended to identity — **or** context kwargs showing up hot in a profile from a real -integration. Part (i) needs neither and can be picked up at any time. +integration. That ruling is now the only gate: parts (i) and (ii) have shipped, +so (iii) is all that remains. What it is worth on its own is unmeasured — the +-28% above is the full inline, (i) and (ii) included. diff --git a/tests/providers/test_context_provider.py b/tests/providers/test_context_provider.py index 57bf6ad..5f01c4b 100644 --- a/tests/providers/test_context_provider.py +++ b/tests/providers/test_context_provider.py @@ -5,7 +5,12 @@ import pytest from modern_di import Container, Group, Scope, providers -from modern_di.exceptions import ArgumentResolutionError, ContainerClosedWarning, ContextValueNotSetError +from modern_di.exceptions import ( + ArgumentResolutionError, + ContainerClosedWarning, + ContextValueNotSetError, + ScopeNotInitializedError, +) request_context_provider = providers.ContextProvider(scope=Scope.REQUEST, context_type=datetime.datetime) @@ -458,3 +463,71 @@ def test_kwargs_context_provider_without_parsed_signature_injects_present_value( app_container = Container(groups=[_KwargsCtxNoSignatureGroup], context={datetime.datetime: now}) app_container.open() assert app_container.resolve_provider(_KwargsCtxNoSignatureGroup.out) == f"ctx={now!r}" + + +def test_scope_error_through_a_context_kwarg_carries_one_breadcrumb_step() -> None: + # The context hop raises a bare scope error and the Factory closure prepends its own step + # exactly once. Routing the hop through the compiler's `_navigate`, which prepends a step + # itself, would render the factory twice while every other assertion stayed green. + class Cfg: ... + + @dataclasses.dataclass(kw_only=True, slots=True) + class Svc: + cfg: Cfg + + class G(Group): + cfg = providers.ContextProvider(Cfg, scope=Scope.REQUEST) + svc = providers.Factory(creator=Svc, scope=Scope.APP) + + container = Container(scope=Scope.APP, groups=[G]) + container.open() + + with pytest.raises(ScopeNotInitializedError) as exc: + container.resolve(Svc) + + assert str(exc.value).count("Svc") == 1 + + +def test_same_scope_context_hop_does_not_call_find_container(monkeypatch: pytest.MonkeyPatch) -> None: + # The hop is an int compare when the container is already at the provider's scope; calling + # find_container to be handed back the same container costs a frame per context kwarg. + class Cfg: ... + + @dataclasses.dataclass(kw_only=True, slots=True) + class Svc: + cfg: Cfg + + class G(Group): + cfg = providers.ContextProvider(Cfg, scope=Scope.REQUEST) + svc = providers.Factory(creator=Svc, scope=Scope.REQUEST) + + app = Container(scope=Scope.APP, groups=[G]) + app.open() + request = app.build_child_container(scope=Scope.REQUEST, context={Cfg: Cfg()}) + + calls: list[object] = [] + original = Container.find_container + monkeypatch.setattr(Container, "find_container", lambda self, scope: calls.append(scope) or original(self, scope)) + + assert isinstance(request.resolve(Svc), Svc) + assert calls == [] + + # The cross-scope hop must still route through find_container, which is the blessed + # extension point 2026-08-01-scope-map-inline-declined.md protects. + class AppCfg: ... + + @dataclasses.dataclass(kw_only=True, slots=True) + class Wider: + app_cfg: AppCfg + + class G2(Group): + app_cfg = providers.ContextProvider(AppCfg, scope=Scope.APP) + wider = providers.Factory(creator=Wider, scope=Scope.REQUEST) + + app2 = Container(scope=Scope.APP, groups=[G2], context={AppCfg: AppCfg()}) + app2.open() + request2 = app2.build_child_container(scope=Scope.REQUEST) + + calls.clear() + assert isinstance(request2.resolve(Wider), Wider) + assert calls == [Scope.APP]