|
| 1 | +### Performance |
| 2 | + |
| 3 | +**The callee-rooting window at a call site is computed, not hardcoded — and the |
| 4 | +measurement says it is not what `#8159` attributed it to.** |
| 5 | + |
| 6 | +`#8084` closed a real moving-GC defect: five call arms lowered the CALLEE into a |
| 7 | +bare register, lowered the arguments below it — each of which can allocate — and |
| 8 | +handed the original register to the consuming call. Under the shipping |
| 9 | +statepoint lowering that register is in no live bundle, so nothing marks it and |
| 10 | +nothing relocates it. The fix asked `rooting/`'s existing machinery for |
| 11 | +protection and paid for it with a hardcoded `collects = true` at every site. |
| 12 | + |
| 13 | +`collects` is not a strategy, it is a **window**: "can anything between this |
| 14 | +operand and its consumer collect?" Hardcoding it `true` buys a slot store, a |
| 15 | +re-read and a release at every call site whether or not the window contains a |
| 16 | +collection point. `operand_protection` has always been able to answer this — its |
| 17 | +`Reuse` arm emits no push, no re-read and no truncate, keeping the pre-`#8084` |
| 18 | +IR byte for byte. It needed the truthful window, which `any_operand_may_collect` |
| 19 | +computes and `with_operands_rooted_window` has passed all along. |
| 20 | + |
| 21 | +- `lower_call/early_branches.rs` (closure-typed local call) — the window is the |
| 22 | + argument lowering. The re-read sits above the unmask because that is where the |
| 23 | + value leaves the tracked domain; a collection point *below* the unmask is a |
| 24 | + separate exposure that rooting the box cannot repair either way. |
| 25 | +- `expr/new_dynamic.rs`, both `js_new_function_construct` arms — the callee's |
| 26 | + window is every argument, argument `i`'s is the arguments after it. Neither |
| 27 | + reaches a collection point of its own: `lower_js_args_array` is an entry |
| 28 | + alloca plus stores, and `emit_call_location_at` emits nothing at all in a |
| 29 | + default build. `new C(a, b)` over plain locals is back to emitting no rooting, |
| 30 | + which is the shape `operand_needs_root`'s own doc already claims for it. |
| 31 | +- `expr/new_dynamic.rs`'s `NewDynamicSpread` and `expr/call_spread.rs` keep |
| 32 | + `true`, and now say why: `bundle_args_rooted` opens with `js_array_alloc`, and |
| 33 | + a spread call reaches `js_array_like_to_array` unconditionally, so those |
| 34 | + windows allocate in every instance of the arm. Nothing to narrow, as opposed |
| 35 | + to nothing narrowed. |
| 36 | + |
| 37 | +Each flag is computed **below** the operand it protects, as |
| 38 | +`with_operands_rooted_window` computes it: the predicate reads `ctx`, so asking |
| 39 | +before the lowering asks about a different state. |
| 40 | + |
| 41 | +**The measurement is negative, and that is the finding.** `#8159` attributed |
| 42 | +`pipeline`'s +3.95% to this rooting sequence. `pipeline` never reaches these |
| 43 | +arms: its `rec = stage(rec)` lowers through `lower_dynamic_closure_call` |
| 44 | +(`js_closure_unbox_callee_checked`), already fully rooted before `#8084`, and |
| 45 | +the emitted IR for `gc-handoff/apps/pipeline.ts` is BYTE-IDENTICAL across this |
| 46 | +change. Instructions retired, min-of-5, identical runtime archives on both arms, |
| 47 | +stdout sha-identical: `pipeline` +0.02%, `interp` −0.04%, `iso_miss` −0.05%, |
| 48 | +`asyncpipe` +0.11%, `shapes` −0.18% — all inside noise. What it does move is the |
| 49 | +population that has these shapes: zod's dep-native live bundles 36611 → 36598, |
| 50 | +relocates 432545 → 432338. So `#8159`'s attribution to the commit stands and its |
| 51 | +attribution to that hunk does not; the cost is elsewhere in `#8084`'s other |
| 52 | +~2500 lines. |
| 53 | + |
| 54 | +**Soundness, both corpora, base versus this change on the same build:** |
| 55 | +dep-native `unrooted 2` (budget 3), `stale 0`, seeded 40/40 — identical; |
| 56 | +curated-native `unrooted 0`, `stale 1`, seeded 39/40 — identical, and both |
| 57 | +halves of that verdict are pre-existing on clean `07c8040bf` (invisible until |
| 58 | +`#8207`, because the job aborted earlier at `--audit-poll-capable`). `#7803`'s |
| 59 | +own shape keeps its root by construction: its arguments are freshly-allocated |
| 60 | +object literals. |
| 61 | + |
| 62 | +`temp_root_coverage/call_callee.rs` states the contract from both sides — two |
| 63 | +differential pairs whose fixtures differ in exactly one thing, the argument's |
| 64 | +kind — and lives in `src/`, so it runs in the per-PR `cargo-test` gate rather |
| 65 | +than the nightly tier. Sabotage-checked against `#8084`'s hardcoded `true`: both |
| 66 | +NEGATIVES go red there and both positives stay green. The callee local's |
| 67 | +object-literal initializer is load-bearing and commented as such — |
| 68 | +`expr_is_known_non_pointer_shadow_value` suppresses rooting for a `LocalGet` |
| 69 | +with no reserved shadow slot whose type proof is not pointer-bearing, so a first |
| 70 | +draft using `Expr::Undefined` had both POSITIVES failing against a correct |
| 71 | +compiler and both negatives passing for that reason rather than the one they |
| 72 | +claim. |
0 commit comments