perf(context): fold the context-kwarg lookup into the compiled closures - #418
Merged
Conversation
Capture cp.provider_id, cp.scope, cp.context_type and the absent disposition at compile time and inline the whole lookup in both the transient and cached closures, rather than re-reading them per resolve through a shared helper. Folding both call sites lets Factory._resolve_context_value be deleted outright: the logic lives in one place per closure instead of being duplicated against a surviving helper. That is the reason to do both rather than only the hot one. Licensed by a maintainer ruling that a registered ContextProvider's scope and context_type are fixed -- the same shape as the scope freeze, extended to identity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t registration Records the ruling that licensed the fold: a registered ContextProvider's scope and context_type are fixed, stated as a contract rather than enforced against direct assignment. resolution.md's 'helpers are reused, not reimplemented' invariant now names the context lookup as its deliberate exception -- the helper was deleted rather than left alongside, so the semantics keep one home per closure instead of two to edit in step. Retires planning/deferred/2026-08-01-context-kwarg-inline.md: all three parts have shipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A 14-mutant sweep found two branches the tests did not actually pin: rewriting
the CACHED hop as _navigate passed all 473 tests (only the transient copy was
covered), and dropping the TRANSIENT override's "continue" passed too (every
transient override test used a parameter with a default, so falling through was
a no-op). Both mutants now fail. The breadcrumb test is parametrized over
cache=False/True and the override test has a transient twin with a
nullable-no-default parameter.
The freeze contract overclaimed: ProviderScopeFrozenError only fires for
registered providers, but the fold captures scope for any provider reaching
plan.context_kwargs -- including one passed only via Factory(kwargs={...}),
which is never registered. providers.md now separates what is enforced from
what is contract.
performance.md hung the _navigate rationale on fetch_context_value, which no
Factory closure calls any more; it moves to the folded loops, which also gained
the comment in code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: f9cb77f | Previous: 8f8611e | Ratio |
|---|---|---|---|
benchmarks/test_guard_by_type.py::test_g16_resolve_by_type |
2934155.1579428455 iter/sec (stddev: 1.2303501871320566e-7) |
2127332.800237366 iter/sec (stddev: 2.0967045897645436e-7) |
0.73 |
benchmarks/test_guard_by_type.py::test_g17_resolve_by_type_large_registry |
3728771.8374690856 iter/sec (stddev: 3.9238774652715766e-8) |
2663213.144443405 iter/sec (stddev: 6.64618449799557e-8) |
0.71 |
benchmarks/test_guard_cold.py::test_g8_cold_first_resolve |
27848.242233468103 iter/sec (stddev: 0.000024577071890394237) |
25644.89286807209 iter/sec (stddev: 0.000024024996496551676) |
0.92 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[1] |
665.3635872518789 iter/sec (stddev: 0.00009995988072133002) |
419.6876140681229 iter/sec (stddev: 0.00016890915361631755) |
0.63 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[2] |
624.5201000305308 iter/sec (stddev: 0.0000989548686449136) |
408.84376071457467 iter/sec (stddev: 0.000033474561974910216) |
0.65 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[4] |
532.7765111054996 iter/sec (stddev: 0.0002452088619204761) |
377.50090441241196 iter/sec (stddev: 0.0000633292454424772) |
0.71 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[1] |
2642.84383739745 iter/sec (stddev: 0.0000300290704184033) |
2423.258728066733 iter/sec (stddev: 0.00002411697153620591) |
0.92 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[2] |
1960.969260046368 iter/sec (stddev: 0.0004107525974305188) |
1760.13081057456 iter/sec (stddev: 0.0003329155349136469) |
0.90 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[4] |
1372.3052313783112 iter/sec (stddev: 0.00004267291727811804) |
1242.614868017291 iter/sec (stddev: 0.00005189822497107809) |
0.91 |
benchmarks/test_guard_lifecycle.py::test_g6_build_child_container |
878646.1036178512 iter/sec (stddev: 2.963452705713142e-7) |
631597.2539675933 iter/sec (stddev: 4.6760996982059613e-7) |
0.72 |
benchmarks/test_guard_lifecycle.py::test_g6b_build_child_container_auto_scope |
838901.1799791306 iter/sec (stddev: 2.567010147039892e-7) |
608046.658511328 iter/sec (stddev: 4.469028002164705e-7) |
0.72 |
benchmarks/test_guard_lifecycle.py::test_g7_request_lifecycle_batch |
2790.1948432490653 iter/sec (stddev: 0.000014984627003587684) |
2266.697117233661 iter/sec (stddev: 0.00000805460526087513) |
0.81 |
benchmarks/test_guard_lifecycle.py::test_g7c_event_loop_floor_control |
83844.05365981365 iter/sec (stddev: 0.0000012982532337613132) |
61640.17977720311 iter/sec (stddev: 0.0000027994175814498994) |
0.74 |
benchmarks/test_guard_lifecycle.py::test_g13_teardown_at_scale |
53351.65493628555 iter/sec (stddev: 0.000001767688473722379) |
43550.20004891142 iter/sec (stddev: 0.0000020500682632179295) |
0.82 |
benchmarks/test_guard_resolve.py::test_g1_transient_resolve |
1805044.96739634 iter/sec (stddev: 4.4708582310802333e-7) |
1327867.8290142089 iter/sec (stddev: 4.48972019786081e-7) |
0.74 |
benchmarks/test_guard_resolve.py::test_g2_cached_resolve |
5208556.318446741 iter/sec (stddev: 4.2070984966577395e-8) |
3362715.638606728 iter/sec (stddev: 4.481897399985886e-8) |
0.65 |
benchmarks/test_guard_resolve.py::test_g3_deep_chain |
764986.5431482138 iter/sec (stddev: 3.046169189442597e-7) |
514887.3926180792 iter/sec (stddev: 5.71698340774013e-7) |
0.67 |
benchmarks/test_guard_resolve.py::test_g4_wide_resolve |
425860.65468166047 iter/sec (stddev: 6.445217920765483e-7) |
327295.7382443137 iter/sec (stddev: 5.862507423986478e-7) |
0.77 |
benchmarks/test_guard_resolve.py::test_g5_cross_scope |
1603408.5592721386 iter/sec (stddev: 2.4964845210040195e-7) |
1174831.4344373737 iter/sec (stddev: 3.9538882216422895e-7) |
0.73 |
benchmarks/test_guard_resolve.py::test_g9_context_resolve |
959016.7404342955 iter/sec (stddev: 2.73685402944663e-7) |
663464.8243621978 iter/sec (stddev: 4.6311516174683525e-7) |
0.69 |
benchmarks/test_guard_resolve.py::test_g12_override_active_resolve |
573205.6930783733 iter/sec (stddev: 2.347292133609714e-7) |
390323.67702238314 iter/sec (stddev: 4.845963586751049e-7) |
0.68 |
benchmarks/test_guard_validate.py::test_g10_validate_deep_chain |
32924.254230282815 iter/sec (stddev: 0.0000023685142710117865) |
30854.040774297147 iter/sec (stddev: 0.0000027232919492231963) |
0.94 |
benchmarks/test_guard_validate.py::test_g11_validate_wide |
20552.14431816973 iter/sec (stddev: 0.0000036834116984776093) |
18297.785108003893 iter/sec (stddev: 0.0000037676184285495203) |
0.89 |
This comment was automatically generated by workflow using github-action-benchmark.
4 tasks
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
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.
Why
A
Factoryparameter backed by aContextProviderre-readcp.provider_id,cp.scope,cp.context_typeand the parameter's absent-disposition on every resolve, through a shared helper that calledfetch_context_value, which calledfind_containerandfind_context. That is the path every framework integration takes for its per-request values.This is part (iii) of
planning/deferred/2026-08-01-context-kwarg-inline.md, which this PR retires — (i) and (ii) shipped earlier.Design
The four bindings are folded into the compiled closure at compile time, and the whole lookup — override front-guard, scope compare, closed-container reopen, registry read, disposition — is inlined.
Both closures are folded, and
Factory._resolve_context_valueis deleted. Folding only the hot one would leave the helper alive for the other, so the same disposition and override logic would exist in two places and have to be edited in lockstep. Deleting it keeps the semantics at one home per closure. That is why this touches the cached path too, which is not itself hot.The licence is your ruling: a
ContextProvider'sscopeandcontext_typeare fixed once something has resolved through it.architecture/providers.mdstates precisely how much of that is enforced versus contract —ProviderScopeFrozenErrorcoversscopeonly for registered providers, so a provider passed only viaFactory(kwargs={...})is unenforced, andcontext_typeis unenforced on both routes.The value itself is still read live on every resolve; only the binding is frozen.
Non-goals
__setattr__guard; the contract is documented rather than mechanised.ContextProvider.resolve— the direct-resolve path keeps callingfetch_context_value, which is now its only caller.Verification
just test-ci: 475 passed, 100% line coverage.just lint-ci: clean.Measured on the repo's blessed A/B/A harness (
ab_run.sh main HEAD g9_context), three runs: -5.15%, -6.42%, -6.14% against baseline drift 0.28-1.52%. Median ~-6.1%, ~42 ns per context kwarg — about 2.4x what part (ii) bought.Coverage forced five new tests onto the cached closure's copy of the folded branches, which the transient tests do not reach.
A review ran a 14-mutant sweep and found two branches my tests did not actually pin, both now fixed and verified to fail:
_navigatepassed all 473 tests — only the transient copy was covered. The breadcrumb test is now parametrized overcache=False/True.continuepassed too, because every existing transient override test used a parameter with a creator default, making the fall-through a no-op. A twin test with a nullable-no-default parameter now catches it.Two documentation corrections came from the same review: the freeze contract overclaimed (it implied
ProviderScopeFrozenErrorbacks the whole thing), andperformance.mdhung the "never_navigate" rationale on a function noFactoryclosure calls any more.Before merging
architecture/providers.md(the identity contract),architecture/performance.md(the fold), andarchitecture/resolution.md— whose "helpers are reused, not reimplemented" invariant now names this as its deliberate exception.just lint-ciandjust test-cipass.Retires
planning/deferred/2026-08-01-context-kwarg-inline.md.