-
-
Notifications
You must be signed in to change notification settings - Fork 158
perf(codegen): CPU-aware default for native LLVM unit workers (#8583) #8593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
proggeramlug
wants to merge
4
commits into
PerryTS:main
from
proggeramlug:perf/8583-native-unit-workers
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
a054dfe
fix(codegen): replace the pre-RS4GC optnone knob with a post-rewrite …
de1225b
docs: changelog fragment for #8586
c1809b3
perf(codegen): spill giant-function GC roots to a shadow frame to bou…
2f86b48
perf(codegen): CPU-aware default for native LLVM unit workers (#8583)
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| ### Changed | ||
|
|
||
| - `PERRY_LL_PREOPT_OPTNONE_INSTRS` is removed (#8583). It stamped `optnone` before `rewrite-statepoints-for-gc`, which makes the pass manager skip `mem2reg`/`sccp` while RS4GC still runs, so a demoted function's root allocas were never promoted and the collector never saw them. The cap defaulted to 0, so no shipped build was affected; a test now pins that an `optnone` function loses every root under the rewrite. | ||
| - `PERRY_LL_RS4GC_MAX_INSTRS` (default 1.5 Mi): after `rewrite-statepoints-for-gc`, a function whose body exceeds the per-function budget fails its codegen unit with the function's name and its sizes before and after the rewrite, instead of entering an optimizer pipeline that is super-linear on statepoint relocation fan-out and would not finish. This is an assertion, not a fallback — no function is ever demoted and the requested optimization level applies to every function. `<n>` raises it, `warn:<n>` only warns, `0` disables. Both caches key on it. | ||
| - `PERRY_CODEGEN_UNIT_TIMINGS` now reports, per codegen unit, the widest function by estimated IR before LLVM starts, and after compile the instruction totals and widest function before and after RS4GC, the growth factor, and rewrite/optimize/emit times. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ### Changed | ||
|
|
||
| - Native GC-root spilling (#8583): a function whose estimated statepoint relocation count — `live_root_slots × safepoints` — exceeds `PERRY_ROOT_SPILL_RELOCATIONS` (default 4,000,000) keeps its GC roots in a heap shadow frame instead of native statepoints. `rewrite-statepoints-for-gc` adds one relocation per live root per safepoint, so a minified-bundle entry function (measured: 795 root slots × ~106k safepoints, grown 439k → 6.5M instructions under RS4GC) drove the `-Os` middle-end super-linear and did not finish; the same unit optimizes in ~5s once that one function is spilled. The function is still compiled at the requested optimization level — only its root representation changes — and its roots stay precise: the runtime already scans shadow-frame and stack-map roots in one walk, and the frame pointer is kept so the FP-chain walker steps over the spilled frame. Each spilled function is reported at default verbosity. `PERRY_ROOT_SPILL_RELOCATIONS=0` disables spilling (every function on native statepoints, the previous behavior). |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ### Changed | ||
|
|
||
| - The default number of concurrent native LLVM codegen-unit workers is now CPU-aware on non-Windows: half the machine's logical CPUs, clamped to `[2, 8]`, instead of a hard-coded `2` (#8583). The `2` default (#8017) was chosen for Windows pagefile pressure and applied everywhere; on a large real bundle (the Claude Code `cli.js` lowers to ~84 units) it left most cores idle while dozens of ~7-minute units ran two at a time. With the giant entry function's roots spilled (#8583) no unit carries an unbounded RS4GC fan-out, so per-unit peak RSS is a bounded ~1-2 GiB and the two-worker cap — not memory — was the wall. Windows keeps the conservative `2`. `PERRY_CODEGEN_UNIT_JOBS` still overrides on every platform. |
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| //! Count the GC safepoints in a function body (#8583). | ||
| //! | ||
| //! `rewrite-statepoints-for-gc` inserts, at every safepoint, one relocation | ||
| //! per GC value live across it — so the optimizer's post-rewrite work grows | ||
| //! with `live_roots × safepoints`. A function whose product is large enough | ||
| //! makes the `-Os`/`-O3` middle-end super-linear: the 68 MB minified entry | ||
| //! body of the Claude Code bundle measured 795 root slots × ~106k safepoints | ||
| //! and grew 439k → 6.5M instructions under RS4GC, and a single `-Os` pass on | ||
| //! the result did not finish in practical time (#8583). | ||
| //! `codegen/helpers::maybe_spill_roots_to_shadow_frame` multiplies this count | ||
| //! by the function's root-slot count and, past a threshold, keeps that | ||
| //! function's roots in a shadow frame instead of statepoints. | ||
| //! | ||
| //! A safepoint is any call-like expression: a call can re-enter the runtime | ||
| //! and collect. The count is an over-approximation biased toward spilling — | ||
| //! a false positive is a shadow frame on a function that would have been fine | ||
| //! (cheap; the shadow lowering is the pre-#7370 default), while a false | ||
| //! negative would let relocation fan-out reach the optimizer. Nested closures | ||
| //! are NOT counted: each compiles to its own `LlFunction` with its own frame, | ||
| //! so its safepoints belong to it (`walk_expr_children` does not descend into | ||
| //! a closure's body, only its parameter defaults). | ||
|
|
||
| use perry_hir::{Expr, Stmt}; | ||
|
|
||
| /// Total call-like expressions reachable from `stmts` without descending into | ||
| /// nested closures. | ||
| pub fn count_safepoint_sites(stmts: &[Stmt]) -> usize { | ||
| let mut n = 0usize; | ||
| for s in stmts { | ||
| count_in_stmt(s, &mut n); | ||
| } | ||
| n | ||
| } | ||
|
|
||
| /// A call-like expression is a potential safepoint: anything whose lowering | ||
| /// emits a call that can re-enter the runtime. Nodes not listed contribute | ||
| /// nothing themselves but are still recursed into, so adding a new call | ||
| /// variant can only make the estimate more conservative (a possible | ||
| /// under-count that the post-RS4GC instruction-budget assertion backstops), | ||
| /// never wrong in a way that hides a fan-out. | ||
| fn is_safepoint(e: &Expr) -> bool { | ||
| matches!( | ||
| e, | ||
| Expr::Call { .. } | ||
| | Expr::CallSpread { .. } | ||
| | Expr::NativeMethodCall { .. } | ||
| | Expr::StaticMethodCall { .. } | ||
| | Expr::SuperCall(_) | ||
| | Expr::SuperCallSpread(_) | ||
| | Expr::SuperMethodCall { .. } | ||
| | Expr::SuperMethodCallSpread { .. } | ||
| | Expr::ObjectSuperMethodCall { .. } | ||
| | Expr::New { .. } | ||
| | Expr::NewDynamic { .. } | ||
| | Expr::NewDynamicSpread { .. } | ||
| | Expr::Await(_) | ||
| | Expr::Yield { .. } | ||
| | Expr::AsyncFirstCall { .. } | ||
| ) | ||
| } | ||
|
|
||
| fn count_in_expr(e: &Expr, n: &mut usize) { | ||
| if is_safepoint(e) { | ||
| *n += 1; | ||
| } | ||
| // Generic recursion into direct sub-expressions. `walk_expr_children` does | ||
| // not descend into a closure's statement body (only its param defaults), | ||
| // which is exactly the boundary we want: a nested closure is a separate | ||
| // frame and its safepoints are not this function's. | ||
| perry_hir::walker::walk_expr_children(e, &mut |child| count_in_expr(child, n)); | ||
| } | ||
|
|
||
| fn count_in_stmt(s: &Stmt, n: &mut usize) { | ||
| match s { | ||
| Stmt::Let { init: Some(e), .. } | ||
| | Stmt::Expr(e) | ||
| | Stmt::Throw(e) | ||
| | Stmt::Return(Some(e)) => count_in_expr(e, n), | ||
| Stmt::Let { init: None, .. } | Stmt::Return(None) => {} | ||
| Stmt::If { | ||
| condition, | ||
| then_branch, | ||
| else_branch, | ||
| } => { | ||
| count_in_expr(condition, n); | ||
| for st in then_branch { | ||
| count_in_stmt(st, n); | ||
| } | ||
| if let Some(else_branch) = else_branch { | ||
| for st in else_branch { | ||
| count_in_stmt(st, n); | ||
| } | ||
| } | ||
| } | ||
| Stmt::While { condition, body } | Stmt::DoWhile { body, condition } => { | ||
| count_in_expr(condition, n); | ||
| for st in body { | ||
| count_in_stmt(st, n); | ||
| } | ||
| } | ||
| Stmt::For { | ||
| init, | ||
| condition, | ||
| update, | ||
| body, | ||
| } => { | ||
| if let Some(init) = init { | ||
| count_in_stmt(init, n); | ||
| } | ||
| if let Some(condition) = condition { | ||
| count_in_expr(condition, n); | ||
| } | ||
| if let Some(update) = update { | ||
| count_in_expr(update, n); | ||
| } | ||
| for st in body { | ||
| count_in_stmt(st, n); | ||
| } | ||
| } | ||
| Stmt::Labeled { body, .. } => count_in_stmt(body, n), | ||
| Stmt::Try { | ||
| body, | ||
| catch, | ||
| finally, | ||
| } => { | ||
| for st in body { | ||
| count_in_stmt(st, n); | ||
| } | ||
| if let Some(catch) = catch { | ||
| for st in &catch.body { | ||
| count_in_stmt(st, n); | ||
| } | ||
| } | ||
| if let Some(finally) = finally { | ||
| for st in finally { | ||
| count_in_stmt(st, n); | ||
| } | ||
| } | ||
| } | ||
| Stmt::Switch { | ||
| discriminant, | ||
| cases, | ||
| } => { | ||
| count_in_expr(discriminant, n); | ||
| for c in cases { | ||
| if let Some(t) = &c.test { | ||
| count_in_expr(t, n); | ||
| } | ||
| for st in &c.body { | ||
| count_in_stmt(st, n); | ||
| } | ||
| } | ||
| } | ||
| // No expression children. | ||
| Stmt::Break | ||
| | Stmt::Continue | ||
| | Stmt::LabeledBreak(_) | ||
| | Stmt::LabeledContinue(_) | ||
| | Stmt::PreallocateBoxes(_) | ||
| | Stmt::PreallocateTdzBoxes(_) | ||
| | Stmt::ReleaseBoxes(_) => {} | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::count_safepoint_sites; | ||
| use perry_hir::types::Type; | ||
| use perry_hir::{Expr, Stmt}; | ||
|
|
||
| fn call(args: Vec<Expr>) -> Expr { | ||
| Expr::Call { | ||
| callee: Box::new(Expr::Undefined), | ||
| args, | ||
| type_args: vec![], | ||
| byte_offset: 0, | ||
| } | ||
| } | ||
|
|
||
| fn empty_closure(body: Vec<Stmt>) -> Expr { | ||
| Expr::Closure { | ||
| func_id: 0, | ||
| params: vec![], | ||
| return_type: Type::Any, | ||
| body, | ||
| captures: vec![], | ||
| mutable_captures: vec![], | ||
| captures_this: false, | ||
| captures_new_target: false, | ||
| enclosing_class: None, | ||
| is_arrow: false, | ||
| is_async: false, | ||
| is_generator: false, | ||
| is_strict: false, | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn counts_calls_across_control_flow_but_not_into_closures() { | ||
| let stmts = vec![ | ||
| Stmt::Expr(call(vec![])), | ||
| Stmt::While { | ||
| condition: Expr::Bool(true), | ||
| body: vec![Stmt::Expr(call(vec![]))], | ||
| }, | ||
| // A call buried in a nested closure body must NOT be counted. | ||
| Stmt::Expr(empty_closure(vec![Stmt::Expr(call(vec![]))])), | ||
| Stmt::Return(Some(call(vec![]))), | ||
| ]; | ||
| assert_eq!(count_safepoint_sites(&stmts), 3); | ||
| } | ||
|
|
||
| #[test] | ||
| fn call_arguments_are_themselves_safepoints() { | ||
| // f(g(), h()) is three calls. | ||
| let nested = call(vec![call(vec![]), call(vec![])]); | ||
| assert_eq!(count_safepoint_sites(&[Stmt::Expr(nested)]), 3); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Include the closure-self root in the spill estimate.
Lines 615-620 omit the slot reserved for
%this_closureat line 897 when the closure has captures. The estimate can fall below the threshold by one live root per safepoint and leave a pathological closure on the RS4GC path.Add that slot to the estimate. A conservative unconditional
+ 1is safe, or compute the capture-root condition before this call.Proposed fix
crate::codegen::helpers::maybe_spill_roots_to_shadow_frame( lf, &llvm_name, - m.len() + capture_root_slots as usize, + m.len() + capture_root_slots as usize + 1, body, );📝 Committable suggestion
🤖 Prompt for AI Agents