Compiler perf: track package alias environments in Salsa (1,167 rebuilds → ~10)#3913
Conversation
…compile) The type-alias environment was rebuilt from scratch at three kinds of sites, none of them memoized: - infer_scope_types rebuilt the package alias map (own aliases plus dependency interface exports) and its recursive-alias set once PER SCOPE (1,053 times compiling the empty project's stdlib) - callable_throws did the same once per analyzed callable (708 times) - MIR's resolved_aliases_for_package recomputed per caller (emit's build_alias_caches and package_lowering_data) Each rebuild re-collected every alias definition and cloned every alias Ty. Now there are two tracked queries, one per environment semantics: - tir::inference::package_alias_env(db, pkg_id): own aliases plus dependency *interface exports* -- the TIR view. infer_scope_types, callable_throws, and the LSP check pass fetch it; TypeInferenceBuilder borrows &'db ResolvedAliases instead of owning a per-scope copy. - mir::resolved_aliases_for_package(db, pkg_id), now #[salsa::tracked(returns(ref))]: ALL dependency aliases -- the erasure view MIR/emit need. emit's alias caches hold &'db refs; package_lowering_data clones once per package. Execution counts per empty-project compile: 1,167 environment builds before, ~10 after. Empty-project cold compile measured back-to-back on battery power: 392ms -> 363ms median (-7.5%); AC numbers to be reconfirmed (the earlier 281ms baseline was measured on AC power). Also deletes the LSP's collect_type_aliases_for_resolution_context helper (now subsumed by the query). Co-Authored-By: Claude Fable 5 <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.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
1 participant
Third PR in the compiler-perf series, stacked on #3912 (which stacks on #3911).
The problem, measured by execution counts
Instrumenting one cold compile of the empty project (which compiles the stdlib) showed the type-alias environment being rebuilt 1,167 times:
infer_scope_typesTy, hand the builder an owned copycallable_throwsresolved_aliases_for_package(MIR)The change
Two tracked queries, one per environment semantics (they are intentionally distinct and documented as such):
tir::inference::package_alias_env(db, pkg_id): own aliases + dependency interface exports (the TIR view).infer_scope_types,callable_throws, and the LSP check pass fetch it;TypeInferenceBuildernow borrows&'db ResolvedAliasesinstead of owning a per-scope copy.mir::resolved_aliases_for_package(db, pkg_id), now#[salsa::tracked(returns(ref))]: all dependency aliases (the erasure view MIR/emit need). Emit's per-package caches hold&'dbrefs;package_lowering_dataclones once per package.After: ~10 environment builds per compile. Also deletes the LSP's now-redundant
collect_type_aliases_for_resolution_contexthelper. Net −71/+79 lines.Numbers (and a measurement caveat)
Measured back-to-back on battery power: 392ms → 363ms median (−7.5%).
The caveat worth recording: our earlier numbers in this series (1.41s → 281ms) were measured on AC power; battery throttling costs ~40% uniformly on this machine and initially masqueraded as a regression of this very change. The −7.5% above is a clean same-power-state A/B. AC-equivalent projection: ~260ms. AC re-measurement to follow.
Beyond wall time, this removes an allocation pattern that scales with project size (one full alias-map clone per scope), and moves the builder to the same borrow-from-Salsa discipline as the rest of the pipeline.
Validation
cargo clippy --all-targets --all-features -- -D warnings: clean🤖 Generated with Claude Code