perf(compiler2+cli): re-land #4016 cold-compile optimizations (2.4s → 0.81s cold) - #4058
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… / callee_generics_for_func as tracked queries Three package-invariant computations were being redone inside hot per-scope/per-call query bodies (re-landed from PR #4016 against current canary, post-#4032): - package_resolved_aliases: the package alias map (a full clone of every alias Ty plus a project walk) was rebuilt inside every infer_scope_types execution (~15.6k executions on the baml_tests corpus), plus per callable_throws / validate_impl_signatures / projection-resolution call. Now a salsa-tracked query keyed by PackageId. It sits in a legitimate cycle (an alias whose RHS contains an associated-type projection resolves through inference, which reads the alias map), handled via cycle_initial seeding an EMPTY alias environment, mirroring infer_scope_types' cycle seed. Returns a bare HashMap<QualifiedTypeName, Ty>: canary's equivalence algebra (baml_type::normalize via GlobalTypeContext/AliasEquivCtx) consumes only the alias map, and #4032 already removed the recursive-alias set that baml_type::ResolvedAliases carries, so that type does not fit here. TypeInferenceBuilder now borrows the cached map instead of owning a per-scope clone. - package_impl_locs: existed as a plain function walking every file in the project per call, called from every impl-resolution loop, coherence checking and type-expr lowering. Now #[salsa::tracked(returns(ref))] per PackageId. - callee_generics_for_func: every call site re-derived the callee's declared generic params (item-tree scan for the enclosing class) and re-lowered its bound type exprs. Now a tracked query per FunctionLoc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-inferring (fixes duplicate lambda diagnostics) Every lambda body was type-inferred twice: once inline while inferring the enclosing Function/Let scope (needed to type the lambda expression itself), then again from scratch by the standalone `ScopeKind::Lambda` `infer_scope_types` query (needed by MIR/LSP for the lambda scope's own tables). The second pass was ~pure duplicate work AND — because both passes emit diagnostics — reported every diagnostic inside a lambda body twice (a user-visible bug). `infer_lambda_body` now captures the lambda's complete inference tables into a `NestedLambdaInference` struct (MOVED, not cloned, via `mem::replace` while restoring parent state), keyed by the lambda's `FileScopeId`. The key is resolved from `func_def.span`, with a fallback to the body's root-expression span so synthetic desugared `test`/`testset` bodies (whose `FunctionDef::span` is default) still resolve to their HIR Lambda scope. These entries are not restored across `infer_lambda_body`, so lambdas at every nesting depth bubble up to the owning Function/Let scope. The `Lambda` arm of `infer_scope_types` then walks to its owning Function/Let scope, calls `infer_scope_types` on it (populating the capture), and PROJECTS the captured tables into its `ScopeInference` instead of re-inferring the body. The projected result carries no `extra`/diagnostics, so a lambda's diagnostics now come only from the owner scope's inference and are reported exactly once. Misses (synthetic template bodies with no backing `Expr::Lambda`, or the empty owner inference produced during a Salsa cycle iteration) fall through to the original standalone inference. Verified de-duplication on the affected snapshots (regenerated centrally): the type-mismatch diagnostic inside a lambda body now appears once (owner scope) instead of twice, and the synthetic-test-body case dedups via the root-expr-span fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s_subtype_of assumption fast path - MIR dispatch_target_for_concrete: gate the per-call impl enumeration behind a package-wide FxHashSet of interface-declared method names (own package + dependency closure), so a plain method / field access returns None in one hash lookup instead of probing every impl pattern via the equivalence algebra. - baml_type::normalize: reflexivity + heads_definitely_differ fast-reject in equivalent() (conservative: same-kind nominal pairs only, since List/EvolvingList and Map/EvolvingMap collapse to the same canonical head post-#4032); and restrict is_subtype_of co-inductive assumption bookkeeping to the expanding arms (Mu / TypeVar / AssociatedTypeProjection left, Mu right) via is_subtype_of_inner, with a termination argument in-comment. Re-land of PR #4016 (audit #12, #7, #8), re-derived against the post-#4032 baml_type algebra. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… per diagnostic batch Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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):
|
📝 WalkthroughWalkthroughThis change adds memoized compiler queries and shared lowering data, caches generic and lambda inference, centralizes resolved aliases, prefilters interface dispatch, adjusts type normalization termination, reuses diagnostic source caches, updates profiling categories, and configures ChangesCompiler optimization and caching
Estimated code review effort: 5 (Critical) | ~120 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 |
Binary size checks passed✅ 7 passed
Generated by |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler_diagnostics/src/render.rs (1)
233-251: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReturn early for empty diagnostics.
render_diagnosticscurrently builds the AriadneSourceCacheeven when there’s nothing to emit;join("\n")already yields an empty string, so this keeps output unchanged while skipping the clone/index work on the success path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_compiler_diagnostics/src/render.rs` around lines 233 - 251, Update render_diagnostics to return an empty string before constructing ariadne_cache when diagnostics is empty. Preserve the existing join("\n") output for non-empty diagnostics and keep the Ariadne and concise rendering paths unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@baml_language/crates/baml_compiler_diagnostics/src/render.rs`:
- Around line 233-251: Update render_diagnostics to return an empty string
before constructing ariadne_cache when diagnostics is empty. Preserve the
existing join("\n") output for non-empty diagnostics and keep the Ariadne and
concise rendering paths unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a1bd8e4d-1a05-4e88-82ff-a34fd1a6a890
⛔ Files ignored due to path filters (4)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/patterns_class_destructure/baml_tests__diagnostic_errors__patterns_class_destructure__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_param_default_forward_reference_checks_lambda_bodies.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
baml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/main.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_hir/src/lib.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_ppir/src/lib.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/builder/associated_projection.rsbaml_language/crates/baml_compiler2_tir/src/builder/interface_resolution.rsbaml_language/crates/baml_compiler2_tir/src/callable.rsbaml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/interfaces/coherence.rsbaml_language/crates/baml_compiler2_tir/src/interfaces/impl_rules.rsbaml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsbaml_language/crates/baml_compiler_diagnostics/src/render.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_type/src/normalize.rs
…ect-wide tag map once, not per lowered function build_class_type_tags walked every file's item tree and re-rendered + re-hashed every class's fully-qualified name inside LoweringContext construction — i.e. once per lowered function/let (~420x on the test corpus; it was the single hottest MIR frame in a CPU sample, plus most of the core::fmt time). Tags are content-addressed, so the map is a pure function of the project's classes: it is now a #[salsa::tracked] query keyed on the Project input, and LoweringContext borrows it like the rest of the package-invariant data. Re-land of #4016 audit item #4 (class_type_tags_for_project), which was initially assumed superseded by #3924's content-addressed tags — #3924 changed the tag values but left the per-function rebuild in place. Also adds the new tracked queries from this PR chain to the profiler's phase_for_query table (per its README convention) so they stop landing in the "other" bucket. Cold check+emit on the corpus: 0.99s -> ~0.85s (medians of 5, disk cache disabled); emit 0.64s -> 0.33s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… pre-size emit buffers - Match checking ran the full usefulness matrix twice per match: once for exhaustiveness over all guard-less arms, then again for unreachable-arm detection over the error-free subset. When no arm had a pattern error (the common case) the two matrices are identical, so the reachability pass now reuses the exhaustiveness report instead of recomputing it. The matrix walk was the hottest attributable check-side subsystem in a CPU sample (exhaustiveness ~11% inclusive). - StackifyCodegen: pre-size bytecode.instructions/meta from the MIR statement count and the local/block maps from the MIR's shape, instead of growing everything from empty per function. Cold check+emit on the corpus: 0.830s -> 0.808s (medians of 5, disk cache disabled); check 0.494s -> 0.462s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…7s cold, byte-identical) (BoundaryML#4073) Makes BAML compilation parallel by default — no flags, no env gates. \`RAYON_NUM_THREADS=1\` gives the serial reference path. Plus three single-threaded fixes for redundant work found by profiling. ## Commits 1. **parallel check + parallel emit codegen** — per-file \`check_file\` and per-function stackification on rayon's global pool, cloned shared-storage salsa handles (rust-analyzer model). Emit workers compile into watermark-based fragment pools; the serial merge replays cross-function \`GenericFunction\` interning in original order, reproducing the serial pool layout exactly. 2. **scope-inference dedup** — \`check_file\` drove diagnostics with HIR-minted ScopeIds while TIR/MIR key by PPIR's expanded-index ScopeIds, so every scope in a \`$stream\`-expanded file was inferred twice and re-inferred during emit. Now keyed by PPIR ScopeIds (HIR-prefix gated): \`infer_scope_types\` 13,331 → 8,719 executions. 3. **function_body discriminant** — step 6 deep-cloned every function's ExprBody arena just to test its kind; reads the item tree instead (~2,256 clones gone). 4. **\`is_subtype\` fast paths** — reflexivity + narrow head-mismatch reject (interface/interface pairs excluded — cross-name interface subtyping via \`requires\` is real). 5. **parallel MIR lowering** — the emit \`Db\` trait gains a defaulted \`parallel_db_handle()\`; \`ProjectDatabase\` mints cloned handles so Stage A's \`lower_function\` fans out too. ## Byte-identity Permanent test \`parallel_emit_is_byte_identical_to_serial\`: full corpus compiled on a 1-thread pool vs the default pool, identical serialized bytes (covers the \`ns_instantiation_expr\` cross-function interning fixtures). Parallel check verified byte-identical diagnostics over two corpora. All suites green: 444/444 lsp2, 1,658/1,658 tir/diagnostics/compiles/emit/mir/instantiation. ## Numbers (cold, disk cache off, medians of 5) | corpus | serial before | this PR | |---|---|---| | baml_tests (77 files, 25k lines) | 0.63 s | **0.265 s** (check 0.18, emit 0.08) | | agent-tries-baml (53 files, 13k lines, check-only) | 0.21 s | **0.126 s** | With BoundaryML#4058 merged earlier this week: cold compile 2.39 s → **0.27 s** (~9×). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Performance** - Improved compilation and diagnostics performance by parallelizing bytecode emission and per-file diagnostic collection for larger projects. - **Bug Fixes** - Fixed class-field metadata and pooled object index handling to ensure correct field/type resolution during compilation. - Improved determinism guarantees so serial and parallel output remains byte-identical. - Corrected scope diagnostics to use canonical scope IDs and streamlined function signature checks. - Improved subtype compatibility checks with early fast-path decisions. - **Tests** - Added determinism coverage comparing serial vs parallel emit results. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Re-lands the still-orthogonal cold-compile optimizations from #4016, re-derived from
scratch against current
canary(which moved underneath #4016 via #4032 and #3924).Not a rebase of #4016 — every change was re-derived and re-measured. #4016's single
biggest win (recursive-alias hoist) is intentionally not here: #4032 already captured
it by deleting the old TIR
StructuralTyalgebra. The profiler itself already landedseparately as #4038.
Measurement
Corpus:
crates/baml_tests/baml_src(77 files, 25,212 lines). Protocol:tools_compile_profile ... --repeat 5, disk cache disabled (BAML_NO_BYTECODE_CACHE=1,fresh
BAML_CACHE_DIR). Cold-cache medians of 5 runs, single-threaded.8c29c827e)3.0x faster cold check+emit, single-threaded.
(For reference, the pre-#4032 baseline this work originally started from was ~16 s; #4032
alone brought cold compile to a few seconds, and this branch takes it under ~1 s.)
Key query-count deltas (cold, corpus):
infer_scope_types15,590 → 13,331 (PPIR→HIRfile_semantic_indexdelegation removes duplicate scope inference);package_resolved_aliases/
package_impl_locsno longer rebuilt inside every one of those inference calls (now ahandful of per-package executions); new memoized queries
file_ast(131, once/file),callee_generics_for_func(1,834), tracked PPIRfunction_body.What's in it (one commit per track)
file_asttracked query — lower CST→AST once per file (items + lowering diagnostics +env refs), shared by both
file_semantic_indexqueries,ppir_expansion_items, theproject-wide expansion collectors, and the LSP check path; PPIR
file_semantic_indexdelegates to HIR's when a file has no
$streamexpansions; PPIRfunction_bodytracked.package_resolved_aliases(+cycle_initialseeding anempty env, mirroring
infer_scope_types— it sits in a real salsa cycle viaassociated-type-projection alias RHS) and
package_impl_locsas tracked queries, pluscallee_generics_for_func, so the alias map / impl-block list / callee generics stopbeing rebuilt per inference call.
owner scope, then again by the standalone
ScopeKind::Lambdaquery), which also emittedduplicate diagnostics inside lambdas. The inline pass now captures the lambda's tables and
the Lambda arm projects them; synthetic desugared
test/testsetbodies fall through tostandalone inference so their diagnostics are still emitted. Snapshot updates where the
duplicate lambda diagnostics disappear are the point.
dispatch_target_for_concretegates itsper-call impl enumeration behind a package-wide
FxHashSetof interface-declared methodnames (own package + dependency closure);
baml_type::normalizegets a reflexivity +heads_definitely_differfast-reject inequivalent()(conservative: same-kind nominalpairs only — List/EvolvingList collapse to the same canonical head post-Delete old TIR type algebra #4032) and
restricts
is_subtype_ofco-inductive assumption bookkeeping to the expanding arms(Mu / TypeVar / AssociatedTypeProjection) via
is_subtype_of_inner, with a terminationargument in-comment. Re-derived onto the post-Delete old TIR type algebra #4032
baml_typealgebra (the onlyequivalence path now).
class_type_tags_for_project— the project-wide class → type-tag map wasrebuilt (every file's item tree walked, every class name re-rendered and re-hashed)
inside every
LoweringContextconstruction, i.e. once per lowered function (~420x onthe corpus; the hottest MIR frame in a CPU sample). Now a
#[salsa::tracked]querykeyed on the
Projectinput;LoweringContextborrows it. This is perf(compiler2): ~32x faster cold compile — remove redundant work, memoize package-invariant queries #4016 audit item [BUMP:cli:0.3.0-canary.0] [BUMP:py_client:1.2.0+canary.0] [BUMP:vscode_ext:0.4.0-canary.0] #4,initially assumed superseded by Incremental compilation: content-addressed bytecode caching with per-file recompilation #3924's content-addressed tags — Incremental compilation: content-addressed bytecode caching with per-file recompilation #3924 changed the tag
values but left the per-function rebuild in place. Also adds this PR chain's new
tracked queries to the profiler's
phase_for_querytable.usefulness matrix twice per
match(exhaustiveness, then an identical second pass forunreachable-arm detection whenever no arm had a pattern error); the reachability pass now
reuses the exhaustiveness report (exhaustiveness: ~11% -> ~1.7% of CPU inclusive).
StackifyCodegenpre-sizes its bytecode/meta buffers and local/block maps from the MIR'sshape instead of growing from empty per function.
baml_cli's global allocator; buildthe ariadne
SourceCacheonce per diagnostic batch instead of once per diagnostic. Verifiedbyte-for-byte identical rendered diagnostics and clean
BAML_CACHE_VERIFY=1(so Incremental compilation: content-addressed bytecode caching with per-file recompilation #3924'scached-diagnostic replay does not diverge).
Deliberately not re-landed
Testing
Full workspace test suite green except two pre-existing/environmental failures unrelated to
this change: a Python cancellation pytest that fails identically on clean
canary(localPython < 3.11:
ExceptionGroup/CancelledError.reason).cargo fmt+clippy -D warningsclean.
Provenance: #4016 (reference implementation, kept as reference, not merged).
🤖 Generated with Claude Code