perf: drop wasted {} defaults in kwargs.get("headers", ...) across enhanced_handler#159
Conversation
Performance Regression Report
|
ee42770 to
c2fb928
Compare
Performance Regression Report
|
…hanced_handler Closes #158. Sub-task of #42. Follow-up to #155 (PR for #151). create_enhanced_handler (the slower / Depends path) had six sites with the same wasted-PyDict_New pattern that #155 fixed in the fast path: 4 sites with `if d:` truthiness check immediately after — drop default to None (None is also falsy, so identity-preserving): * L890 in async enhanced_handler (parse headers) * L1073 in sync enhanced_handler (parse headers) 2 sites already coercing with `or {}` — drop the inner default (`or {}` keeps the contract; the inner `{}` was double-allocating): * L964 in async enhanced_handler (build _Request) * L1148 in sync enhanced_handler (build _Request) 2 dep-context sites that consumer reads back via `headers.items()` — switch to `kwargs.get(...) or {}` to keep the dict contract while saving the wasted default-arg `{}` literal on the headers-present (common) path: * L982 in async enhanced_handler (dep context) * L1164 in sync enhanced_handler (dep context — only when handler uses Depends/Security) All six are identity-preserving for every observed call shape. The DependencyResolver still receives a dict; HeaderParser still gets None or a dict (both falsy when empty); the _Request constructor still gets a dict. Also rolls in a pre-existing isort fix that ruff flagged on the same import block (residue from #148 — first-party turboapi.exceptions import was sorted alongside stdlib). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c2fb928 to
0048106
Compare
💡 Codex ReviewturboAPI/python/turboapi/request_handler.py Lines 1410 to 1411 in c2fb928 When a parameterized handler is annotated as returning a model but uses the supported ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #158. Sub-task of #42. Follow-up to #155 (which fixed the same pattern in the fast handlers).
Summary
create_enhanced_handler(the slow / Depends path) had six sites with the same wasted-PyDict_Newpattern as #155 —kwargs.get("headers", {})where the empty-dict default is either never observed (caller checks truthiness immediately) or paid twice (once for the default arg, again whenor {}re-coerces).headers_dict = kwargs.get("headers", {})kwargs.get("headers")headers=kwargs.get("headers", {}) or {},kwargs.get("headers") or {},"headers": kwargs.get("headers", {}),kwargs.get("headers") or {},headers_dict = kwargs.get("headers", {})kwargs.get("headers")headers=kwargs.get("headers", {}) or {},kwargs.get("headers") or {},"headers": kwargs.get("headers", {}),kwargs.get("headers") or {},All six are identity-preserving for every observed call shape:
if d:sites (890, 1073) treat None and{}the same — both falsy.or {}sites (964, 1148) —or {}already coerces None to{}; the inner default was double-allocating.DependencyResolver._resolve_singlereads back viaheaders.items(), so the value must be a dict; switching tokwargs.get(...) or {}keeps that contract while skipping the wasted default-arg{}literal on the headers-present (common) path.Files / subsystems touched
python/turboapi/request_handler.py— 6 sites increate_enhanced_handler(sync + async). Also rolls in a pre-existing isort fix that ruff flagged on the same import block (residue from perf: hoist parse_qs / HTTPException imports out of fast handler closures #148).benchmarks/bench_enhanced_handler_headers_default.py— new isolated microbench targeting the four shapes used in the patch.No public API change. No dependency change. No
uv.lockchange.Red-to-green
Microbench (load-bearing artifact)
.venv/bin/python benchmarks/bench_enhanced_handler_headers_default.py, n=1M × 5 runs, free-threaded 3.14t, M-series macOS:The patched paths drop 7-9 ns / call (-18 to -25 % median). The
or {}pair also drops p99 by 33% — the wasted{}literal alloc was the source of variance under GC pressure.Aggregate per-request savings on the slow / Depends path: roughly 6 × 7-9 ns ≈ 40-50 ns per request through
enhanced_handler. Modest in absolute terms but every request through this path benefits, and it's the path that runs whenever a route usesDepends,Form,File, orRequestinjection.Targeted tests
Result: 24 passed in 34.54s. Pre-existing
PytestReturnNotNoneWarnings (test_mixed_sync_async,test_async_error_handling) are unchanged by this PR.Risk
Low. The two dep-context sites (982, 1164) are the only ones that needed care —
DependencyResolver._resolve_singledoesfor k, v in headers.items()(and.lower()checks) oncontext["headers"], which would crash on None. Audited and confirmed: switching tokwargs.get(...) or {}keeps the dict contract, so the consumer sees identical inputs.The 4 truthiness-checked sites (890, 964, 1073, 1148) are direct counterparts to the merged #155 / #155 pattern — same identity-preserving swap.
Checklist
main: yes (branched frommainat13a3709).uv.lock,.zig-cache/,zig-out/untouched. Thezig/zig-pkg/dhi-...build artifact was explicitly NOT staged.kwargs.get("headers", ...)shapes in isolation. Caches: stdlib defaults. Cold/warm: warmed steady-state (20k-iter warmup before each measurement). Number of runs: 5 medians. Machine: local M-series macOS Apple Silicon, free-threaded CPython 3.14.0.CONTRIBUTING.md: confirmed.🤖 Generated with Claude Code