perf(compiler2): interface-method-name pre-filter in MIR dispatch + is_subtype_of assumption fast path - #4044
perf(compiler2): interface-method-name pre-filter in MIR dispatch + is_subtype_of assumption fast path#4044hellovai wants to merge 1 commit into
Conversation
…s_subtype_of assumption fast path dispatch_target_for_concrete enumerated every impl block in the package closure for every method call and field access, only to conclude "no impl provides this member" for the overwhelmingly common plain member. Collect all interface-declared method names (own package + dependency closure) once per package into PackageLoweringData and answer that case with a single hash lookup. The set is package-wide, so any name declared by any reachable interface still takes the full enumeration — a pure fast path. baml_type::normalize::is_subtype_of ran the co-inductive assumption bookkeeping (deep clone + full-tree hash of the (lhs, rhs) pair) on every recursive step. Only the expanding arms (mu-unfolding, type-variable / associated-projection bound lookup) can revisit a pair; purely structural arms descend into strictly smaller subterms, so the bookkeeping is now restricted to the expanding arms via an is_subtype_of_inner split (termination argument documented at the split). Re-derivation of items 12 and 7 from the cold-compile audit (#4016) against post-#4032 canary, where equivalence goes exclusively through baml_type::normalize. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes optimize concrete interface dispatch by precomputing reachable method names and restructure equirecursive subtype cycle tracking around expansion-aware comparisons. ChangesMIR dispatch prefilter
Subtype cycle handling
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Binary size checks passed✅ 7 passed
Generated by |
|
Folded into the combined re-landing PR #4054 (per maintainer preference for a single PR post-tool-merge). Branch kept for provenance; individual before/after measurements remain in this PR's description. |
1 participant
Re-lands two slices of the cold-compile performance audit (items 12 and 7 of PR #4016), re-derived against current canary. Both are pure fast paths: dispatch results and subtype verdicts are unchanged.
What was being recomputed
1. MIR dispatch pre-filter (
baml_compiler2_mir/src/lower.rs)dispatch_target_for_concreteenumerated every impl block in the package closure (l1_impls_for_recv, which probes each impl's pattern against the receiver, invoking alias normalization / subtype checks per probe) for every method call and field access — almost always just to conclude "no impl provides this member". Since #4032 deleted the TIR structural algebra, each of those probes goes throughAliasEquivCtx/baml_type::normalize, making the per-call enumeration even more expensive.Fix: collect every interface-declared method name (required + default, own package + dependency closure) once per package into the existing Salsa-tracked
PackageLoweringData, and havedispatch_target_for_concretereturnNonein one hash lookup when the member name is absent. The set is package-wide, not per receiver type, so any name declared by any reachable interface still takes the full enumeration — the filter cannot change dispatch results.2.
is_subtype_ofassumption fast path (baml_type/src/normalize.rs)The co-inductive assumption bookkeeping (deep
NormalTyclone + full-tree hash of the(lhs, rhs)pair, insert + remove) ran on every recursive step of the subtype check. Canary already had the reflexivity short-circuit, so only the bookkeeping split was needed.Fix: split the structural rules into
is_subtype_of_inner; the outer function performs the pair bookkeeping only for the expanding arms —Mu,TypeVar,AssociatedTypeProjectionon the left, orMuon the right.Termination argument (also documented as a comment at the split): assumption tracking exists solely to terminate cycles, and a cycle can only regress through arms that expand a type — μ-unfolding (substitution can regenerate the same pair) and variable/projection bound lookup (a bound can mention the variable). Purely structural arms descend into strictly smaller subterms of finite trees, so no cycle can form through them; any infinite path must pass through an expanding arm infinitely often, and those still record assumptions (drawn from the finite subterm closure of the regular operands).
Measurements
Profiler from PR #4038 (
tools_compile_profile, not part of this PR),crates/baml_tests/baml_src(77 files, 25k lines),BAML_NO_BYTECODE_CACHE=1, fresh Salsa db per run,--repeat 5, Apple Silicon:Emit (where MIR lowering and dispatch run) drops ~28%; total cold compile ~18%.
Testing
cargo test --workspace: all compiler/runtime suites pass, including the 442-testbaml_testssuite — no snapshot diffs, so dispatch results and subtype verdicts are byte-identical.sdk_test_typescript_node(missingnode_modulesin the test sandbox —tsc/vitest/attwnot installed) and 4sdk_test_python_pydantic2cancellation tests (fixture usesExceptionGroup, undefined on the sandbox's Python 3.10, plus asyncio-timing asserts).-D warnings) pass.Provenance: re-derivation of c1466f3 (PR #4016) items 12 and 7.
Made with Cursor
Summary by CodeRabbit