perf(gc): make the shadow-stack runtime ops cheap - #7079
Conversation
…arrier Replace the three parallel Vecs (values / slot_ptrs / active bits) behind the shadow stack with one Vec of 16-byte entries, pack the frame header into a single entry, and gate the per-store incremental-mark root barrier on PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT. Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9
LLVM re-forms every length-dependent zero-fill into a memset call, and tail-merges even a constant-size store with a neighbouring variable one. Outline the large-frame path so the common frame keeps its inline stp q0, q0 pair. Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe shadow stack now uses packed entries instead of parallel vectors. Frame operations, slot binding, liveness tracking, root scanning, savepoint restoration, and barrier behavior were updated, with extensive tests covering GC forwarding, stale-state prevention, frame chaining, and malformed handles. ChangesShadow-stack runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
MeasurementHost Raspberry Pi 5, Cortex-A76 @ 2.400 GHz, governor Both arms are the same pinned A/B: base vs opt (14 reps/arm, 224 runs, 0 dropped, 0 oracle mismatches)
Instruction-count cv is 0.00 % on every workload above 100 M instructions. The ceiling, re-measured rather than inferredSame experiment as the original ceiling run (
The two ceilings are comparable because the OFF arm is the same program in both:
The ON arms reproduce equally well: this session's ceiling ON counts agree with What that means for the stack-map questionThe cost profile is not uniform, and the shape of what is left is the useful
So: a stack-map redesign has considerably less to win on recursive pointer code Sanity check on the mechanism
Static, from the shipped
|
| symbol | before | after |
|---|---|---|
js_shadow_frame_push |
159 instructions, up to 3 memset calls on the fast path |
65, 0 calls on the fast path |
js_shadow_frame_pop |
44 | 36 |
js_shadow_slot_set |
61, plus a barrier call that did its own TLS lookup | 58, barrier inline-gated, 0 calls |
js_shadow_slot_bind |
62, same barrier call | 51, whole entry written with one stp, 0 calls |
Every bl remaining in these four symbols is on a cold or panic path.
Wall time
Directional only — wall cv is 2–13 % on this host. Medians: w3 2.625 → 2.147 s,
w1 3.163 → 2.849 s, w4 3.080 → 2.919 s, w6 2.373 → 2.294 s.
w2_allocring is the one arm where wall moved the wrong way (median −6.6 % in
the opt arm) while its instruction count improved 1.8 %. Its paired spread
crosses zero ([−14.8 %, +11.3 %]) and its backend-stall counter is the noisiest
in the set, so it is not a measured regression — but I could not attribute it
either way and record it rather than explain it away.
Gates
scripts/gc_repsel_matrix.sh --arms all --pressure 8, node 26.5.0:
PASS=339 UNVER=100 XFAIL=1 FAIL=0 over 440 cells, byte-exact 439/440 — the
required baseline, unchanged.
gc-ratchet: OK. More usefully, I built a pristine-main reference
runtime in the same worktree, on the same host, and measured it the same way:
all 72 gated retention and evacuation counters are bit-identical between
pristine main and this branch. (The small copied_objects movements the
checker reports against the pinned baseline artifact — ±0.02 % on 5 of 8 probes
— are present identically on pristine main, so they are pre-existing drift
from intervening commits, not this change.)
cargo test -p perry-runtime --lib: 1544 pass, 0 fail single-threaded;
gc:: 522/522. In parallel the run drops 2–3 tests
(gc::tests::copying::large_object_old_born_array_slot_write_keeps_young_child_alive,
gc::tests::teardown::map_set_side_allocations_release_*,
object::tests::closure_name_and_length_ignore_plain_assignment), with an
unstable failing set. I verified the same tests fail in a parallel run of a
pristine main checkout in this worktree, so they are the known
macOS parallel-ordering flake family, not this change.
Sabotage verification
Nine sabotages, each reverted after. Every one is caught.
| # | Sabotage | Result |
|---|---|---|
| S1 | delete the clear_slots call in js_shadow_frame_push |
3 named tests FAIL; the full gc:: suite SIGSEGVs |
| S2 | clear only ShadowEntry::value, leave meta (binding + liveness bit) |
same 3 FAIL, incl. fresh_frame_does_not_inherit_popped_frame_bindings |
| S3 | meta = 0 instead of meta & SLOT_PTR_MASK on the clear path |
clearing_a_bound_slot_keeps_the_binding_for_later_reactivation FAIL |
| S4 | bound_slot_meta returns raw | SLOT_ACTIVE (truncates a misaligned address) |
bound_meta_encoding_rejects_addresses_that_would_clobber_the_liveness_bit FAIL |
| S5 | scanner always hands out the mirror, never the bound pointer | 3 FAIL, incl. bound_slot_survives_and_is_rewritten_by_a_copying_minor |
| S6 | invert the barrier gate to == 0 (never shades) |
slot_set_and_bind_shade_the_stored_value_while_a_cycle_is_active FAIL |
| S7 | scanner ignores the liveness bit | 4 FAIL |
| S8 | swap the packed header fields (prev_frame_top ↔ slot_count) |
SIGSEGV |
| S9 | remove the js_shadow_frame_pop bounds guard |
SIGSEGV |
Not verified
- The Linux/aarch64 disassembly. The static instruction counts above are
macOS/aarch64 (tlv_get_addrTLS); the Pi binaries are stripped, so I read
the Linux result dynamically (instruction counts) rather than statically. w2_allocring's wall-time direction, as above.- Non-aarch64 targets. No x86-64 or Windows measurement was taken; the
change is portable Rust and the gates that ran are aarch64 macOS + aarch64
Linux.
|
Matrix re-confirmed on the final commit ( The Pi instruction-count measurement was taken on that same final commit, not |
A ceiling measurement put the shadow stack at +71.5 % instructions on
application-shaped code (
w6_records, Pi 5) versus not emitting it at all —roughly 65–70 instructions per shadow op — and named two implementation costs
rather than anything inherent to shadow stacks. This PR removes both, without
changing the root-set contract.
What changed
1.
js_shadow_frame_pushdid threeVec::resizecalls per activationThe per-slot state lived in three parallel thread-local
Vecs —stack: Vec<u64>(mirrored values),slot_ptrs: Vec<usize>(bindings to compiledlocals),
active: Vec<bool>(liveness bits) — and the frame header took twou64words. One frame push therefore cost five capacity checks, three lengthupdates, and, read out of the shipped archive's disassembly, up to three
memsetcalls for the 2–4-slot frames codegen actually emits.The three words are now one 16-byte
ShadowEntry { value: u64, meta: usize },the header packs
prev_frame_topandslot_countinto a single entry(
SHADOW_STACK_HEADER_SLOTS2 → 1), and the slot clear is a constant-sizestore.
aarch64release disassembly, fast path,libperry_runtime.a:js_shadow_frame_pushbl memsetmovi+ 2 ×stp q0, q0, no calljs_shadow_slot_setVecheaders,blroot barrier (which does its own TLS lookup)ldar+cbnz), no calljs_shadow_slot_bindstp, no callKeeping the clear call-free took three attempts, each verified by
disassembling the linked archive rather than assumed:
matchon the slot count with a spelled-out arm per size → LLVM re-formedit into a compare chain computing a byte count and
bl memset;write_bytesfor wide frames→ LLVM tail-merged the two into one
csel-the-length-then-bl memset;#[cold] #[inline(never)]→movi.2d v0, #0+stp q0, q0× 2. Shipped.2.
js_shadow_slot_set/js_shadow_slot_bindcalled the root write barrier on every storeIt is still called — it is now gated. The barrier is the incremental-mark
root shading barrier, not the generational remembered-set barrier: old→young
edges are logged by the heap-slot barriers (
runtime_write_barrier_slotandfriends), which this PR does not touch. Its whole body is
incremental_mark_barrier_value, which reads the thread-localINCREMENTAL_MARK_BARRIER_VALID_PTRSand returns immediately when it is null —so on the overwhelmingly common path the cost was a call plus a second TLS
lookup to learn there was nothing to do.
It is now gated inline on
PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT, thesame guard codegen already emits around
js_write_barrier_root_nanboxforpersistent shadow slots (
expr/shadow_slot.rs::emit_persistent_shadow_root_barrier).The gate is observationally identical, not a narrowing.
incremental_mark_barrier_enableinstalls the thread-local and thenincrements the count, both before returning to the mutator, so on any thread
"this thread's
VALID_PTRSis non-null ⟹ the count is ≥ 1". A zero counttherefore proves this thread's pointer is null, i.e. proves the call would
have returned
falsehaving done nothing. A non-zero count is conservative inthe harmless direction: another thread's cycle makes us take the call, which
then observes its own null pointer and returns.
Soundness, per property
Liveness — an active entry is still visited by
visit_shadow_stack_root_slotsand still marked. The liveness bit moved fromactive: Vec<bool>into bit 0 ofmeta; a compiled local slot is ani64/doublealloca and so 8-byte aligned, leaving bit 0 free. An addressthat would collide with the tag is recorded active-but-unbound rather than
truncated — the mirrored value is still marked and still rewritten, and the
collector is never handed a mis-derived address to write a forwarded pointer
into.
Rewritability — unchanged in kind. A bound entry still exposes the compiled
local's address to the visitor, so an evacuating collection rewrites the
storage the mutator reads after the safepoint. An unbound entry exposes the
mirror word itself;
ShadowEntryis#[repr(C)]withvalueat offset 0, sothat is the same correctly-aligned
*mut u64the parallel-Veclayout handedout.
The value the mutator stored —
js_shadow_slot_bindstill snapshots*value_slotonce, at the call, and roots that word. Nothing re-reads a slot ata safepoint.
No stale roots — the zero-fill is load-bearing, not hygiene: the buffer past
the current length still holds the popped frame's entries, so a frame that
inherited them would report dead values and, worse, a live binding into a
stack frame that has since returned. Clearing
metamatters as much as clearingvalue.Also hardened while here: the malformed-
frame_handleguard injs_shadow_frame_popwasbase + HEADER_SLOTS > len, which wraps for a handlenear
usize::MAXand lets exactly the corrupted handles it exists for slipthrough into an unchecked read. It is now
base >= len.Tests
New
gc::tests::shadow_stack_ops(13 tests) covers the four properties above.Every test was verified to fail under a targeted sabotage of the thing it
covers — see the table in the PR discussion.
Verification
See the PR discussion for the measured instruction counts, matrix, and ratchet
results.
Summary by CodeRabbit
Performance Improvements
Bug Fixes
Tests