perf: shapes 0.139 -> 0.061 s (beats node) and pipeline 0.240 -> 0.175 s — a 2000-element array was born immortal, and this.vals[i]=v had no inline arm - #7895
Conversation
…s to 128 KB A 2000-element array is 16 400 bytes, sixteen over the flat 16 KB line, so it was born in old-gen with GC_FLAG_TENURED — which a minor never sweeps. Its remembered-set edges then kept every object it referenced live forever.
…local array receiver `this.vals[i] = v` had no inline arm at all — a full js_typed_feedback_array_set_f64_extend call — while the matching read has a complete guarded diamond. A strictly in-bounds store changes no head and no length, so it needs no writeback slot and can be inlined for receivers lower_index_set_fast cannot serve.
999 was read off retain/retain_wide/deeplist, and that reading was partly an artifact of the flat born-tenured threshold: array growth abandoned its intermediate backing stores into old-gen, so the garbage was never in the young generation to be counted. With those stores nursery-resident, retain's FIRST cycle measures 992 deterministically and every later one measures 1000. Also update the old-gen fixtures that sized themselves off the flat constant.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe runtime now selects large-object thresholds by object type, updates promotion limits, and aligns GC tests with those thresholds. Non-local typed-array stores gain a guarded inline path for strictly in-bounds writes while retaining the extending fallback. ChangesType-aware garbage-collection thresholds
Guarded in-bounds array stores
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant IndexSet
participant GuardedStore
participant ArrayStorage
participant ExtendHelper
IndexSet->>GuardedStore: submit receiver, index, and value
GuardedStore->>ArrayStorage: validate metadata and in-bounds state
alt Guards pass
GuardedStore->>ArrayStorage: store element and update layout bookkeeping
else Guards fail
GuardedStore->>ExtendHelper: extend-capable fallback store
end
Possibly related PRs
Suggested reviewers: ✨ 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 |
What this is
Two independent findings from the
shapes/pipelineround. Both are measured on thequiet M1 mini, best-of-5, exit-checked, against my own reference build of the same commit.
1.
shapeswas not a promotion-cost problem. It was a born-tenured leak.arena_alloc_gcbirths anything overLARGE_OBJECT_THRESHOLD_BYTES(16 KB) in the oldgeneration and stamps
GC_FLAG_TENURED— and a minor collection never sweeps old-gen.gc-handoff/apps/shapes.tsbuilds a 2000-elementNode2D[]per round and drops it. Itsbacking store is
8 + 2048*8 + 8 = 16 400bytes: sixteen bytes over the line. So everyround's array was immortal, the write barrier had recorded an old→young edge for each of
its 2 000 stores, and every subsequent minor's remembered-set scan marked all of them live
again — through containers nothing referred to any more.
PERRY_GC_TRACE=1, before:newly_markedshapes's actual live set is ~3 200 objects.770 800 = 47 × 16 400and94 000 = 47 × 2 000: 47 dead arrays, one per completed round.The controlled experiment, before touching any code:
shapes_half.tsrunsbuild(1000)× 120 instead ofbuild(2000)× 60 — identical total work, identicaloutput, and a 1000-element backing store is 8 200 bytes, one step under the line. Same
binary, same runtime, same box: 2 cycles / 93.9 ms of GC / 739‰ / 94 000 re-marks becomes
1 cycle / 5.0 ms / 30‰ / 0 re-marks.
The change
The threshold is now type-dependent, because crossing it trades two costs that are
only the same quantity for a pointer-free object:
memcpy, bounded by the object's own size;pointer_freeobject, its own bytes; for a pointer-bearingone, transitively everything it names, until a full mark-sweep.
So
pointer_freetypes keep 16 KB and arrays / objects / closures getLARGE_POINTER_BEARING_OBJECT_THRESHOLD_BYTES= 128 KB — V8'skMaxRegularHeapObjectSize, which draws this line for this reason. Selection reads theexisting
GcTypeInfo::pointer_freeflag rather than a hardcoded type list; an unknown typekeeps the conservative value.
Deliberately not widened for strings: a >16 KB string would newly move, and this repo
has a known class of latent "borrowed
&[u8]across an allocation" bugs. That risk buysnothing, because a dead pointer-free object retains only itself.
The widened value is inside both structural ceilings of the copier — the 1 MB nursery block
and
move_young's 1 MiBMAX_YOUNG_MOVE_BYTESrefusal — so everything it admits to thenursery is provably movable.
MAX_YOUNG_MOVE_BYTESis hoisted out of the function body soa unit test can assert that.
2.
this.vals[i] = vhad no inline arm at alllower_index_set_fastgivesa[i] = va guarded diamond only whenais a stacklocal, because it needs a slot to write a realloc'd head back to. Every other receiver
shape —
this.vals[i],obj.arr[i], a closure-captured array — fell through to afive-argument
js_typed_feedback_array_set_f64_extendcall, while the matching read hashad a complete inline diamond for both tiers all along.
The unlock: a strictly in-bounds store changes no head and no length, so it needs no
writeback and can be inlined for exactly the receivers that path cannot serve.
index == length(an extend), sparse writes, growth and every exotic array still take the helper.expr/index_set_guarded.rsstates the guard conjunction it proves, againstjs_typed_feedback_plain_array_index_set_guard's; the slot write reusesemit_jsvalue_slot_store_scalar_aware_on_block+emit_array_numeric_write_note_on_blockverbatim from the local-receiver arm, so the string addref, layout note, write barrier and
raw-f64 downgrade are one implementation, not two.
3. …and the one-permille interaction that fell out of it
Fixing (1) made
retain1+15.7% andretain_wide1+11.3%, and the mechanism is exact.young_survival_permille, deterministic on every run of all fourretain*variants andphase_flip:#7888's
UNTRACED_PROMOTION_SURVIVAL_PERMILLEis 999, so cycle 1 went from untraced(2.3 ms) to traced (18.1 ms). That single cycle was the whole regression.
Why the ratio moved, and why the old reading was the wrong one.
all.push(rec)growsits backing store by doubling, and under the flat 16 KB threshold every intermediate store
past 2048 elements was born in old-gen — so the garbage each growth abandons was never in
the young generation to be counted. 999‰ was measuring a nursery with its own array
garbage removed. With those stores nursery-resident the first cycle sees them and reads
992; every cycle after reads 1000 rather than 999, i.e. the estimator is strictly more
confident once that garbage is actually collected instead of parked in old-gen.
So 999 was over-fitted to an artifact of the policy this PR changes. Re-derived to 990:
the live mode now measures 992–1000 and the garbage mode still 0–4, three orders of
magnitude apart, and nothing in this corpus lands between them. The exposure it widens is
bounded twice already —
note_untraced_promotionchargespromoted × (1000 − permille) / 1000against the 32 MBPROMOTED_DEAD_BUDGET_BYTES, so anuntraced run capped at the 128 MB floor carries at most 1.28 MB of assumed-live-but-dead
bytes at 990 against 0.128 MB at 999. The binding bound is the untraced-bytes budget in
both cases, unchanged, and
phase_flipstill disarms on the flip cycle exactly as before.It more than pays the regression back:
retain1's GC pause goes 46.5 ms (base) → 60.4(without this) → 42.0,
retain70.0 → 80.4 → 57.5,retain_wide140.7 → 49.4 →33.8.
Validation
Quiet mini, best-of-5, exit-checked, VERDICT CLEAN (load 2.33 → 2.46, foreign 0 → 0)
s2baseis my own build of the matched merge base, not a borrowed binary.shapesnow beats node by 1.35× (was 1.71× slower) and is 1.6× scriptc (was 3.66×).pipelineis 1.86× node (was 2.81× at the start of this round) and 1.41× faster thanscriptc. Twelve programs with zero reach set the noise floor at ±1.3%.
GC pause and object counts, same quiet window (
PERRY_GC_TRACE=1):shapesbaseshapesthis PRretainbase → PRretain1base → PRretain_widebase → PRpipelinebase → PR★ On ns/promoted-object, read the counts, not the ratio.
shapesbefore is85.87 ms / 193 592 promoted = 443.6 ns per promoted object (244.3 ns per handled
object, counting the 157 909 copies). After, the figure is undefined because zero
objects are promoted: the 193 592 the base promoted were ~98% garbage, so the win is
not a cheaper promotion, it is that the promotion no longer exists. Per handled object
the ratio RISES (244 → 563 ns) purely because the fixed per-cycle root scan is now
amortised over 7 416 objects instead of 351 501 — which is exactly the "a share rises as
a program gets faster" trap, and why the counts are quoted beside it.
The rest
iso_misscanary, plain + stressedchecksum 437840 misses 0, instrument live (50 retired sets)PROTECT_FROMSPACE=1+DEPTH=800,VERIFY_EVACUATION=1,SCHEDULE_RATE=1)SCHEDULE_ALLOC_KB=0,FORCE_EVACUATE=1)cmp, both codegen arms on ONE pinned runtimeshapes−54.8% (71.4 → 32.3 MB),bigarr_move−58.8%,pipeline−17.2%;retain/retain_wide/deeplist/churn/interp/phase_flip(#7888's own RSS bound probe) all within ±0.1%cargo test --release -p perry-runtime --lib/-p perry-codegen --libnode_fail → parity_failin the same run, which is the oracle-shift tell. One IMPROVEMENT (test_gap_iterator_helpers_2874: parity_fail → pass)gc_root_dominance_corpus.sh+ checkerbench/idxset_recv.tssemantics probeindex === length(extend), sparse extend with holes, negative index, frozen array (throws), an array index carrying an accessor descriptor, a plain-object receiver, a raw-f64 downgrade, and an OOB read fed back into an in-bounds storearena_alloc_gcto the flat threshold fails the new test★
shapesnow retires only one from-space set (it has one minor left), so it is thinstress coverage by construction. The evidence for the newly-movable band comes from a probe
built for it:
bench/bigarr_move.tskeeps a rolling window of eight 2000/3000-elementarrays so they genuinely survive and are evacuated, then reads and checksums every
element of every survivor — a stale from-space read is an observable wrong answer. Output
20600072000 8on both arms and node; GC 222.9 ms → 11.0 ms; wall 0.34 → 0.10(node 0.13).
The footprint trade, stated rather than hidden
bench/bigarr_live.tsis the adversarial case, built to find the price: 600 arrays × 2500elements — all in the newly-nursery-resident band, all retained to the end, so every
one now transits Eden/survivor instead of being born in place.
Faster, not slower — and a real footprint cost in the all-survive case. Note the price is not the
choice of 128 KB — anything that fixes
shapesmust admit a 16.4 KB array to the nursery,so it admits a 20 KB one too. What it buys is that the footprint is now bounded and
collectable instead of unbounded: under the old policy those bytes, and everything they
named, were held until a full mark-sweep that many programs never run.
Summary by CodeRabbit
Performance
Bug Fixes
Tests