From ffa9e915af614e53af3fc1e586f98d63f9f938dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 14:01:29 +0200 Subject: [PATCH] perf(gc): skip the element-shape table when neither address of a move advertises a proof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `transfer_element_shape` runs for every relocated array and already decides `had_bit` from header words it has read anyway — then took the side table's `RefCell` and hashed both addresses regardless, for two removes that remove nothing whenever the source proved nothing and the destination advertises nothing. It now returns before the table in that case. The gate cannot be `!had_bit` alone: a destination still advertising a proof describes storage the move has just replaced, so that case keeps the full fail-closed path. Both halves are pinned by a new test, which fails on its named assertion if the gate is widened. Leaving a record behind at an address whose bit is clear is not a new state: the bit is the sole authority for a read, `establish` draws identities from `ELEMENT_SHAPE_PROOF_SEQ` rather than from the record at the address, and `prune_dead_element_shape_owners` drops it on the next collection — the same guarantees a fail-closed transfer already depended on. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- .../9808-element-shape-transfer-gate.md | 25 +++++++ .../perry-runtime/src/array/element_shape.rs | 29 +++++++++ .../src/array/element_shape_tests.rs | 65 +++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 changelog.d/9808-element-shape-transfer-gate.md diff --git a/changelog.d/9808-element-shape-transfer-gate.md b/changelog.d/9808-element-shape-transfer-gate.md new file mode 100644 index 0000000000..b7d3b19b3f --- /dev/null +++ b/changelog.d/9808-element-shape-transfer-gate.md @@ -0,0 +1,25 @@ +**A relocated array whose element-shape proof does not exist no longer takes +the side table to find that out** (#9792). + +`transfer_element_shape` runs from `layout_transfer` for every relocated +array — growth forwarding, a copying minor, an old-gen defrag. It already +computes `had_bit` for free from two header words it has read anyway, and +then took `ELEMENT_SHAPES`' `RefCell` and hashed both addresses regardless, +for two removes that on the overwhelmingly common path remove nothing. It now +returns when neither address advertises a proof. + +The gate has exactly one safe shape and both halves are pinned by +`a_transfer_skips_the_table_only_when_neither_address_advertises_a_proof`. +Skipping on `!had_bit` alone would be wrong: a destination that still +advertises a proof describes storage the move has just replaced, so that case +keeps the full fail-closed path. + +What skipping leaves behind is a record at an address whose bit is clear, and +that state was already part of the design rather than new: the bit is the sole +authority for a read (`element_shape_proof` returns `None` before touching the +table, and `note_element_store` is gated on the same bit), and `establish` +draws every identity from `ELEMENT_SHAPE_PROOF_SEQ` rather than from whatever +record sits at the address — precisely so a survivor cannot donate its epoch +to the next array proven there. `prune_dead_element_shape_owners` drops it on +the next collection, the same footprint-only guarantee a fail-closed transfer +already relied on. The test asserts both defences directly. diff --git a/crates/perry-runtime/src/array/element_shape.rs b/crates/perry-runtime/src/array/element_shape.rs index 0158dbad33..29f0f245cc 100644 --- a/crates/perry-runtime/src/array/element_shape.rs +++ b/crates/perry-runtime/src/array/element_shape.rs @@ -672,6 +672,25 @@ pub(crate) fn transfer_element_shape(old_user: usize, new_user: usize) { // exactly the versioning a consumer guards on. let had_bit = array_gc_header(old_user as *const ArrayHeader) .is_some_and(|old_header| header_has_bit(old_header)); + // #9792: neither address advertises a proof, so there is nothing to + // move and nothing to fail closed about — the `clear_bit` below would + // clear a bit that is already clear. Skipping is what the siblings in + // `gc::layout_tables` do with their emptiness flag, decided here from + // the header words this function has already read rather than from a + // side-table probe. + // + // What it leaves behind is a record at an address whose bit is clear, + // and that state is already part of the design: the bit is the sole + // authority for a read (`element_shape_proof` returns `None` without + // touching the table), and `establish` draws every identity from + // `ELEMENT_SHAPE_PROOF_SEQ` rather than from whatever record sits at + // the address, precisely so a survivor cannot donate its epoch to the + // next array established there. `prune_dead_element_shape_owners` + // drops it on the next collection, which is the same footprint-only + // guarantee a fail-closed transfer already relied on. + if !had_bit && !header_has_bit(new_header) { + return; + } let moved = ELEMENT_SHAPES.with(|m| { let mut map = m.borrow_mut(); map.remove(&new_user); @@ -795,6 +814,16 @@ pub(crate) unsafe fn test_element_shape_bit_set(arr: *const ArrayHeader) -> bool array_gc_header(arr).is_some_and(|header| header_has_bit(header)) } +/// Clear the advertising bit and leave the record in the table — the survivor +/// state a skipped [`transfer_element_shape`] produces, and the one the +/// bit-is-authority rule has to hold up under. +#[cfg(test)] +pub(crate) unsafe fn test_clear_element_shape_bit_only(arr: *mut ArrayHeader) { + if let Some(header) = array_gc_header(arr) { + clear_bit(header); + } +} + #[cfg(test)] #[path = "element_shape_tests.rs"] mod tests; diff --git a/crates/perry-runtime/src/array/element_shape_tests.rs b/crates/perry-runtime/src/array/element_shape_tests.rs index 80199a91a0..fa93b4f76c 100644 --- a/crates/perry-runtime/src/array/element_shape_tests.rs +++ b/crates/perry-runtime/src/array/element_shape_tests.rs @@ -541,6 +541,71 @@ fn a_fail_closed_transfer_leaves_no_record_for_the_next_array_to_inherit() { ); } +#[test] +fn a_transfer_skips_the_table_only_when_neither_address_advertises_a_proof() { + // #9792: `transfer_element_shape` runs for every relocated array, and its + // `had_bit` verdict is free — two header words it has already read. When + // neither address advertises a proof it takes the side table anyway, for + // two removes that can only remove what no reader could reach. The gate + // that skips that has exactly ONE safe shape, and this pins both halves. + // + // Arm 1 — neither bit set: skipping is correct, and the record it leaves + // behind stays unreadable and cannot donate its identity. + // Arm 2 — the DESTINATION still advertises one: skipping would leave a + // live proof describing storage that has just been overwritten, so the + // gate must NOT fire on `!had_bit` alone. + let _serialized = test_serialize(); + + // Arm 1. + let src = built_from_pushes(CLASS_A, 2); + let dst = built_from_pushes(CLASS_A, 2); + let survivor = proof(dst).expect("proven").epoch; + unsafe { clear_element_shape(src) }; + unsafe { test_clear_element_shape_bit_only(dst) }; + assert!( + test_element_shape_record_exists(dst as usize), + "the fixture must actually leave a record behind the cleared bit" + ); + + transfer_element_shape(src as usize, dst as usize); + + assert!( + proof(dst).is_none(), + "the bit is the sole authority for a read, so a record behind a \ + cleared bit must not read as a proof" + ); + let reproven = unsafe { ensure_element_shape(dst) }.expect("still homogeneous"); + assert_ne!( + reproven.epoch, survivor, + "establishing draws a fresh identity from ELEMENT_SHAPE_PROOF_SEQ, so \ + a survivor record can never donate its epoch" + ); + + // Arm 2: source proves nothing, destination still advertises a proof. + let src2 = built_from_pushes(CLASS_A, 2); + let dst2 = built_from_pushes(CLASS_A, 2); + unsafe { clear_element_shape(src2) }; + assert!( + unsafe { test_element_shape_bit_set(dst2) }, + "the fixture must leave the destination advertising a proof" + ); + + transfer_element_shape(src2 as usize, dst2 as usize); + + unsafe { + assert!( + !test_element_shape_bit_set(dst2), + "a transfer whose source proved nothing must still fail the \ + destination closed — gating on `!had_bit` alone would leave a \ + live proof over storage the move has just replaced" + ); + } + assert!( + !test_element_shape_record_exists(dst2 as usize), + "and it must take the destination's record with it" + ); +} + // --------------------------------------------------------------------------- // Lifecycle hooks — what stops a recycled address inheriting a stale identity // ---------------------------------------------------------------------------