perf(compile): parallel check/MIR/emit + inference dedup (0.63s → 0.27s cold, byte-identical) - #4073
Merged
Merged
Conversation
…l output Check: collect_compiler2_diagnostics fans per-file check_file out on rayon's global pool (first file serial to warm package-level queries; each task owns a cloned shared-storage salsa handle — ProjectDatabase is Send but deliberately !Sync, so handles are pre-cloned and moved in; projects of <=8 files stay serial). Output unchanged by construction — diagnostics are sorted at the end; verified byte-identical on two corpora. Emit: Pass 4's MIR lowering (salsa) stays serial; the pure codegen core (lambda + function stackification) runs across rayon workers, each compiling into a watermark-based fragment pool (objects_base threaded through MirCodegenContext; class-field reads served by a precomputed ClassFieldSnapshot instead of live pool reads). The serial merge replays fragments in original function order with per-object index mapping and cross-function GenericFunction interning replay, then rewrites operands via the relink visitors — reproducing the serial pool layout exactly. A 1-thread rayon pool (RAYON_NUM_THREADS=1) takes the serial reference paths; the emit_determinism suite gains a permanent test asserting the parallel output is byte-identical to serial over the full test corpus (including the ns_instantiation_expr cross-function interning fixtures). Cold check+emit on the 25k-line corpus: 0.63s -> 0.41s (check 0.38->0.21, emit 0.27->0.19; medians of 5, disk cache off). The 13k-line agent-tries-baml corpus checks in ~140ms. 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):
|
Contributor
📝 WalkthroughWalkthroughRayon dependencies enable parallel compiler function emission and large-project diagnostics. Compiler code snapshots class fields, tracks absolute object-pool offsets, merges worker fragments, and validates serial/parallel byte identity. Diagnostic scope handling and subtype checks also gain targeted fast paths. ChangesParallel compiler emission
Parallel project diagnostics
Scope and subtype checks
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CompilerPass4
participant RayonWorkers
participant WorkerPools
participant ProgramObjectPool
CompilerPass4->>RayonWorkers: compile function bodies
RayonWorkers->>WorkerPools: emit bytecode and pooled objects
WorkerPools->>ProgramObjectPool: merge fragments with absolute indices
ProgramObjectPool-->>CompilerPass4: register compiled functions
sequenceDiagram
participant ProjectDatabase
participant RayonTasks
participant FileChecker
participant DiagnosticsCollector
ProjectDatabase->>RayonTasks: clone database per chunk
RayonTasks->>FileChecker: check files in parallel
FileChecker->>DiagnosticsCollector: send chunk diagnostics
DiagnosticsCollector-->>ProjectDatabase: append package diagnostics and sort
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@baml_language/crates/baml_compiler2_emit/src/lib.rs`:
- Around line 4194-4195: Update the documentation comment for Pass 4 near
emit_functions_serial to remove the obsolete BAML_PARALLEL_EMIT=1 requirement,
and describe parallel emission as being selected automatically based on the
Rayon thread count.
In `@baml_language/crates/baml_project/src/check.rs`:
- Line 76: Update sort_diagnostics to define a total deterministic ordering by
adding tie-breakers after file, start, and message, including available range
end, diagnostic ID, severity, phase, annotations, and related information. Add a
unit test using diagnostics with identical primary sort keys to verify
consistent ordering, then run cargo test --lib.
In `@baml_language/crates/baml_tests/tests/emit_determinism.rs`:
- Around line 102-107: Update the parallel case in the determinism test to
execute compile_to_bytes within an explicit multi-thread Rayon thread pool,
using the pool’s install method. Keep the existing one-thread pool for the
serial case and ensure the parallel pool has more than one worker so
emit_functions_parallel is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: feff115d-6525-466e-a55a-45966a0c17b2
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
baml_language/Cargo.tomlbaml_language/crates/baml_compiler2_emit/Cargo.tomlbaml_language/crates/baml_compiler2_emit/src/emit.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_project/Cargo.tomlbaml_language/crates/baml_project/src/check.rsbaml_language/crates/baml_tests/Cargo.tomlbaml_language/crates/baml_tests/tests/emit_determinism.rs
Binary size checks failed❌ 2 violations · ✅ 5 passed
Details & how to fixViolations:
Add/update baselines:
[artifacts.baml-cli]
file_bytes = 19512048
stripped_bytes = 19512096
gzip_bytes = 9320305
[artifacts.bridge_wasm]
file_bytes = 16160508
gzip_bytes = 4402884Generated by |
check_file drove its per-scope diagnostics loop with HIR-minted salsa ScopeIds while TIR internals and MIR lowering key infer_scope_types by PPIR's expanded-index ScopeIds — so every scope in a $stream-expanded file was type-inferred twice (once per key space; ~4.3k duplicate executions on the test corpus, re-run during emit). The loop now iterates the PPIR index's scope_ids, gated to the HIR prefix so synthetic $stream scopes surface no new diagnostics. The prefix alignment of original scopes in the expanded index is already load-bearing in TIR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Step 6 called hir::body::function_body — which deep-clones the whole ExprBody arena per execution — for every function, only to test the body's discriminant. The item tree in hand already carries it; test func_data.body directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…subtype is_subtype ran two allocating canonicalization walks on every call even when both sides are the identical type. Add the same reflexivity short-circuit equivalent() has, plus a conservative head-mismatch reject that deliberately excludes interface/interface pairs (cross-name interface subtyping via requires is real); the remaining same-kind nominal pairs are provably false in is_subtype_of_inner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stage A of the parallel emit pass still ran lower_function serially — the emit crate only sees &dyn Db, which cannot be cloned for worker threads. The (previously empty) emit::Db marker trait gains a defaulted parallel_db_handle() -> Option<Box<dyn mir::Db + Send>>; ProjectDatabase overrides it with a cloned shared-storage handle. Stage A now lowers the first function serially (memo warm-up) and fans the rest out in chunks on rayon::scope, one pre-cloned handle moved per chunk (the same pattern as parallel check), results reassembled in original order. Serial fallback for small function counts, 1-thread pools, or dbs that mint no handles — the byte-identity reference path is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hellovai
changed the title
… emit test Addresses CodeRabbit review on #4073: - sort_diagnostics only sorted by (file, start, message), leaving distinct diagnostics tied on those in arrival order. Under parallel check that order is nondeterministic, so output could vary between runs. The primary key is unchanged (non-tied output is byte-identical), and exact ties are now broken by span end + a structural Debug encoding of the remaining fields (id/severity/phase/annotations/related_info) — a total order. Adds a unit test that sorts every permutation of primary-key-tied diagnostics to the same sequence. - emit_determinism's parallel case ran in the ambient rayon pool, which is single-threaded on a 1-core runner — silently taking the serial path and testing nothing. Both cases now use explicit pools (1 vs 4 threads). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hellovai
enabled auto-merge
July 17, 2026 07:01
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_project/src/check.rs (1)
218-218: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider using
sort_unstable_by.Since
sort_diagnosticsnow establishes a total order where ties are completely identical, stable sorting is no longer required. Replacingsort_bywithsort_unstable_bywill avoid allocating a temporary buffer and generally improve performance.♻️ Proposed refactor
- diagnostics.sort_by(|a, b| { + diagnostics.sort_unstable_by(|a, b| {🤖 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_project/src/check.rs` at line 218, In sort_diagnostics, replace diagnostics.sort_by with diagnostics.sort_unstable_by while preserving the existing comparator and total ordering behavior.
🤖 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_project/src/check.rs`:
- Line 218: In sort_diagnostics, replace diagnostics.sort_by with
diagnostics.sort_unstable_by while preserving the existing comparator and total
ordering behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fd253f73-1e76-4212-b72b-f41bca452f81
📒 Files selected for processing (2)
baml_language/crates/baml_project/src/check.rsbaml_language/crates/baml_tests/tests/emit_determinism.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/baml_tests/tests/emit_determinism.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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)
With #4058 merged earlier this week: cold compile 2.39 s → 0.27 s (~9×).
🤖 Generated with Claude Code
Summary by CodeRabbit