Repository navigation
Counted loops lose packed-array admission to any abrupt statement — break/continue/return/throw all 6x, even when never taken #9151
Description
Activity
- changed the title
[-]Counted loops lose packed-array admission to any early exit — break and return both 6x, even when never taken[/-][+]Counted loops lose packed-array admission to any abrupt statement — break/continue/return/throw all 6x, even when never taken[/+]on Aug 30, 2026 Follow-up for the
throwrow, which #9154 left at 4.76 ns against node's 0.56.I tried the obvious extension — admitting
Stmt::Throwinstmt_is_packed_f64_loop_safe— on the argument that a throw is the one place a call in the body is harmless: it leaves the loop permanently, so an evacuation triggered while building the thrown value can move the array freely because nothing reads the clone's cached base afterwards.That change measured as a complete no-op (throw stayed at 4.76), and the reason is worth recording so the next person doesn't repeat it. The HIR-level statement predicate is not what rejects these loops.
fast_clone_call_freeatstmt/loops.rs:4607scans the emitted fast-clone blocks for GC-unsafe calls:let fast_clone_call_free = !ctx.func.blocks()[fast_pre_idx].contains_gc_unsafe_call() && … .all(|idx| !ctx.func.blocks()[idx].contains_gc_unsafe_call());
throw new Error(…)allocates, so that allocation call lands inside the clone's blocks and the scan rejects it regardless of what the HIR predicate said. The scan is a blanket "any call in the clone is unsafe" check and structurally cannot express "this call is on a path that exits the loop".So a real fix has to teach that scan the difference — e.g. excluding blocks that provably end in an unwind from the call-free requirement — rather than widening the statement predicate. That is a GC-safety invariant, so it wants a deliberate design rather than a threshold change, and I have not attempted it.
Validation tooling for whoever does: a differential where the throws actually fire at varying indices, run under
PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_SCHEDULE_SEED=<n> PERRY_GC_SCHEDULE_RATE=1with the arrays escaping into a sink so they are not scalar-replaced — that configuration reached 107 collections and 1,589 moved objects on a small fixture, versus 61 when the arrays did not escape.Third attempt at the
throwrow (4.76 ns vs node's 0.56), and a more precise account of what blocks it. All reverted; nothing shipped.There are two gates, and relaxing either alone does nothing. My earlier comment established that
stmt_is_packed_f64_loop_safe(loops.rs:5011) is not the only one —fast_clone_call_free(loops.rs:4607) scans the emitted clone blocks viacontains_gc_unsafe_call, andthrow new Error(…)allocates inside them. This round I relaxed both together, which is necessary but still not sufficient.The idea was to give the scan a terminator vocabulary, as suggested on this issue: a block ending in
unreachablehas no "after", so a call inside it cannot invalidate anything the clone caches and later re-reads — which is precisely the property the scan exists to enforce. I implemented that inLlBlock::contains_gc_unsafe_call(returnfalsewhen the block's last instruction isUnreachable) alongside admittingStmt::Throwat the HIR gate.It measured as a complete no-op, and the disassembly says why. For
if (l < 0) throw new Error("x")inside a counted loop, the emitted function contains ajs_throwcall and nounreachableat all:bl <_js_typed_feedback_numeric_array_index_get_guard> bl <_js_typed_feedback_array_index_get_fallback_boxed> bl <_js_throw>js_throwis declaredVOID(runtime_decls/strings_part2.rs:742), not noreturn, and codegen does not terminate the block after it. So the exemption keys on a shape that does not currently exist, and the loop stays on the generic typed-feedback path.What a working fix needs, in order:
- Make the throw actually terminate its block — declare
js_thrownoreturnand emitunreachableafter the call. Until the IR says control cannot continue, the "no second read" argument is true at runtime but not true in the IR, and a scan is entitled to disbelieve it. - Then the terminator exemption in
contains_gc_unsafe_callbecomes reachable and sound. - Then admitting
Stmt::Throwat the HIR gate has an effect.
Step 1 is the load-bearing one and touches the throw ABI, so it wants its own change and its own validation rather than riding along with a loop optimisation. Worth noting
gc_call_effects.rs:244already refers to "the audited noreturn funnel" for the throw path, so the concept exists somewhere in the pipeline — whoever picks this up should start by finding out whether that funnel andjs_throw's declaration disagree.Validation harness if useful: a differential where throws actually fire at varying indices, run under
PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_SCHEDULE_SEED=<n> PERRY_GC_SCHEDULE_RATE=1with arrays escaping into a sink — that reaches 107 collections and 1,589 moved objects where non-escaping arrays reach 61.- Make the throw actually terminate its block — declare
Fourth pass, and the residual is narrower than the last comment said: on current main, only
newis blocked — calls and allocations in a thrown expression already take the fast path.Measured on main (
84185b5656, i.e. with #9185), same loop, ns/op, node is 0.56 for all four:thrown expression perry throw new Error("x")4.76 — blocked throw "bad" + l(allocates a string)0.58 throw String(l)(a runtime call)0.58 throw PRE(pre-built)0.58 So my previous comment here was too pessimistic. I had written that the construction is emitted in blocks preceding the
unreachable-terminated one and therefore needs a region/dominance property. That is not what happens — for these shapes the operand is emitted into the same block that ends inunreachable, so #9185's terminator check already covers them. A call in a thrown expression is fine today; so is an allocation.I verified this by building the region property anyway — marking every block created while lowering a throw's operand — and it measured byte-identical to main on all four rows. A no-op, and worth recording as one so nobody builds it again.
What is actually left
Only
new.throw new Error(…)stays on the generic path whilethrow String(l)does not, and the difference between those two is not "is there a call" — both call. That points at the whole-function materialization hazard rather than at either of the two gates discussed above, and the admission code names it directly (loops.rsaround theread_body_is_safecomputation, whose comment describes the hazard as "tripped by the verynew Array(n).fill(0)construction calls that build these buffers").So whoever picks this up should start at the materialization hazard and its interaction with
newin a loop body, not atstmt_is_packed_f64_loop_safeorcontains_gc_unsafe_call— both of those are already open for this shape, and I have now confirmed each of them separately.Three gates found, two open, one remaining. Recording the map so the fourth attempt starts where the third stopped.
Found the gate, and it is none of the three we guessed. It is
stmt_preserves_array_length, reached throughclassify_for_length_hoist.I stopped guessing and built the trace (#9204). It answered on first use:
[packed-loop] considering a counted loop x3 [packed-loop] admitted x1 [packed-loop] rejected: length_hoist_declined x2match_packed_f64_versioned_loopnever reaches any of the conditions previously suspected — it exits at the?onclassify_for_length_hoist/classify_for_length_hoist_impl, which was the one early exit reporting nothing. (That?is now alet-elsewith a reason, so the silence that misled three attempts cannot recur.)The actual condition
classify_for_length_hoist_implrequires every body statement to satisfystmt_preserves_array_length— the loop body must provably not changearr.length, because the hoisted bound would go stale. ItsStmt::Throw(e)arm recurses into the thrown expression, andnew Error(…)is a constructor call it cannot prove harmless.That is consistent with every measurement on this issue, including the one that looked paradoxical:
thrown expression result why throw PRE0.58 ns no call throw "bad" + l0.58 ns concat is length-preserving throw String(l)0.58 ns a call, but a known-pure builtin throw new Error("x")4.76 ns new— not provably length-preservingString(l)passing is the datum that rules out "any call blocks it" and points specifically at hownewis classified.What a fix needs, and the trap in it
Teaching
expr_preserves_array_lengththat a builtin Error-family constructor cannot touch an unrelated local array. The soundness question is not the constructor's behaviour but its identity: user code may shadow the global (class Error { constructor() { arr.length = 0 } }), so the fix must confirm theNewresolves to the intrinsic rather than trusting the name"Error". A name-based check would be a wrong-answer bug, not a missed optimisation.I have not implemented it — establishing that identity is the whole of the work and it deserves its own change rather than riding on a diagnostic PR.
State
Three gates suspected across three attempts, all three now confirmed innocent for this shape (
stmt_is_packed_f64_loop_safeadmitsThrow;contains_gc_unsafe_callexempts unreachable-terminated blocks; the block-region property measured byte-identical to main and was reverted). The fourth is the real one and is named above.- added 5 commits that reference this issue
on Aug 30, 2026 Closing the loop on this thread, since chasing the trace's answer turned up something bigger than the original shape.
Using #9204's trace, the gate on
throw new Error(…)wasstmt_preserves_array_length. Widening it to accept the Error-family HIR nodes is sound — HIR only emits them for the intrinsic (lower_newgates on!shadowed_by_user_binding), so the identity question I flagged earlier is already answered upstream, and four shadowing forms confirm it. But I measured it as a no-op and did not land it: admission rejects a constructed throw operand before the length hoist is ever consulted, so the arm changes nothing.throw new Error("bad")and a barethrow "bad " + iboth sit at exactly 7.99 ns — the Error-ness is irrelevant.What the trace turned up instead: #9185, my own merged change, was a silent wrong answer. It admitted
throwwithout the loop-carried writeback on the unwind edge, so acatchread the stale pre-loop slot —scame back 0 where node gives 780.break/continuewere unaffected (normal CFG edges flush at the exit block) and a closure-captured accumulator was unaffected (boxed, never register-promoted), which is exactly why it was narrow enough to ship.- fix(codegen): a taken throw out of a packed loop no longer loses loop-carried locals #9215 reverts the admission — correctness first, cost stated.
- perf(codegen): flush packed accumulators at the throw site, restoring the fast path (7.99 → 0.95 ns) #9230 fixes it properly: the writeback is emitted at the throw site.
throw PREin a counted loop goes 7.99 → 0.95 ns/op against node's 1.10, so that shape now beats node by 1.16× instead of trailing it by 7.2×. - Packed loop: a throw whose value is CONSTRUCTED stays on the generic path (7.99 vs 0.95 ns) — needs a GC-safety argument, not just admission #9232 records the residual (constructed operands, 7.99 vs node's 7.73 — parity) and, more usefully, why closing it is a GC-safety design question rather than an admission tweak.
The generalisable part, and the second time this exact shape has bitten: an abrupt-exit optimisation is only really tested by an exit that is actually taken AND a subsequent read of something the loop wrote. #9185 shipped two taken-throw tests; both read the thrown value or an untouched variable, which observes the clone's live SSA value — correct — and never the frame slot left behind. #9154's labeled-
breakbug had the identical structure: two conditions that must coincide, and every test satisfied exactly one.- added 8 commits that reference this issue
on Aug 30, 2026
Summary
A counted loop over a packed array loses its fast path when the body contains any abrupt-completion statement —
break,continue,returnorthrow— whether or not it is ever taken. All four cost the same 6x against perry's own straight-line loop, and ~8.5x against node.A plain conditional is not the trigger: an
ifwhose body only assigns is fully fast. Andcontinuedoes not leave the loop at all, which is the datum that rules out "early exit" as the explanation.Repro
arrholds only non-negative values, so no branch is ever taken in any variant.Measurement
Quiet Mac mini, node v26.5.1, ns/op, best of 3:
if (l < 0) { l = 0; }if (l < 0) { sink = 1; }breakin the outer loopif (l < 0) break;if (l < 0) continue;if (l < 0) return -1;if (l < 0) throw …;The array is a bare local throughout, so no receiver or property-lookup effect is involved. The four abrupt forms landing on the same number to within 0.02 ns suggests one predicate rejecting all four rather than four separate causes.
Node is flat across every variant, so the fast path is not inherently incompatible with abrupt control flow.
Where it is
crates/perry-codegen/src/stmt/loops.rs:5011, instmt_is_packed_f64_loop_safe, which lumps every abrupt statement into one rejection arm:It is one predicate, not two, which is what the four identical timings were telling us. The caller at
loops.rs:4792requires every body statement to pass it in order to setread_body_is_safe; when that fails and there is no store kind and no ordinary hoist, admission returnsNoneatloops.rs:4796and the loop falls to the generic path.The relaxation this predicate guards is documented just above the call site:
By that argument
break,continue,LabeledBreakandLabeledContinuelook admissible as written: none of them calls out, mutates the array, or invalidates the entry guard.Return(Some(expr))andThrow(expr)need their operand checked (athrow new Error(…)does construct), but a barereturndoes not. Two other places in the same file already treat these statements as harmless for the analogous question —stmt_array_length_effect(loops.rs:7439) andstmt_preserves_array_length(loops.rs:7872) both answerPreserves/truefor exactly this set.The remaining question is not whether the body is safe but whether the emitted fast clone can carry a mid-loop exit edge. There is a related constraint in
stmt/stable_packed_loop.rs, which rejects separately withbreak_replays_current_iteration:That is a shared-block problem — a user
breakand a guard deopt land on one block meaning two different things ("leave the loop" vs "resume generically at this index") — so a fix likely has to give the user edge its own block before relaxing the predicate above.Why it matters
Find-first, any/all and early-out scans are the natural shape here, and they are exactly the loops whose exit is rarely taken — so the penalty falls hardest on loops that never actually exit early.
Found via
Isolated while measuring #9149 (the loop-invariant property-array hoist), which takes
holder.arr[i]in an early-return loop from 21.59 to 3.59 ns; this is the residual. The two are independent — the table above uses a bare local with no hoisting involved.