Repository navigation
Conversation
Four independent fixes to the compile pipeline, found by profiling compile_empty_project (which compiles the full 55-file stdlib): 1. normalize: lhs==rhs fast path in is_same_normalized_type / is_subtype_of, and find_recursive_aliases rewritten as a single memoized tri-color DFS (O(nodes+edges) instead of a fresh DFS per alias). 2. emit Pass 1: read intrinsic-kind off the item tree instead of fully MIR-lowering every function just to inspect one field. Previously every function in the project was lowered twice (Pass 1 + Pass 4). 3. mir: package-shared DispatchCandidateCache memoizing interface_method_candidates_for across all functions lowered in a package. Key includes the function's generic_param_bounds - the only per-function input of the resolution family (verified by transitive self-field audit). 4. mir: LoweringContext no longer deep-copies all nine TIR inference tables into per-function merged maps; it now holds &'db ScopeInference views per FileScopeId and reads through tir_* point accessors. Co-Authored-By: Claude Fable 5 <[email protected]>
|
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.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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🧪 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 |
⏭️ 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):
|
Binary size checks passed✅ 7 passed
Generated by |
The pub struct's doc linked the private LoweringContext::interface_method_candidates_for, which fails `-D rustdoc::private-intra-doc-links` in CI. Plain code formatting instead of an intra-doc link. Co-Authored-By: Claude Fable 5 <[email protected]>
… compile) Type comparisons (is_same_normalized_type, is_subtype_of, and the match_ty_pattern family) took a bare alias HashMap and re-derived the recursive-alias set on every call: a full DFS over the alias graph per type comparison, called from the interface-dispatch and inference inner loops. Profile: find_recursive_aliases was 35% inclusive (168ms) of the 482ms compile, almost all of it under is_same_normalized_type (37%). The comparison entry points now take &ResolvedAliases (the existing baml_type struct carrying the alias map plus its precomputed recursive set), and every environment construction site computes the set exactly once via the new normalize::resolved_aliases_from_map: - TypeInferenceBuilder wraps its map once per scope at construction - MIR lowering passes its existing per-package ResolvedAliases (which always had the set; the old signature just threw it away) - coherence registry build, callable-throws analysis, and the LSP check-pass helper each wrap once at their construction points Also memoizes registry_dispatch_target_for_concrete in the package-shared DispatchCandidateCache keyed (receiver ty, method), caching negative answers too (the "no interface provides this method" scan over every impl rule is the common case and never short-circuits). On the empty-project benchmark this adds only ~1% (its cost was dominated by the per-call DFS the currency change already removed); it bounds the rule scan for projects with more impl rules and call sites. Empty-project cold compile: 482ms -> 281ms median (-42%); cumulative from the 1.41s baseline: 5x. Validated by the tir/mir/emit/lsp2_actions unit suites (337 tests); full integration suite deferred to CI. Co-Authored-By: Claude Fable 5 <[email protected]>
9ff0898 to
4fd0548
Compare
|
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. |
Stacked on #3911. Second PR in the incremental compiler-perf series: after #3911 lands, this one takes the empty-project cold compile from 482ms to 281ms median (−42%). Cumulative from the 1.41s baseline: 5× faster.
The problem
Every type-comparison entry point (
is_same_normalized_type,is_subtype_of, thematch_ty_patternfamily) took a bare aliasHashMapand re-derived the recursive-alias set internally, on every call:These are called from the hottest loops in the compiler: interface-dispatch candidate resolution in MIR lowering, pattern matching in TIR inference, coherence checking. Profile of the 482ms compile:
find_recursive_aliasesat 35% inclusive (~168ms), almost all of it underis_same_normalized_type(37%).The irony: MIR lowering already carried the precomputed set in
ResolvedAliases { aliases, recursive }, the old signatures just couldn't accept it.The change (architectural)
The alias environment's currency is now
&ResolvedAliases(the existingbaml_typestruct: alias map + its recursive set) instead of a bare&HashMap. The recursive-alias analysis is computed exactly once per environment, at construction, via the new sanctioned constructornormalize::resolved_aliases_from_map:TypeInferenceBuilder::newLoweringContextcheck.rsalias helperSignature ripple: 6 params in
interfaces.rs+ 15 incoherence.rs+ 4 inimpl_rules.rs+ 2 incallable.rs+ 6 in MIRlower.rs+ 23 in LSPcheck.rs; ~130 call sites converted. Purely a plumbing change: no comparison logic was touched, andnormalize's internal machinery still works on the raw map (it is the thing that computes the set).Also included: registry dispatch-target memo
registry_dispatch_target_for_concrete(blanket-impl method lookup for concrete receivers) scans every impl rule of every package per call site, and its negative answer ("no interface provides this method") is both the common case and the most expensive (the scan never short-circuits). It is now memoized in the package-sharedDispatchCandidateCachekeyed(receiver type, method), caching misses too.Honest note: on the empty-project benchmark this adds only ~1% (~3ms), because the currency change above already removed the per-call DFS that dominated its cost. It is included because it bounds the rule scan for real projects with more impl rules and call sites, and it reuses the cache infrastructure from #3911 verbatim.
Numbers
Benchmark:
cargo bench --bench compiler_benchmark -- compile_empty_project(100 samples, idle machine).Validation
baml_compiler2_tir/mir/emit/lsp2_actionsunit suites: 337 tests, all greencargo clippy --all-targets --all-features -- -D warnings: cleanbaml_testsintegration suite: deferred to CI (per the incremental-PR plan; Compiler perf: 2.9x faster cold compile (1.41s → 482ms empty project) #3911's four changes passed the full 2,600-test suite locally)Not in this PR (next in series)
package_alias_envsalsa query (kills the per-scope alias-map rebuild ininfer_scope_types)🤖 Generated with Claude Code