Single-path compiled-closure resolver - #334
Merged
Merged
Conversation
…or research Supersedes the additive fast path (2026-07-16.01): make the compiled closure the single resolve path, hoisting the per-node tax to build time. Backed by a competitor-perf audit (dishka/wireup/dependency-injector/that-depends source + CPython 3.12+ microbenchmarks) showing closures capture ~80-90% of the ceiling and exec buys 0-4% at fixed arity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dation.md The runtime cycle test only fails against unfixed rotation logic when run alongside the rest of the file (ambient stack depth seeds which node the guard catches on); in isolation it can pass by coincidence. Add a direct unit test of build_cycle_error that seeds a cycle ring at a non-minimum provider_id node, independent of any runtime stack state, giving a reliable RED for the rotation. Also update validation.md's runtime-guard section, which still described the old prepend_step accumulation this change (4f7031b) removed.
…rential scaffold Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gs in differential harness Container.resolve_provider's compiled branch dropped the entry-container _warn_and_reopen_if_closed() call that the interpreted path always made, so resolving a shallower-scope provider from a closed entry container skipped the warn+reopen. Restore it ahead of dispatch, mirroring _resolve_provider_interpreted; the compiled closure's own target.closed guard is unchanged (it still covers a distinct closed cross-scope target). Also widen the differential harness's outcome comparison to include emitted warning names (previously warning-blind), add the missing closed-REQUEST-child / open-entry-with-closed-target cases that pin the above fix, and fix two stale comments referencing cut-1 names.
Replace creator(**kwargs) with a positional creator(v0, v1, ...) call in the compiled transient and cached resolvers, but only when a compile-time predicate (_positional_names) proves it safe: the whole parsed signature must be provider deps, in order, with no static/context/default-omitted/keyword-only params and no kwargs-overlay extra. Anything else keeps the kwargs call. The choice is made once at compile time (two closures), so the hot path carries no per-call branch. Add a minimal is_keyword_only signal to SignatureItem so the predicate can reject keyword-only params. Both positional closures preserve the two-phase error handling (dependency _STEP_ERRORS breadcrumb, then TypeError -> body propagate / binding -> CreatorCallError). Differential-verified against the forced-interpreted path; 100% coverage on both branches and both call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_positional_names judged a creator eligible for the positional fast path whenever tuple(plan.provider_kwargs) == tuple(f._parsed_kwargs), but a positional-only parameter with a default is dropped entirely by _parse_parameter and never enters _parsed_kwargs. Since positional-only params come first in the signature, the positional call then bound args one slot too early -- a silent wrong-value bug, not a crash. Guard against it by inspecting the creator's signature for any POSITIONAL_ONLY parameter once the other checks pass, falling back to the kwargs call when one is present.
…promote docs Finalize the single-path compiled-resolver rewrite: the shipped tree now has exactly one resolve path. Remove the DEV-ONLY `_force_interpreted` module global and `_resolve_provider_interpreted` from container.py (killing the last global state), delete the differential harness, and delete the now-dead interpreted bodies (Factory.resolve/_resolve_kwargs, Alias.resolve, _ContainerProvider.resolve). The two compiled Factory paths' `plan.unwireable` bridges used to route to the interpreted path; they now dispatch to a compiled `_compile_unwireable_factory` that mirrors the old `_resolve_kwargs` unwireable branch (override front-guard, scope navigation, closed-target reopen, freshly-built ArgumentResolutionError). AbstractProvider.resolve decision (by grep): after removing the dead overrides, the only remaining `provider.resolve(` caller is the compiled ContextProvider closure delegating to `ContextProvider.resolve`. `integrations.Marker.resolve` is a different class (delegates to resolve_dependency), and no external contract calls `.resolve` generically. So the `@abstractmethod resolve` is removed from AbstractProvider and ContextProvider.resolve stays concrete; the other providers no longer implement resolve. Coverage: deleting the harness dropped resolver_compiler.py to 88%; each lost branch got an equivalent single-path test in the matching standard test file (no resurrected harness). test-ci green at 100% line coverage (410 passed). Docs promoted in-PR: architecture/resolution.md rewritten for the compiled path; providers.md/validation.md touched where they narrated the old resolve_provider -> provider.resolve -> _resolve_kwargs recursion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…g2 regression) The from-import in ProvidersRegistry.resolver_for ran on every call, including both warm-hit early returns, though compile_resolver is only used on a cache miss. Moving it past the early returns removes ~260ns/call from the hot path and turns the g2 warm-cached-singleton regression (+18% vs main) into a -31% win; guard tier and comparative-vs-dishka re-measured, bundle summary updated with corrected numbers.
…guard inlining; pin cached-context coverage Whole-branch review polish: resolution.md Step 4 no longer implies the wiring plan is built per-resolve (it is consulted at compile time); a comment marks the per-closure override front-guard as intentionally inlined; and a deterministic test pins the cached-factory present-context cold-build branch that was only covered incidentally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: 6074f79 | Previous: 6813ebc | Ratio |
|---|---|---|---|
benchmarks/test_guard_lifecycle.py::test_g6_build_child_container |
488899.78372443863 iter/sec (stddev: 0.00006621920099425868) |
368222.00446106744 iter/sec (stddev: 0.00006328366629011951) |
0.75 |
benchmarks/test_guard_lifecycle.py::test_g7_request_lifecycle |
85303.38316817382 iter/sec (stddev: 0.000011414774074094085) |
73361.86418823789 iter/sec (stddev: 0.000013220898269650964) |
0.86 |
benchmarks/test_guard_resolve.py::test_g1_transient_resolve |
1625263.7908536857 iter/sec (stddev: 5.676737178607e-7) |
511943.998335751 iter/sec (stddev: 6.131396685959697e-7) |
0.31 |
benchmarks/test_guard_resolve.py::test_g2_cached_resolve |
3241792.0281775575 iter/sec (stddev: 4.0694250184531693e-8) |
1550807.9045308023 iter/sec (stddev: 6.820993252995485e-8) |
0.48 |
benchmarks/test_guard_resolve.py::test_g3_deep_chain |
739265.6620024806 iter/sec (stddev: 2.8921966292041304e-7) |
179163.0979694334 iter/sec (stddev: 9.649800378847799e-7) |
0.24 |
benchmarks/test_guard_resolve.py::test_g4_wide_resolve |
439670.94466186065 iter/sec (stddev: 3.735902439394497e-7) |
100293.67307553429 iter/sec (stddev: 0.0000011331824315185444) |
0.23 |
benchmarks/test_guard_resolve.py::test_g5_cross_scope |
1458196.464609265 iter/sec (stddev: 2.614885698245671e-7) |
504042.03203661385 iter/sec (stddev: 5.251446376383832e-7) |
0.35 |
This comment was automatically generated by workflow using github-action-benchmark.
…xists) The lazy import inside resolver_for carried a stale "avoid import cycle" claim. providers_registry is imported at runtime only by container.py; every other reference (resolver_compiler, wiring, factory) is TYPE_CHECKING-guarded, so providers_registry -> resolver_compiler -> factory/alias/... has no runtime back-edge. Verified: `import modern_di` and the full suite import cleanly. No perf change on the resolve path: the import was already below the warm-hit early return (it only ran on a compile/miss); at module level it is a one-time load and the per-resolve path is untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lesnik512
added a commit
that referenced
this pull request
Jul 17, 2026
…evers (#335) The single-path compiled resolver shipped (2026-07-16.02, #334). Capture the comparative-tier standing (5-run medians): modern-di is mid-pack and the only zero-dep pure-Python framework holding its own — beats Cython dependency-injector on 3/4, ties dishka on lifecycle, trails the codegen frameworks 1.3-1.9x on transient/chain. Two levers remain: C2 warm-singleton whole-aggregate memoization (highest leverage, non-trivial due to per-container caches + runtime overrides + cache clear), and the codegen ceiling (accepted floor, no exec). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lesnik512
added a commit
that referenced
this pull request
Jul 17, 2026
…rovider set (#341) docs/providers/advanced-api.md promised, under "## Supported extension points", that you could subclass AbstractProvider and implement resolve(container). #334 closed the provider set and deleted AbstractProvider.resolve, so that recipe now raises TypeError at first resolve. The docs were left describing a capability the code no longer has. Custom provider support was never designed — it was an emergent property of Container.resolve_provider ending in a polymorphic provider.resolve(self) call, which the compiled resolver replaced with dispatch on exact type identity. The closure was deliberate, asserted by test_resolve_provider_raises_for_unhandled_provider_type and reasoned from in 2026-07-17-per-provider-compile-seam-declined; only the docs were left behind. Retract the section and state the closed set positively, naming the two things a user cannot infer: a Factory subclass fails identically (dispatch is type(x) is, not isinstance), and validate() does not catch it (validation walks the dependency graph, compilation is lazy at first resolve). Three adjacent references to methods #334 deleted are corrected in the same pass: find_container ("the primitive a custom AbstractProvider.resolve calls"), CacheSettings.is_async_finalizer (credited to Factory.resolve; cache_registry.py:68 is now its only reader), and the _lock internals entry (singleton creation "inside Factory.resolve"; the compiled cached resolver now passes the lock to CacheItem.get_or_create). container.md's See-also item drops subclassing. No architecture/ promotion: architecture/providers.md documents the four concrete types and never claimed the set was open. Decision: planning/decisions/2026-07-17-custom-providers-retracted.md Change: planning/changes/2026-07-17.05-retract-custom-provider-docs.md Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lesnik512
added a commit
that referenced
this pull request
Jul 17, 2026
Cover the 2.28.0->2.29.0 backlog: the single-path compiled resolver (#334) and its measured perf, the wiring-plan memoization (#326), the three correctness fixes (#340 ContextProvider-via-kwargs default, #321 NoneType default/nullability, #320 validate traverses kwargs=), and the closed provider set under an explicit breaking-change heading (per planning/decisions/2026-07-17-custom-providers-retracted.md, the release notes carry that warning, not the docs). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the interpreted resolve recursion with one per-provider compiled-closure resolver, memoized on
ProvidersRegistrylike the wiring plan. Each closure front-guards its own override, navigates its target once (same-scope deps skip via an int compare), inlines the kwargs build + creator call, and calls dependency resolvers by reference. All four provider types compile; the dev-only differential scaffold that proved equivalence was deleted before merge — the shipped tree has exactly one resolve path.Rationale, design, and rejected alternatives (exec/codegen, additive fallback): see the change file — this body does not restate them.
planning/changes/2026-07-16.02-single-path-compiled-resolver.mdplanning/audits/2026-07-16-competitor-perf-research-report.mdBehavior: equivalent to the prior interpreted path (dev-time differential A/B harness across the full battery), plus canonical, self-contained cycle errors. 411 tests, 100% line coverage, one resolve path.
Perf (measured, medians). Guard tier vs
main: transient -37%, warm-cached singleton -31%, deep-chain -61%, wide -62%, lifecycle -6.5%, build-child ~flat — no regression. Comparative vs dishka: C1 1.44x, C2 1.19x, C3 ~1.88x, C4 ~par. The gap to dishka narrowed from the original ~3-5x but is not closed; closing the remainder would need the deferred whole-aggregate memoization or codegen (out of scope here). Honest call for review.🤖 Generated with Claude Code