Repository navigation
perf(compiler): reuse inference caches - #4461
Conversation
|
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 Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe PR centralizes alias and enum-variant fact resolution and updates HIR inference, flow narrowing, and pattern analysis to use canonical subtype caching. ChangesHIR type resolution and inference
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This performance refactor reuses existing inference caches without an intended behavior change. The reported validation passes, and no actionable merge-blocking risk remains after normal checks and review. 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 |
Post-merge audit — this PR merged before its adversarial review completedVerdict had this been pre-merge: SHIP-WITH-FIXES. Part (a) — threading the canonical cache through F1 (likely perf regression): the memo removal hit F2 (measurement honesty): the PR's bench baseline was NOT rerun in-session (the log says so), and both table rows are regressions (+3.8% / +2.9%) waved off as within-band — Minor: F4 — make Fast-follow PR dispatched. |
## Summary - restore alias and enum memoization only in the project-scale candidate scans in impls_for_type_cached and resolve_within_depth - keep the memo-free AliasOnlyFacts constructor at Facts::project, infer/obligations.rs, method_resolution.rs, and interface_requires_inner - make provable_subtype an InferenceContext method so the body facts and canonical cache are structurally paired at every call - correct the one-shot context documentation and document the Salsa re-entrancy-window caveat for scan-local memos This is the focused correction for the BoundaryML#4461 post-merge audit. It restores repeated alias-head lookup reuse without widening memoization to the four one-shot sites. ## Fresh back-to-back A/B Both refs were measured consecutively in the same shell and target cache with profiling disabled. Both workloads used exactly five one-compile samples: cargo bench -p baml_tests --bench compiler_benchmark -- compile_ --sample-count 5 --sample-size 1 Environment: Rust 1.93.0, CARGO_BUILD_JOBS=8, BAML_PROFILE=0. | benchmark | canary 07c118b, 5 samples | candidate 0eb5f09, 5 samples | median delta | | --- | ---: | ---: | ---: | | compile_baml_tests_project | 2.180 s (2.144-2.351 s) | 2.133 s (2.122-2.295 s) | -47 ms (-2.2%) | | compile_empty_project | 497.5 ms (485.8-499.9 ms) | 485.6 ms (478.2-488.5 ms) | -11.9 ms (-2.4%) | No earlier baseline values are reused. Canary 07c118b was the exact BoundaryML#4461 tip when this branch and A/B began; BoundaryML#4462 landed afterward and changes only nextest scheduling configuration. ## Validation - cargo check -p baml_compiler2_hir_ty - focused nextest: 384/384 passed for baml_type, baml_compiler2_hir, and baml_compiler2_hir_ty with all features - strict all-target/all-feature Clippy passed for those packages; workspace commit-hook Clippy also passed - cargo fmt --all -- --check and git diff --check - exact pinned full gate: 3,748/3,748 passed, 24 skipped, 50 slow in 1,375.321 s - doctests clean; no unreferenced snapshots and no snapshots to review Full gate command: CARGO_BUILD_JOBS=8 rustup run 1.93.0 cargo insta test --test-runner nextest -p baml_tests -p baml_cli -p baml_lsp2_actions -p baml_lsp2_actions_tests -p baml_surface --all-features --unreferenced=reject <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Improved type inference consistency across assignments, pattern matching, flow narrowing, and catch-arm analysis. * Enhanced handling of aliases and enum variants during type resolution. * Repeated type checks are now more efficient, helping reduce unnecessary compiler work. * Existing subtype, union, coverage, and narrowing behavior remains consistent while benefiting from more centralized analysis. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
This is a pure performance refactor with no intended semantic delta.
Benchmarks
Both deltas are within the established +/-5% run-to-run band. The aggregate result is neutral/noisy, so this PR does not claim an end-to-end speedup; the win is the removal of redundant cache construction and canonical subtype work.
Scope note
The optional #4453 M4 item was skipped because it belongs to separate VM/GC benchmark and retention work rather than this contained compiler follow-up.
Validation
Full gate command: CARGO_BUILD_JOBS=8 rustup run 1.93.0 cargo insta test --test-runner nextest -p baml_tests -p baml_cli -p baml_lsp2_actions -p baml_lsp2_actions_tests -p baml_surface --all-features --unreferenced=reject
Summary by CodeRabbit
Bug Fixes
Performance