Skip to content

Commit 7f55f03

Browse files
lesnik512claude
andauthored
perf(container): inline the resolve_provider body into resolve (#419)
* perf(container): inline the resolve_provider body into resolve Container.resolve called find_provider and then tail-called resolve_provider, paying a second Python frame on the by-type path -- the one every @Inject marker and framework integration takes. It now carries its own copy of that body. Measured -19% (~38 ns) per by-type resolve on g16_by_type, with the by-reference control flat. The duplicated 8-line body is the standing cost and is permanent: both copies must be edited together. Licensed by planning/decisions/2026-08-03-resolve-provider-not-a-seam.md, which records that resolve_provider stopped being an interception seam when the compiled resolvers landed in 2.29.0 -- an override already sees only top-level calls, demonstrated at 1 call for a 4-node chain. The 3.10 coverage gate needed more than the deferred item predicted. A by-reference cycle test does reach resolve_provider's RecursionError handler and fails without it, but coverage cannot record that line when tracing the whole modern_di package below 3.12: the RecursionError tears the trace function down first. It records fine when tracing container.py alone, which is how the earlier check missed this. The line therefore carries a pragma with the evidence, and the test stays as the real guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review fixes: drop the pragma, restore real 3.10 coverage The review refuted the pragma and its justification. The comment claimed resolve_provider's handler is "reached only by a by-reference cycle" -- false: a by-type cycle re-enters resolve_provider through the back-edge thunk, hitting it six times. What actually changed is that the two tests reaching that line WITHOUT a stack overflow (SelfRec raises RecursionError directly, tracer intact) go by type, so they now land on resolve's copy. The fix is a by-reference twin of that overflow-free shape, not a pragma: test_by_reference_recursionerror_passes_through. 3.10 and 3.14 both back to 100.00% with no suppression on a production line. The cycle test stays, with a corrected comment: it contributes no coverage but is the sole behavioural guard on resolve_provider's own conversion -- replacing that conversion with a bare re-raise kills only that test. Also corrects four architecture pages the review found still naming resolve_provider as the sole compile-and-dispatch path: resolution.md's entry point list, performance.md's inlined-lookup counts (four across three call sites -> six across four) plus a note recording the duplication where a reviewer would look for it, concurrency.md's reopen paragraph, and containers.md's self-heal line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent df72b27 commit 7f55f03

9 files changed

Lines changed: 168 additions & 95 deletions

File tree

architecture/concurrency.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ state at a single-threaded edge, nothing prevents several threads from then
3232
independently calling `resolve` on that (now-closed) container at once — each
3333
unaware the others are doing the same. A container is open from construction
3434
(see [containers.md](containers.md#optional-open-lifecycle)), so this is the
35-
only path back to `closed = True` in the first place. `resolve_provider` calls
36-
`_prepare()` whenever `self.closed` is `True`; `_prepare()` warns and sets
35+
only path back to `closed = True` in the first place. `resolve` and
36+
`resolve_provider` each call `_prepare()` whenever `self.closed` is `True`; `_prepare()` warns and sets
3737
`closed = False`, unlocked. The reopen needs no lock because it is idempotent —
3838
N threads racing a closed container all write the same `False`, and they go on
3939
to share one singleton via the cache lock below. What is *not* serialized is the

architecture/containers.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ explicitly under `Container` rather than inferred from a type annotation).
137137
`close_async()` can clean them up.
138138

139139
2. **`closed = True`** — set in a `finally` block, even if finalizers raised. A subsequent
140-
`resolve_provider` (or a nested provider resolving at a closed ancestor scope) self-heals: it
140+
`resolve` / `resolve_provider` (or a nested provider resolving at a closed ancestor scope) self-heals: it
141141
reopens the container via `_prepare()` and emits `ContainerClosedWarning`, rather than raising.
142142
Re-enter the container via `with`/`async with`, or call `container.open()`, for a silent reopen
143143
instead — see [Optional-open lifecycle](#optional-open-lifecycle).
@@ -157,9 +157,11 @@ close, ready to be returned again without re-running the creator.
157157

158158
### Open and reopen (context-manager protocol)
159159

160-
`_prepare()` — not `open()` — is the primitive the resolve path calls: `resolve_provider` (and the
161-
compiled-resolver dispatch it wraps) calls it whenever `self.closed` is `True`, before doing anything
162-
else. That caller-side `if closed` check is the only guard: `_prepare()` itself takes no lock and makes
160+
`_prepare()` — not `open()` — is the primitive the resolve path calls: `resolve_provider` and `resolve`
161+
(and the compiled-resolver dispatch they wrap) call it whenever `self.closed` is `True`, before doing
162+
anything else — `resolve` holds its own copy of that check rather than delegating
163+
([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)).
164+
That caller-side `if closed` check is the only guard: `_prepare()` itself takes no lock and makes
163165
no re-check, warning with `ContainerClosedWarning` and clearing `closed` unconditionally. Concurrent
164166
reuse of one closed container therefore warns **at least once**, not exactly once — see
165167
[concurrency.md](concurrency.md#the-lifecycle). `open()` is a separate, public entry point that clears

architecture/performance.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,32 @@ deliberate — there is no interpreted fallback to inherit shared behaviour from
5353

5454
## Inlined memo hits
5555

56-
Four lookups are hand-inlined across three call sites, with the method called
56+
Six lookups are hand-inlined across four call sites, with the method called
5757
only on a miss:
5858

5959
| Call site | Inlines | Method still owns |
6060
|---|---|---|
6161
| `Container.resolve_provider` | `providers_registry._resolvers.get(pid)` | the cycle guard and memo write, on a miss |
62+
| `Container.resolve` | `providers_registry._providers.get(dependency_type)` and `._resolvers.get(pid)` | `find_provider`'s absence result, and `resolver_for`'s cycle guard and memo write, on a miss |
6263
| `_compile_cached_factory`'s `resolve` | `cache_registry._items.get(pid)` | `setdefault`, which is what makes concurrent first-resolvers share one `CacheItem` |
6364
| `_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 |
6465

6566
In each case the method being inlined *opens with exactly that lookup and
6667
returns*, so the inline is not a reimplementation that can drift — it is the
67-
method's own fast path, hoisted past its frame. All three keep calling the real
68+
method's own fast path, hoisted past its frame. All four keep calling the real
6869
method on a miss, so the miss-path invariants (cycle detection, single shared
6970
`CacheItem`, the dangling-source error) are untouched.
7071

72+
`Container.resolve` goes further than a hoisted lookup: it carries a **copy of
73+
`resolve_provider`'s whole body** — the closed check, the memo hit, the
74+
`resolver_for` fallback, the resolver call and the `RecursionError` conversion.
75+
That is the one place in the library where a block of logic is deliberately
76+
duplicated rather than shared, and both copies must be edited together. It is
77+
worth ~-19% on a by-type resolve, and it is licensed by `resolve_provider` not
78+
being an interception seam — an override has seen only top-level calls since the
79+
compiled resolvers landed
80+
([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)).
81+
7182
The alias case inlines two lookups rather than one, because the hop is two indirections deep: without them an
7283
alias costs four Python frames (`_find_source`, `find_provider`, `resolve_provider`, then the source's
7384
resolver) where every `Factory` dependency costs one.

architecture/resolution.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ made true.
77
## Entry points
88

99
- `container.resolve(SomeType)` — looks up `SomeType` in `providers_registry` (raising
10-
`ProviderNotRegisteredError`, with closest-match suggestions, if none is registered), then delegates to
11-
`resolve_provider`.
10+
`ProviderNotRegisteredError`, with closest-match suggestions, if none is registered), then dispatches to the
11+
compiled resolver itself. It holds its own copy of `resolve_provider`'s body rather than delegating, so the
12+
by-type path pays no extra frame; the two copies must be edited together
13+
([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)).
1214
- `container.resolve_provider(provider)` — resolves by provider reference, skipping the registry lookup.
1315
It reopens the entry container if it was closed (see [containers.md](containers.md#closing)), then calls
1416
`providers_registry.resolver_for(provider)(self)` and wraps any escaped `RecursionError` (the runtime cycle

architecture/validation.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,10 @@ as a resolution breadcrumb — including the aligned scope column — so a cycle
9292
read identically. See [resolution.md](resolution.md#one-renderer) for that drawer.
9393

9494
> **Runtime resolution has a cycle guard too — but `validate()` remains the way to see all errors up front.**
95-
> `Container.resolve_provider` wraps the compiled-resolver dispatch (`resolver_for(provider)(self)`) in
96-
> `try/except RecursionError`. The
95+
> `Container.resolve_provider` **and `Container.resolve`** each wrap the compiled-resolver dispatch
96+
> (`resolver_for(provider)(self)`) in `try/except RecursionError``resolve` carries its own copy of that
97+
> body rather than delegating, so the by-type entry point pays no extra frame
98+
> ([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)). The
9799
> handler first short-circuits: if the registry is already validated (`_validated` is `True`), the static
98100
> graph is known acyclic, so the overflow is genuine self-recursion and the `RecursionError` re-raises untouched
99101
> without any walk. Otherwise, when an unvalidated circular graph's first resolve overflows the stack, the handler

modern_di/container.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -191,15 +191,28 @@ def lock(self) -> "threading.RLock | None":
191191
return self._lock
192192

193193
def resolve(self, dependency_type: type[types.T]) -> types.T:
194-
"""Resolve a dependency by its type."""
195-
provider = self.providers_registry.find_provider(dependency_type)
196-
if not provider:
194+
"""Resolve a dependency by its type.
195+
196+
Carries its own copy of `resolve_provider`'s body rather than calling it: the extra
197+
frame is ~19% of a by-type resolve. The duplication is deliberate and the two must be
198+
edited together -- see planning/decisions/2026-08-03-resolve-provider-not-a-seam.md.
199+
"""
200+
registry = self.providers_registry
201+
provider = registry._providers.get(dependency_type) # noqa: SLF001
202+
if provider is None:
197203
raise exceptions.ProviderNotRegisteredError(
198204
provider_type=dependency_type,
199-
suggestions=suggester.suggest(dependency_type, self.providers_registry),
205+
suggestions=suggester.suggest(dependency_type, registry),
200206
)
201-
202-
return self.resolve_provider(provider)
207+
if self.closed:
208+
self._prepare()
209+
try:
210+
resolver = registry._resolvers.get(provider.provider_id) # noqa: SLF001
211+
if resolver is None:
212+
resolver = registry.resolver_for(provider)
213+
return resolver(self)
214+
except RecursionError as exc:
215+
_handle_recursion_error(provider, self, exc)
203216

204217
def resolve_dependency(self, dependency: "AbstractProvider[types.T] | type[types.T]") -> types.T:
205218
"""Resolve a provider reference or a type — the marker-dispatch entry point for integrations.
@@ -213,7 +226,10 @@ def resolve_dependency(self, dependency: "AbstractProvider[types.T] | type[types
213226
return self.resolve(dependency)
214227

215228
def resolve_provider(self, provider: "AbstractProvider[types.T]") -> types.T:
216-
"""Resolve a specific provider by reference via its compiled resolver."""
229+
"""Resolve a specific provider by reference via its compiled resolver.
230+
231+
`resolve` holds a copy of this body; any change here belongs there too.
232+
"""
217233
if self.closed:
218234
self._prepare()
219235
try:
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
---
2+
summary: `resolve_provider` is not an interception seam and `Container` subclassing is not a supported way to intercept resolution — since the compiled resolvers landed it has only ever seen top-level calls, so inlining it into `resolve()` narrows nothing that worked.
3+
---
4+
5+
# `resolve_provider` is not an interception seam
6+
7+
**Decision:** `Container.resolve_provider` is an entry point, not a hook. Overriding
8+
it in a `Container` subclass is not a supported way to observe or intercept
9+
resolution, and the resolve path is free to bypass it. This licenses inlining its
10+
body into `Container.resolve`. `find_container` is **not** affected and remains a
11+
blessed extension point.
12+
13+
## Context
14+
15+
Inlining `find_provider` + `resolve_provider` into `Container.resolve` measures
16+
**-19% (~38 ns) on every by-type resolve** — the path every `@inject` marker and
17+
framework integration takes. It was deferred partly because the same structural
18+
objection that killed
19+
[`2026-08-01-scope-map-inline-declined.md`](2026-08-01-scope-map-inline-declined.md)
20+
appears to apply: `resolve_provider` is a public method on a subclassable class,
21+
and `Container.__init__` builds children via `self.__class__`, so a subclass rides
22+
the whole tree. Bypassing it would mean a subclass's override no longer runs for
23+
by-type calls.
24+
25+
## Decision & rationale
26+
27+
**It is already not a seam, and that is measurable rather than arguable.** Since
28+
the compiled resolvers shipped in 2.29.0, a resolver calls its dependencies'
29+
resolvers *directly*; nothing routes a nested node through `resolve_provider`. Its
30+
only callers are `resolve()`, `resolve_dependency()`, and the cycle back-edge
31+
thunk in `ProvidersRegistry.resolver_for`. Demonstrated on `main` before this
32+
change: a `Container` subclass overriding `resolve_provider` and resolving a
33+
**4-node chain** records exactly **1** call — the top-level one. An override has
34+
never seen the graph. Inlining removes one of the three top-level call sites; the
35+
by-reference and marker-dispatch entries still route through it.
36+
37+
So the thing the objection protects does not exist. What a subclass can still do
38+
after this change is instrument the *entry points* by overriding `resolve` and
39+
`resolve_provider` — which is what someone wanting that would actually reach for,
40+
and it keeps working.
41+
42+
**This is deliberately narrower than the `_scope_map` ruling, which stands.**
43+
`find_container` is consulted on every cross-scope hop, and the container it
44+
returns owns the cached instance and runs its finalizer — bypassing an override
45+
there silently relocates lifecycle ownership, which is a bug, not a missed hook.
46+
`resolve_provider` has no such consequence: bypassing an override loses
47+
observation, not correctness. The two are not the same call and are not being
48+
ruled on together.
49+
50+
**Field check.** An audit of all 13 sibling integration wheels found zero
51+
`Container` subclasses and zero `resolve_provider` overrides. `Container`
52+
subclassing is not documented as an extension point anywhere in `architecture/` or
53+
`docs/`.
54+
55+
**Accepted costs**, disclosed rather than discovered later:
56+
57+
- A genuinely duplicated ~8-line body (closed check, memo hit, `resolver_for`
58+
fallback, resolver call, `RecursionError` conversion) now lives in both `resolve`
59+
and `resolve_provider` and must be edited in lockstep. This is the real price and
60+
it is permanent.
61+
- An exception raised through `resolve()` loses one traceback frame (5 → 4;
62+
`resolve_provider` no longer appears). Verified directly.
63+
- Recursion headroom moves by one frame in the benign direction.
64+
65+
**Consequence worth naming.** Together with
66+
[`2026-07-30-debug-resolution-tracing-declined.md`](2026-07-30-debug-resolution-tracing-declined.md),
67+
modern-di offers no built-in way to observe *per-node* resolution. That was already
68+
true — the compiled resolvers removed the last interior call — and this decision
69+
records it rather than creating it. Entry-point instrumentation remains available
70+
by overriding both public entry methods.
71+
72+
## Revisit trigger
73+
74+
A concrete request for per-resolve interception from a real integration or user.
75+
The answer then is a designed seam with a stated contract — not a re-blessing of
76+
subclass overrides, which the compiled resolve path stopped honouring in 2.29.0.

planning/deferred/2026-08-01-resolve-by-type-inline.md

Lines changed: 0 additions & 76 deletions
This file was deleted.

tests/test_runtime_cycle_guard.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,3 +220,43 @@ class G(Group):
220220
pytest.fail("expected CircularDependencyError")
221221
finally: # pragma: no cover
222222
sys.setrecursionlimit(limit)
223+
224+
225+
def test_by_reference_cycle_raises_circular_dependency_error() -> None:
226+
# By-reference twin of the by-type test above, and the only guard on `resolve_provider`'s
227+
# own conversion: `Container.resolve` carries a second copy, so replacing the conversion
228+
# here with a bare re-raise still passes every by-type cycle test. This is a behavioural
229+
# guard, not a coverage one -- the line is covered by
230+
# `test_by_reference_recursionerror_passes_through`, which reaches it without an overflow.
231+
# Same `except`-clause shape and shallow limit, per `_SHALLOW_RECURSION_LIMIT`.
232+
container = Container(groups=[CycleGroup])
233+
container.open()
234+
original_limit = sys.getrecursionlimit()
235+
sys.setrecursionlimit(_SHALLOW_RECURSION_LIMIT)
236+
try:
237+
container.resolve_provider(CycleGroup.a)
238+
except exceptions.CircularDependencyError as exc: # pragma: no cover
239+
_assert_simple_cycle(exc)
240+
else: # pragma: no cover
241+
pytest.fail("expected CircularDependencyError")
242+
finally: # pragma: no cover
243+
sys.setrecursionlimit(original_limit)
244+
245+
246+
def test_by_reference_recursionerror_passes_through() -> None:
247+
# Reaches `resolve_provider`'s handler WITHOUT a stack overflow, so the trace function is
248+
# still alive and the line is recorded on CPython below 3.12. The by-type twin of this
249+
# (`test_validated_graph_reraises_recursionerror_without_walk`) now lands on `resolve`'s
250+
# own copy of the handler, leaving this entry point otherwise untraced.
251+
class SelfRec:
252+
def __init__(self) -> None:
253+
raise RecursionError
254+
255+
class G(Group):
256+
s = providers.Factory(scope=Scope.APP, creator=SelfRec)
257+
258+
container = Container(scope=Scope.APP, groups=[G])
259+
container.validate()
260+
container.open()
261+
with pytest.raises(RecursionError):
262+
container.resolve_provider(G.s)

0 commit comments

Comments
 (0)