chore(tools): add tools_compile_profile — salsa/pipeline profiling harness - #4038
Conversation
…rness Standalone black-box profiler for the compiler pipeline (parse → HIR → PPIR → TIR → MIR → emit): wall-clock per phase, per-query execution and cache-hit counts via salsa::Event callbacks, cold/warm run comparison, JSON output. Ported from PR #4016 (perf/compiler2-cold-compile); the PROF_* TIR audit counters that only existed on that branch are dropped, and the audit README section is annotated as a historical record predating #4032/#3924. 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.
|
|
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 (1)
📝 WalkthroughWalkthroughAdds a standalone Rust profiling harness for the BAML compiler. It measures cold and warm pipeline timings, Salsa query activity, diagnostics, and phase distribution, with human-readable, JSON, and summary-line outputs plus usage documentation. ChangesCompiler profiling harness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as tools_compile_profile CLI
participant Sources as BAML source loader
participant DB as ProjectDatabase
participant Events as Salsa EventCallback
participant Report as Report formatter
CLI->>Sources: discover and read .baml sources
CLI->>DB: build database and inputs
DB->>Events: emit query and cycle events
CLI->>DB: run check and optional get_bytecode
Events-->>CLI: return query counters
CLI->>Report: format timing and query statistics
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
baml_language/crates/tools_compile_profile/src/main.rs (1)
524-643: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for
resolve_query_namesandphase_for_query.Both are pure, logic-bearing functions (collision-suffix disambiguation, phase classification) with no test coverage. As per coding guidelines, Rust changes should prefer unit tests over integration tests where possible — a small
#[cfg(test)] mod testsin this file covering a couple of known query names (phase mapping + fallback to"other") and a synthetic name collision would guard this logic cheaply as the query list evolves.Based on coding guidelines: "Prefer writing Rust unit tests over integration tests where possible" (
**/*.rs).🤖 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/tools_compile_profile/src/main.rs` around lines 524 - 643, Add a #[cfg(test)] module in the same Rust file covering phase_for_query with known lexer/compiler query mappings and the "other" fallback, plus resolve_query_names with synthetic colliding ingredient names and distinct indexed suffixes. Use focused unit-test fixtures for ProjectDatabase and RawStats, preserving the existing sorting and disambiguation behavior.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.
Inline comments:
In `@baml_language/crates/tools_compile_profile/README.md`:
- Line 133: Update the sample-output code fence in the README to use the text
language tag (```text) instead of an untyped fence, preserving the sample
content unchanged.
- Around line 84-85: Update the usage documentation for tools_compile_profile to
state that the CLI accepts directories containing .baml files, a baml_src/
directory, or baml.toml project roots, while preserving the existing invocation
example.
- Around line 47-64: The README’s cache-mode description incorrectly labels
every measured run as cold and warm invocations as cache-exclusive. Update the
“Cache mode: cold by default” section to state that only the first invocation
for each fresh ProjectDatabase is cold, while --warm-runs reuses Salsa
memoization but still performs uncached wrapper work; revise the warm-run
explanation consistently and preserve the existing database lifecycle details.
- Around line 73-74: Update the benchmark statements in README.md around the
emit and related benchmark sections to label the 13.6s/12.1s and
16.0s/8.1s/13.2s figures as historical runs, or replace them with current
measurements consistent with the documented ~0.5s compile time. Ensure readers
can clearly distinguish historical audit data from present-day performance.
In `@baml_language/crates/tools_compile_profile/src/main.rs`:
- Around line 470-497: Update the emit gate around error_count and
get_bytecode() to consider only user-file error diagnostics, matching
get_bytecode()’s existing behavior rather than counting compiler2 builtin
errors. Reuse the same diagnostic filter or delegate the decision to
get_bytecode(), while preserving check-only handling and warning counting.
---
Nitpick comments:
In `@baml_language/crates/tools_compile_profile/src/main.rs`:
- Around line 524-643: Add a #[cfg(test)] module in the same Rust file covering
phase_for_query with known lexer/compiler query mappings and the "other"
fallback, plus resolve_query_names with synthetic colliding ingredient names and
distinct indexed suffixes. Use focused unit-test fixtures for ProjectDatabase
and RawStats, preserving the existing sorting and disambiguation behavior.
🪄 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: 92a2a54e-4032-4aa2-ab50-4df03e7ebedc
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
baml_language/Cargo.tomlbaml_language/crates/tools_compile_profile/Cargo.tomlbaml_language/crates/tools_compile_profile/README.mdbaml_language/crates/tools_compile_profile/src/main.rsbaml_language/stow.toml
…g, unit tests Addresses CodeRabbit review feedback on the tools_compile_profile PR: - Emit gate now mirrors get_bytecode(): only user-file check errors abort emit. db.check() also covers compiler2 builtin stub files whose diagnostics never block codegen, so the previous 'any error' gate could skip emit even when bytecode generation would have proceeded. Total error/warning counts are still reported unfiltered. - Clarify cold-vs-warm measurement semantics in the README and module doc: only the first (cold) invocation fills the cache; warm invocations hit a warm query cache but still pay uncached wrapper cost (db.check() / db.get_bytecode() materialization). - README: document project-root (baml_src/, alongside baml.toml) input form; add a 'text' language tag to the sample-output fence (markdownlint MD040). - Add unit tests for phase_for_query (known phase mappings, 'other' fallback, and collision-suffix stripping). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed the review feedback in 6dda35b:
On the suggestion to also unit-test Verified locally: @coderabbitai review |
|
Kicking off a fresh review of the updated code now. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/tools_compile_profile/README.md (1)
255-260: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReconcile the on-disk cache claims.
Lines 184-192 state that upstream change
#3924added content-addressed on-disk incremental caching, but this section says the compiler has no on-disk cache and everybaml checkis cold. Clarify that the profiler intentionally uses a fresh in-memory database and does not exercise the compiler’s on-disk cache, or update the historical wording.🤖 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/tools_compile_profile/README.md` around lines 255 - 260, Reconcile the historical caching statement around the profiler description with the “What's deliberately NOT here” section: clarify that the profiler uses a fresh in-memory Salsa database and therefore does not exercise the compiler’s content-addressed on-disk cache, while preserving the distinction between profiler behavior and compiler capabilities.
🤖 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.
Outside diff comments:
In `@baml_language/crates/tools_compile_profile/README.md`:
- Around line 255-260: Reconcile the historical caching statement around the
profiler description with the “What's deliberately NOT here” section: clarify
that the profiler uses a fresh in-memory Salsa database and therefore does not
exercise the compiler’s content-addressed on-disk cache, while preserving the
distinction between profiler behavior and compiler capabilities.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 65bad91b-a413-4489-affd-fba869019b22
📒 Files selected for processing (2)
baml_language/crates/tools_compile_profile/README.mdbaml_language/crates/tools_compile_profile/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/crates/tools_compile_profile/src/main.rs
…ns (2.4s → 0.81s cold) (BoundaryML#4058) Re-lands the still-orthogonal cold-compile optimizations from BoundaryML#4016, re-derived from scratch against current `canary` (which moved underneath BoundaryML#4016 via BoundaryML#4032 and BoundaryML#3924). Not a rebase of BoundaryML#4016 — every change was re-derived and re-measured. BoundaryML#4016's single biggest win (recursive-alias hoist) is intentionally **not** here: BoundaryML#4032 already captured it by deleting the old TIR `StructuralTy` algebra. The profiler itself already landed separately as BoundaryML#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. | | check | emit | **total** | |---|---|---|---| | canary (`8c29c827e`) | 1.114 s | 1.277 s | **2.392 s** (min 2.354 / max 2.537) | | this branch | 0.462 s | 0.345 s | **0.808 s** (min 0.791 / max 0.828) | **3.0x faster** cold check+emit, single-threaded. (For reference, the pre-BoundaryML#4032 baseline this work originally started from was ~16 s; BoundaryML#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_types` 15,590 → **13,331** (PPIR→HIR `file_semantic_index` delegation removes duplicate scope inference); `package_resolved_aliases` / `package_impl_locs` no longer rebuilt inside every one of those inference calls (now a handful of per-package executions); new memoized queries `file_ast` (131, once/file), `callee_generics_for_func` (1,834), tracked PPIR `function_body`. ## What's in it (one commit per track) - **`file_ast` tracked query** — lower CST→AST once per file (items + lowering diagnostics + env refs), shared by both `file_semantic_index` queries, `ppir_expansion_items`, the project-wide expansion collectors, and the LSP check path; PPIR `file_semantic_index` delegates to HIR's when a file has no `$stream` expansions; PPIR `function_body` tracked. - **package-level TIR queries** — `package_resolved_aliases` (+ `cycle_initial` seeding an empty env, mirroring `infer_scope_types` — it sits in a real salsa cycle via associated-type-projection alias RHS) and `package_impl_locs` as tracked queries, plus `callee_generics_for_func`, so the alias map / impl-block list / callee generics stop being rebuilt per inference call. - **nested-lambda inference projection** — lambda bodies were inferred twice (inline in the owner scope, then again by the standalone `ScopeKind::Lambda` query), which also emitted duplicate diagnostics inside lambdas. The inline pass now captures the lambda's tables and the Lambda arm projects them; synthetic desugared `test`/`testset` bodies fall through to standalone inference so their diagnostics are still emitted. Snapshot updates where the duplicate lambda diagnostics disappear are the point. - **MIR dispatch prefilter + subtype fast paths** — `dispatch_target_for_concrete` gates its per-call impl enumeration behind a package-wide `FxHashSet` of interface-declared method names (own package + dependency closure); `baml_type::normalize` gets a reflexivity + `heads_definitely_differ` fast-reject in `equivalent()` (conservative: same-kind nominal pairs only — List/EvolvingList collapse to the same canonical head post-BoundaryML#4032) and restricts `is_subtype_of` co-inductive assumption bookkeeping to the expanding arms (Mu / TypeVar / AssociatedTypeProjection) via `is_subtype_of_inner`, with a termination argument in-comment. Re-derived onto the post-BoundaryML#4032 `baml_type` algebra (the only equivalence path now). - **memoized `class_type_tags_for_project`** — the project-wide class → type-tag map was rebuilt (every file's item tree walked, every class name re-rendered and re-hashed) inside every `LoweringContext` construction, i.e. once per lowered function (~420x on the corpus; the hottest MIR frame in a CPU sample). Now a `#[salsa::tracked]` query keyed on the `Project` input; `LoweringContext` borrows it. This is BoundaryML#4016 audit item BoundaryML#4, initially assumed superseded by BoundaryML#3924's content-addressed tags — BoundaryML#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_query` table. - **match usefulness report reuse + emit buffer pre-sizing** — match checking ran the full usefulness matrix twice per `match` (exhaustiveness, then an identical second pass for unreachable-arm detection whenever no arm had a pattern error); the reachability pass now reuses the exhaustiveness report (exhaustiveness: ~11% -> ~1.7% of CPU inclusive). `StackifyCodegen` pre-sizes its bytecode/meta buffers and local/block maps from the MIR's shape instead of growing from empty per function. - **CLI mimalloc + diagnostic rendering** — mimalloc as `baml_cli`'s global allocator; build the ariadne `SourceCache` once per diagnostic batch instead of once per diagnostic. Verified byte-for-byte identical rendered diagnostics and clean `BAML_CACHE_VERIFY=1` (so BoundaryML#3924's cached-diagnostic replay does not diverge). ## Deliberately not re-landed - The BoundaryML#4016 recursive-alias hoist — superseded by BoundaryML#4032. - Skip-builtin-diagnostics — superseded by BoundaryML#3924's per-toolchain builtin-diagnostics cache. ## 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` (local Python < 3.11: `ExceptionGroup`/`CancelledError.reason`). `cargo fmt` + `clippy -D warnings` clean. Provenance: BoundaryML#4016 (reference implementation, kept as reference, not merged). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
baml_language/crates/tools_compile_profile, a standalone black-box profiling harness for the compiler pipeline (parse → HIR → PPIR → TIR → MIR → emit). It reports wall-clock per phase, per-Salsa-query execution / cache-hit / blocked counts, cold-run variance, cold-vs-warm comparison (--warm-runs), and JSON output.perf/compiler2-cold-compile), with two adaptations: thePROF_*TIR audit counters (which only existed on that branch) are removed rather than ported, and the "July 2026 cold-compile audit" README section is annotated as a historical record predating Delete old TIR type algebra #4032 / Incremental compilation: content-addressed bytecode caching with per-file recompilation #3924.mimalloc = "0.1.52"added to workspace deps (used only by this tool, as its global allocator — noted in the README), and"compile"added to thetoolsnamespaceapproved_prefixesinstow.toml.Provenance: #4016. The remaining findings from that audit are being re-landed as small independent PRs measured with this tool.
Baseline on current canary (2660b8b)
Protocol:
cargo build --release -p tools_compile_profile, then./target/release/tools_compile_profile baml_language/crates/baml_tests/baml_src --repeat 5with
BAML_NO_BYTECODE_CACHE=1andBAML_CACHE_DIRpointed at a fresh temp dir (so #3924's disk cache is out of the picture). Corpus: 77 files, 25,212 lines. Apple Silicon, single-threaded, medians of 5 cold runs.For comparison, #4016's pre-optimization baseline on this corpus was 16.0s and its final state ~0.50s. Canary's own movement (#4032 deleting the old TIR type algebra, #3924's caching work) already brought cold compile from 16.0s to ~2.4s; the still-orthogonal optimizations from #4016 target the remainder.
Top 10 queries by executions (cold, representative run)
infer_scope_typesfunction_in_scope_generic_param_boundsfunction_bodycallable_throwsclass_generic_param_boundsresolve_class_fieldsfunction_parameter_defaultsfunction_signatureimpl_dataimpl_data_source_map25,972 total query executions; TIR is 18,697 of them (72%).
Test plan
cargo build --release -p tools_compile_profilecleanbaml_language/crates/baml_tests/baml_src(--repeat 5)-D warnings), cargo stow, markdown validationMade with Cursor
Summary by CodeRabbit