Record ADR-010 and an opt-in mypyc-accelerated engine build#48
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 44 44
Lines 4670 4670
Branches 817 817
=========================================
Hits 4670 4670 ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Pull request overview
This PR records ADR-010, capturing the exploration of mypyc as an optional accelerated build as a proposed, deferred decision. It documents the validated path, the measured spike results (~1.30x construction, ~1.38x validation on the core modules), and the costs (per-platform wheels plus a pure-Python fallback) so the analysis is preserved rather than rediscovered. No code is adopted; pure Python remains the default and guaranteed fallback, consistent with ADR-004 and ADR-005.
Changes:
- Add
adr/010-mypyc-accelerated-build.mddocumenting the proposed/deferred mypyc decision, its rationale, consequences, and prerequisites. - Add ADR-010 to the index in
adr/README.md, with table columns realigned to fit the longer title.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
adr/010-mypyc-accelerated-build.md |
New ADR recording mypyc as a proposed, deferred optional accelerated build; claims verified against current code. |
adr/README.md |
Adds the ADR-010 index row and widens the table columns to accommodate the new entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Captures the mypyc exploration as a proposed (not adopted) decision, with
the actual build code behind it.
An earlier spike compiled the four core modules and reported ~1.30x
construction and ~1.38x validation, but it never ran the suite against the
compiled build. Building it for real and running the full suite tells a
narrower story: only the validation engine compiles cleanly.
error.py breaks the error model. It reads default_code off the type
object, which mypyc stores as an instance slot, so .code
raises and the failure path segfaults.
schema.py will not compile a non-native Schema, which it must be, since
DataclassSchema/TypedDictSchema and user code subclass it.
mypyc trips an internal assertion on _compile_self's closure.
markers.py compiles but breaks isinstance on the non-native marker
subclasses, so marker ordering and equality go wrong.
_engine.py, the validation hot loop, compiles with zero source changes and
full behavioral parity (the whole suite passes against it):
construction 0.87s to 0.81s 1.08x
validation 0.80s to 0.61s 1.29x
The payoff is validation, not the construction win the spike advertised,
because construction lives in schema.py. The opt-in build is wired behind
`just build-fast` (scripts/build_accelerated.py drops the compiled .so next
to the source; `just clean` removes it). Pure Python stays the default and
guaranteed fallback; adoption is deferred.
frenck
changed the title
Breaking change
None. The default build stays pure Python and nothing is adopted. This adds an opt-in, dev-only accelerated build behind
just build-fast.Proposed change
Records ADR-010 and the actual opt-in build code behind it, capturing the mypyc exploration as a proposed, deferred decision so the validated path and its costs are written down rather than rediscovered later.
After the profile-driven perf work flipped construction to ~1.64x faster than voluptuous and held a ~2.3x validation lead, the profiles show the hot paths doing irreducible work (validation is ~47% one Python
forloop). The next significant speedup is architectural.An earlier spike compiled the four core modules (
error,markers,_engine,schema) and reported ~1.30x construction and ~1.38x validation, but it never ran the test suite against the compiled build. Building it for real and running the full suite tells a narrower story: "compiles without error" is not "behaves correctly", and three of the four modules fail the suite once compiled.error.pybreaks the error model. TheInvalidhierarchy reads its per-subclassdefault_codeoff the type object, which mypyc stores as an instance slot rather than a class attribute, so reading.coderaises and the failure path segfaults. It is pure failure-path code, so compiling it buys nothing on the hot path.schema.pycannot be compiled.Schemamust stay non-native becauseDataclassSchema/TypedDictSchemaand user code subclass it (mypyc forbids an interpreted class inheriting a compiled one), and a non-nativeSchemathen trips an internal mypyc assertion on_compile_self's nested closure. This is the module that holds construction, the spike's headline.markers.pycompiles but silently breaksisinstance(x, Marker)on the non-native marker subclasses, so marker ordering and equality go wrong.Only
_engine.py, the validation hot loop, compiles cleanly: zero source changes, full behavioral parity (the whole suite passes against it). On a Home-Assistant-representative workload:The payoff is validation, not the construction win the spike advertised, because construction lives in
schema.py. The opt-in build is wired behindjust build-fast, which runsscripts/build_accelerated.pyto drop the compiled.sonext to the source;just cleanremoves it.Decision: pure Python stays the default and guaranteed fallback; the compiled engine is an opt-in accelerator and adoption is deferred. This PR stays a draft on purpose, so the record and the runnable build exist and can be picked up when the trade is worth it.
Type of change
Additional information
Checklist
uv run --no-sync just testpasses locally. A pull request cannot be merged unless CI is green.uv run --no-sync just lintpasses.uv run --no-sync just typecheckpasses.