perf: linear cold compiles, and one warm cache for check/run/pack#4098
Open
hellovai wants to merge 4 commits into
Open
perf: linear cold compiles, and one warm cache for check/run/pack#4098hellovai wants to merge 4 commits into
hellovai wants to merge 4 commits into
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):
|
Contributor
📝 WalkthroughWalkthroughThe PR adds cache-aware project checking and packing, incremental and package-level diagnostics, unchecked bytecode generation after validation, memoized Jinja type environments, shared database maps, and target-specific SHA-2 assembly configuration. ChangesIncremental compilation pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ProjectDatabase
participant Diagnostics
participant BytecodeCache
CLI->>ProjectDatabase: build project database
ProjectDatabase->>Diagnostics: collect incremental diagnostics
Diagnostics-->>CLI: return merged and fresh diagnostics
CLI->>BytecodeCache: load reuse plan
CLI->>ProjectDatabase: compile dirty artifacts
ProjectDatabase->>BytecodeCache: verify and store artifacts
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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: 1
🧹 Nitpick comments (2)
baml_language/crates/baml_project/src/db.rs (1)
645-660: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a debug-only guard on
get_bytecode_unchecked's precondition.This is
pub(needed cross-crate bybex_project.rs) and can panic in the runtime-conversion boundary if called on an error-bearing project, per its own doc comment. Since the diagnostics query it would re-derive is salsa-memoized, adebug_assert!re-checking the "no user-file errors" invariant would be nearly free when callers already checked (cache hit) and would catch future misuse early.🛡️ Optional debug-only safety net
pub fn get_bytecode_unchecked( &self, ) -> Result<bex_vm_types::Program, baml_compiler2_emit::LoweringError> { + #[cfg(debug_assertions)] + debug_assert!( + !self.get_bytecode().is_err_or(|e| !matches!(e, baml_compiler2_emit::LoweringError::ProjectHasErrors { .. })), + "get_bytecode_unchecked called on a project with unresolved user-file errors" + ); let opts = baml_compiler2_emit::CompileOptions { emit_test_cases: false, }; baml_compiler2_emit::generate_project_bytecode(self, &opts) }🤖 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/db.rs` around lines 645 - 660, Add a debug-only assertion at the start of get_bytecode_unchecked that rechecks the documented no-user-file-errors precondition using the existing memoized diagnostics query or established validation helper. Keep the release behavior and bytecode generation flow unchanged, and make the assertion clearly fail when the method is called on an error-bearing project.baml_language/crates/baml_cli/src/check_command.rs (1)
37-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the shared warm-cache orchestration into one helper.
Both sites re-implement the identical
prepare_warm_db→ incremental-diagnostics →compile_program_artifacts/verify_and_storesequence, and both comment-blocks explicitly claim parity with each other (and withrun_command.rs, not shown here). Any future change to the cache contract (WarmPrep/ReusePlanshape, error handling, sampled verification) has to be hand-applied in every copy, and a missed spot would silently regress that command's warm-cache path without a compiler error to catch it.
baml_language/crates/baml_cli/src/check_command.rs#L37-L147: extract theprepare_warm_db+ incremental-diagnostics-collection + conditional-seed sequence into a shared helper inbytecode_cache.rs(or a new small module) parameterized by the emit-failure policy (log-and-continue for check vs. bail for pack/run).baml_language/crates/baml_cli/src/pack_command.rs#L266-L352: replace the equivalent inline sequence inload_and_compile_projectwith a call to that same shared helper.🤖 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_cli/src/check_command.rs` around lines 37 - 147, Extract the duplicated warm-cache orchestration into a shared helper in bytecode_cache.rs or a small shared module, covering prepare_warm_db, incremental diagnostics collection, conditional compile_program_artifacts, and verify_and_store. Update baml_language/crates/baml_cli/src/check_command.rs lines 37-147 to use the helper with log-and-continue emit failures, and baml_language/crates/baml_cli/src/pack_command.rs lines 266-352 to use it with bail-on-error behavior; preserve each command’s existing diagnostics and cache verification semantics.
🤖 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/Cargo.toml`:
- Around line 294-297: Update the sha2 dependency declaration in Cargo.toml to
remove the asm feature while retaining the existing version and oid feature.
Remove the adjacent comment’s asm/ARMv8 performance claims so it accurately
describes the remaining configuration.
---
Nitpick comments:
In `@baml_language/crates/baml_cli/src/check_command.rs`:
- Around line 37-147: Extract the duplicated warm-cache orchestration into a
shared helper in bytecode_cache.rs or a small shared module, covering
prepare_warm_db, incremental diagnostics collection, conditional
compile_program_artifacts, and verify_and_store. Update
baml_language/crates/baml_cli/src/check_command.rs lines 37-147 to use the
helper with log-and-continue emit failures, and
baml_language/crates/baml_cli/src/pack_command.rs lines 266-352 to use it with
bail-on-error behavior; preserve each command’s existing diagnostics and cache
verification semantics.
In `@baml_language/crates/baml_project/src/db.rs`:
- Around line 645-660: Add a debug-only assertion at the start of
get_bytecode_unchecked that rechecks the documented no-user-file-errors
precondition using the existing memoized diagnostics query or established
validation helper. Keep the release behavior and bytecode generation flow
unchanged, and make the assertion clearly fail when the method is called on an
error-bearing project.
🪄 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: a8771fb6-b2e2-44b4-ab0f-b0e7693a4292
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
baml_language/Cargo.tomlbaml_language/crates/baml_cli/src/check_command.rsbaml_language/crates/baml_cli/src/pack_command.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_project/src/check.rsbaml_language/crates/baml_project/src/db.rsbaml_language/crates/baml_project/src/lib.rsbaml_language/crates/bex_project/src/project.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/mod.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/types.rs
This was referenced Jul 21, 2026
… the cache-path check Three independent O(N^2) / serialization fixes, found by profiling a 5000-module (295k LOC) corpus where cold check burned 126s of CPU: - Memoize the package-wide Jinja type environment as a salsa query (jinja_base_types, keyed by NamespaceId) instead of rebuilding it inside every check_file call, and skip it entirely for files with no LLM prompts or template_strings. The env walk was O(package items) per file, i.e. O(N^2) per full check - and ran twice per compile because get_bytecode's error gate re-checks the project. PredefinedTypes and friends gain PartialEq so salsa can backdate: edits that leave the env unchanged don't invalidate prompt checks. - Arc-wrap ProjectDatabase's file_map / file_id_to_path so cloning a db handle is O(1) instead of O(files). The parallel check and emit drivers mint a handle per work chunk (~1.55N clones per compile), which deep-cloned two N-entry PathBuf maps each time: quadratic coordinator CPU and the dominant term in peak RSS. - Run the narrowed (cache-path) diagnostics collector through the same rayon fan-out as the honest collector. baml check / run route through the narrowed collector, which checked files serially - the parallel machinery next to it was only reachable with the cache disabled. - compile_outcome (LSP) now proves the project error-free once and calls a new get_bytecode_unchecked instead of letting get_bytecode re-run a full project check inside emit. To keep that sound, the diagnostics candidate now includes package-level diagnostics (cross-file name conflicts / namespace shadows) - which previously never reached the editor at all. Measured on the 295k LOC corpus: cold check 21-46s -> 7.1s wall, 125.7s -> 8.9s user CPU, peak RSS 5.3GB -> 3.6GB; scaling is now ~linear in project size (verified to 8000 modules). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
Two gaps kept most workflows off the incremental path: - baml pack never opened a CacheContext: every pack was a fully cold compile, even immediately after a successful run of the same sources. Pack now follows run's exact flow - whole-program hit when nothing changed, per-file unit reuse on a dirty edit, store after a fresh compile. Pack compiles with emit_test_cases:false, so it shares run/check's cache key space; the packaged Program is target-independent (the --target triple only selects host binary bytes) and emit determinism guarantees a reused image is byte-identical to a fresh compile. Standalone --file mode stays cold, same as run. - baml check reused cache entries but never wrote them, so a check-only workflow (editors, CI lint jobs) recompiled everything forever. A clean check now runs the same emit-and-store path as run/test - gated to checks that actually advance the manifest (missing/stale manifest or dirty files), so no-op checks stay emit-free. Seeding is best-effort: an emit failure on a diagnostics-clean project is logged, never surfaced as a check failure. End-user effect (59k LOC project): check 1.0s (cold, seeds) -> check 0.5s -> run 0.10s -> pack 0.13s, all from one warmed cache. Re-pack of a 295k LOC project: 40.5s -> 0.61s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
The warm bytecode-cache path hashes every cached unit payload on each run (~230MB of hashing at a 295k LOC project) and was using sha2's software fallback - the asm feature (SHA-NI / ARMv8 crypto extensions, runtime-detected, target-gated so wasm builds are unaffected) was never enabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
sha2's asm feature does not build under MSVC (sha2-asm ships GAS .S sources cl.exe cannot assemble), so move the feature from the workspace dependency to target-gated tables in bex_cache: every non-MSVC target keeps the hardware backend, MSVC falls back to the portable implementation. Also fix a rustdoc -D warnings failure: the doc comment on collect_package_level_diagnostics linked the private package_level_diagnostics item. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
hellovai
force-pushed
the
hellovai/compiler-perf-fixes
branch
from
July 22, 2026 03:17
b45539e to
7862b2c
Compare
Contributor
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/baml_project/src/db.rs (1)
163-180: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winShare the remaining owned maps too
#[derive(Clone)]still deep-copiescompiler2_file_mapandremoved_file_tombstones, so eachparallel_db_handle()clone can still scale with those maps’ size. Move them behind shared/COW storage if clone cost matters.🤖 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/db.rs` around lines 163 - 180, Wrap compiler2_file_map and removed_file_tombstones in shared copy-on-write storage, matching the existing Arc and Arc::make_mut pattern used by file_map and file_id_to_path. Update their mutation sites and accessors accordingly so parallel_db_handle() clones remain O(1) while preserving independent mutations.
🤖 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/baml_project/src/db.rs`:
- Around line 163-180: Wrap compiler2_file_map and removed_file_tombstones in
shared copy-on-write storage, matching the existing Arc and Arc::make_mut
pattern used by file_map and file_id_to_path. Update their mutation sites and
accessors accordingly so parallel_db_handle() clones remain O(1) while
preserving independent mutations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a593b13e-4b3b-4ae2-8ebe-87cce68307f2
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
baml_language/Cargo.tomlbaml_language/crates/baml_cli/src/check_command.rsbaml_language/crates/baml_cli/src/pack_command.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_project/src/check.rsbaml_language/crates/baml_project/src/db.rsbaml_language/crates/baml_project/src/lib.rsbaml_language/crates/bex_cache/Cargo.tomlbaml_language/crates/bex_project/src/project.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/mod.rsbaml_language/crates/sys_jinja_types/src/evaluate_type/types.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- baml_language/Cargo.toml
- baml_language/crates/sys_jinja_types/src/evaluate_type/types.rs
- baml_language/crates/baml_cli/src/check_command.rs
- baml_language/crates/baml_lsp2_actions/src/check.rs
- baml_language/crates/baml_cli/src/pack_command.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.
What this does
Six fixes that came out of benchmarking the BAML compiler head-to-head against tsgo (native TypeScript) and
go buildon generated, semantically-identical corpora from 1.5k to 295k LOC.Compiler core
jinja_base_typessalsa query per package+namespace) + skipped for files with no prompts/template_strings. It was rebuilt per checked file — O(N²) — and twice per compile viaget_bytecode's error gate. This was ~95% of the quadratic CPU.ProjectDatabasehandle clones O(files) → O(1) (Arc-wrapped file maps). The parallel drivers clone a handle per work chunk (~1.55N per compile); the deep map copies were the second quadratic (CPU + peak RSS).check/runrouted through a serial loop while the identical parallel machinery sat next to it (only reachable with the cache disabled).get_bytecodeno longer re-checks the whole project inside emit — the LSP proves errors once and callsget_bytecode_unchecked. To keep that sound, the LSP diagnostics candidate now includes package-level diagnostics (cross-file name conflicts / namespace shadows), which previously never reached the editor at all.CLI cache surface
5.
baml packnow uses the bytecode cache (it never opened one — every pack was fully cold). Whole-program hit / per-file reuse / store, sharing run+check's key space. PackedProgramis target-independent; emit determinism keeps reused images byte-identical.6.
baml checknow seeds the cache after a clean check (only when it advances the manifest — no-op checks stay emit-free; a failed emit is logged, never a check failure). Check-only workflows finally reach the warm path.Plus
sha2/asm(hardware SHA for the ~230MB of unit hashing per warm run at large sizes; target-gated, wasm unaffected).End-user impact (measured, Apple Silicon, hyperfine medians)
Cold compiles scale linearly now (was ~quadratic: 5× the code cost 33× the CPU):
checkbeforecheckafterrunbeforerunafterpackbeforepackafterAt 295k LOC: user CPU 125.7 s → 8.9 s (14×), peak RSS 5.3 GB → 3.6 GB.
One warm cache now serves every command (59k LOC project):
Re-pack of a 295k LOC project: 40.5 s → 0.61 s (packed binary verified to execute).
For context at 59k LOC: cold check+emit+cache-store now lands at classic
tsc --noEmitspeed (1.01 vs 1.00 s) and ahead ofgo build(1.23 s); tsgo remains far ahead (0.13 s) — closing that further is warm-floor work (tracked below), not this PR.Trade-off made deliberately: cold
checkat mid-size projects (~15–30k LOC) is ~10% slower than before because it now emits + stores once to warm the cache; every subsequent check/run/pack is then served warm.Editor fix riding along: cross-file duplicate-name / namespace-shadow errors now appear in editor diagnostics (they were silently dropped by the LSP's per-file sweep).
Not in this PR (known, profiled, next)
checkloads all unit bytes it doesn't need (~90% of warm no-op cost at 295k LOC). Design for fixing it (TIR-space throws compare + manifest-resident unit metadata) is under adversarial review.baml test's separateemit_test_caseskeyspace still doesn't share this cache.Letting CI run the test suites for verification; diagnostics output was verified byte-identical (cold + warm) against the pre-fix binary on an error-heavy corpus during development.
🤖 Generated with Claude Code
https://claude.ai/code/session_01V4oee6wrCHzXH6mM2h8eJo
Summary by CodeRabbit
Performance
Bug Fixes
Developer Experience