Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions changelog.d/9020-map-tombstone-delete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
Ordered `Map` deletes are O(1) instead of O(N).

Emptying an N-entry Map was O(N²) three ways at once: every delete memmoved the
trailing entries down, span-barriered every moved slot, and walked all three
side indexes decrementing every offset above the hole. Per-delete cost doubled
with N (0.96 µs at N=2k to 8.2 µs at N=16k) where node is flat at ~0.03 µs —
263× at N=16k. On the ECS archetype-migration row, `delete_entry_at_index` and
its memmove were ~14% of the frame.

A delete now tombstones in place: the key slot takes a reserved hole marker, the
value slot is cleared through the barriered store so SATB marking still shades
the overwritten child, and the live `size` drops while the array extent (`used`,
a new `MapHeader` field) stays put. Raw entry indices are therefore stable — no
shifting, no span barrier over the tail, and the side indexes only forget the
deleted key, so the offset-repair walkers are deleted outright.

The hole marker can never collide with a stored key: `normalize_zero`
canonicalizes it to `undefined` on the resolved-key path, the string-key path
writes a STRING_TAG-boxed pointer that cannot equal it, and compaction only
moves keys that were already normalized on insert.

Insertion order is unchanged: iteration walks raw indices and skips holes, and
delete-then-re-add still appends. Compaction runs when tombstones outnumber live
entries, before growing, or when a raw-indexed accessor observes holes — after
which the typed `for…of` lane's `used == size` admission holds again, so the
codegen lane self-heals rather than misreading holes.

The GC contract bounds the Map slot descriptor's range by `used`, with
`size ≤ used ≤ capacity` as the corruption guard; holes are non-pointer markers
the tag-filtered scan skips.
48 changes: 47 additions & 1 deletion crates/perry-codegen/src/expr/arrays_finds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ use super::index_get::numeric_index_has_integer_array_index_proof;
/// other shape — a subclass instance, a plain object, an out-of-range or
/// negative index, an unpublished handle — takes the runtime helper exactly
/// as before, so the two paths are equivalent by construction.
/// Byte offset of `MapHeader::used`, which the tombstoned-delete lane loads to
/// check `used == size` (no holes) before admitting a raw entry read.
///
/// `perry-codegen` does not depend on `perry-runtime`, so nothing binds this
/// literal to the struct it describes. The runtime pins the offset with an
/// `offset_of!` assertion, which catches a field REORDER — but the natural fix
/// for that assertion is to update its expected value, which leaves this string
/// stale and still compiling, and generated code would then load the wrong word
/// of the header. `map_header_used_offset_is_what_codegen_assumes` ties the two
/// together by reading the runtime source, exactly as `hot_tls.rs` does.
const MAP_HEADER_USED_OFFSET: &str = "32";

fn lower_map_entry_at_inline(
ctx: &mut FnCtx<'_>,
m_box: &str,
Expand Down Expand Up @@ -146,9 +158,18 @@ fn lower_map_entry_at_inline(
let live = blk.icmp_eq(I8, &forwarded, "0");
let size_ptr = blk.inttoptr(I64, &m_handle);
let size = blk.load(I32, &size_ptr);
// Tombstoned deletes leave `used > size`; a raw entry read is only
// dense-correct with no holes present, so a holey map falls back to
// the runtime helper — which compacts, after which this admission
// holds again (the lane self-heals).
let used_addr = blk.add(I64, &m_handle, MAP_HEADER_USED_OFFSET);
let used_ptr = blk.inttoptr(I64, &used_addr);
let used = blk.load(I32, &used_ptr);
let dense = blk.icmp_eq(I32, &used, &size);
let in_range = blk.icmp_ult(I32, &i_i32, &size);
let a = blk.and(I1, &is_map, &live);
let admitted = blk.and(I1, &a, &in_range);
let b = blk.and(I1, &a, &dense);
let admitted = blk.and(I1, &b, &in_range);
blk.cond_br(&admitted, &fast_label, &slow_label);
}
ctx.current_block = fast_idx;
Expand Down Expand Up @@ -1422,3 +1443,28 @@ pub(crate) fn lower(
_ => unreachable!("expr/mod.rs dispatched a variant not handled by this submodule"),
}
}

#[cfg(test)]
mod map_header_layout_tests {
/// The offset the runtime pins with `offset_of!(MapHeader, used) == N`.
fn runtime_map_used_offset() -> usize {
let src = include_str!("../../../perry-runtime/src/map.rs");
let needle = "offset_of!(MapHeader, used) == ";
let rest = src
.split_once(needle)
.expect("MapHeader::used offset assertion not found in map.rs - was it renamed?")
.1;
let digits: String = rest.chars().take_while(char::is_ascii_digit).collect();
digits.parse().expect("offset is a decimal literal")
}

#[test]
fn map_header_used_offset_is_what_codegen_assumes() {
assert_eq!(
super::MAP_HEADER_USED_OFFSET.parse::<usize>().unwrap(),
runtime_map_used_offset(),
"codegen emits a stale MapHeader::used offset; the tombstone lane \
would read the wrong header word and mis-admit holey maps",
);
}
}
24 changes: 16 additions & 8 deletions crates/perry-runtime/src/collection_iter_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,37 +265,45 @@ unsafe fn dispatch_map_iterator_method_emit(
}
let cursor = f64::from_bits(js_object_get_field(iter_obj(), 1).bits()) as u32;
let last_key = js_object_get_field(iter_obj(), 4);
let size = crate::map::js_map_size(map());
let used = crate::map::map_used_entries(map());
// Is the last-returned key still at cursor-1? (SameValueZero, so a
// NaN key matches itself.) If so, no delete shifted an entry at/below
// the cursor.
let in_place = cursor > 0 && {
let prev = crate::map::js_map_entry_key_at(map(), cursor - 1);
let prev = crate::map::map_entry_key_raw(map(), cursor - 1);
crate::value::js_jsvalue_same_value_zero(prev, f64::from_bits(last_key.bits())) != 0
};
let idx = next_read_index(cursor, in_place, || {
let mut idx = next_read_index(cursor, in_place, || {
crate::map::find_key_index(map(), f64::from_bits(last_key.bits()))
});
if idx >= size {
js_object_set_field(iter_obj(), 1, JSValue::number(size as f64));
// Tombstoned deletes leave holes in the raw entry order; the
// cursor walks raw indices, so step over them here.
while idx < used
&& crate::map::map_entry_key_raw(map(), idx).to_bits()
== crate::map::MAP_HOLE_KEY_BITS
{
idx += 1;
}
if idx >= used {
js_object_set_field(iter_obj(), 1, JSValue::number(used as f64));
// Once a collection iterator is exhausted it stays exhausted,
// even if entries are appended later.
js_object_set_field(iter_obj(), 0, JSValue::undefined());
return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true);
}

let entry_key = crate::map::js_map_entry_key_at(map(), idx);
let entry_key = crate::map::map_entry_key_raw(map(), idx);
// Record state for the next re-derive BEFORE any allocation below.
js_object_set_field(iter_obj(), 1, JSValue::number((idx + 1) as f64));
js_object_set_field(iter_obj(), 4, JSValue::from_bits(entry_key.to_bits()));

let value = match kind {
KIND_KEYS => JSValue::from_bits(entry_key.to_bits()),
KIND_VALUES => {
JSValue::from_bits(crate::map::js_map_entry_value_at(map(), idx).to_bits())
JSValue::from_bits(crate::map::map_entry_value_raw(map(), idx).to_bits())
}
_ => {
let val = crate::map::js_map_entry_value_at(map(), idx);
let val = crate::map::map_entry_value_raw(map(), idx);
JSValue::from_bits(make_pair_array(entry_key, val).to_bits())
}
};
Expand Down
5 changes: 3 additions & 2 deletions crates/perry-runtime/src/gc/layout_slot_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,15 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors(
GcRewriteDescriptorKind::Map => {
let map = user_ptr as *mut crate::map::MapHeader;
let size = (*map).size;
let used = (*map).used;
let capacity = (*map).capacity;
// Corruption guard only: mirror Set's 16M bound (set.rs
// gc_element_slot_range). Every GC walk (mark, copy, rewrite,
// dirty-scan, verify) funnels through this descriptor, so a
// lower cap makes larger maps invisible to the collector —
// entries reachable only through a >cap map would be swept
// while live and never rewritten after a move.
if size > capacity || size > 16_000_000 || (*map).entries.is_null() {
if size > used || used > capacity || used > 16_000_000 || (*map).entries.is_null() {
return;
}
// Defensive tripwire (# fabricated-Map): if a fabricated Map
Expand All @@ -248,7 +249,7 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors(
return;
}
visit(GcMutableSlotDescriptor::Range {
range: HeapSlotRange::new((*map).entries as *mut u64, size as usize * 2),
range: HeapSlotRange::new((*map).entries as *mut u64, used as usize * 2),
layout_kind: None,
});
// #6759 phase 1: the metadata edge. This arm is the MARK path as
Expand Down
8 changes: 8 additions & 0 deletions crates/perry-runtime/src/gc/tests/barrier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ unsafe fn alloc_old_test_map(
let entries = std::alloc::alloc_zeroed(layout) as *mut u64;
assert!(!entries.is_null());
(*map).size = 0;
(*map).used = 0;
(*map).capacity = capacity;
(*map).entries = entries as *mut f64;
(map, entries, layout)
Expand All @@ -26,6 +27,7 @@ unsafe fn retire_old_test_map(
layout: std::alloc::Layout,
) {
(*map).size = 0;
(*map).used = 0;
(*map).capacity = 0;
(*map).entries = std::ptr::null_mut();
std::alloc::dealloc(entries as *mut u8, layout);
Expand Down Expand Up @@ -625,6 +627,7 @@ fn test_old_young_edge_verifier_accepts_map_external_slot() {
let map_header = unsafe { header_from_user_ptr(map as *const u8) };
unsafe {
(*map).size = 1;
(*map).used = 1;
*entries = ptr_bits(young);
(*map_header).gc_flags |= GC_FLAG_MARKED;
}
Expand Down Expand Up @@ -932,6 +935,7 @@ fn test_dirty_page_map_entry_scan_is_external_range_bounded() {
let (map, entries, layout) = unsafe { alloc_old_test_map(2048) };
unsafe {
(*map).size = 2048;
(*map).used = 2048;
}
let (dirty_idx, clean_idx) = unsafe { field_indices_on_distinct_pages(entries, 4096) };
let dirty_slot = unsafe { entries.add(dirty_idx) };
Expand Down Expand Up @@ -1045,6 +1049,7 @@ fn test_dirty_page_map_external_dedupes_and_clears() {
let (map, entries, layout) = unsafe { alloc_old_test_map(16) };
unsafe {
(*map).size = 16;
(*map).used = 16;
*entries.add(1) = POINTER_TAG | young as u64;
}
let slot = unsafe { entries.add(1) };
Expand Down Expand Up @@ -1077,6 +1082,7 @@ fn test_dirty_page_map_realloc_span_marks_new_entries_pages() {
let (map, entries, layout) = unsafe { alloc_old_test_map(1024) };
unsafe {
(*map).size = 1024;
(*map).used = 1024;
*entries.add(1023) = POINTER_TAG | young as u64;
}
let new_layout = std::alloc::Layout::from_size_align(2048 * 16, 8).unwrap();
Expand Down Expand Up @@ -1462,6 +1468,7 @@ fn test_incremental_barrier_marks_external_map_and_set_slots() {
let (set, elements, set_layout) = unsafe { alloc_old_test_set(1) };
unsafe {
(*map).size = 1;
(*map).used = 1;
(*set).size = 1;
}
mark_user_ptr(map as usize);
Expand Down Expand Up @@ -1687,6 +1694,7 @@ fn test_rewrite_remembered_dirty_range_updates_map_external_entry_span() {
let (map, entries, layout) = unsafe { alloc_old_test_map(2048) };
unsafe {
(*map).size = 2048;
(*map).used = 2048;
}
let (dirty_idx, clean_idx) = unsafe { field_indices_on_distinct_pages(entries, 4096) };
let dirty_slot = unsafe { entries.add(dirty_idx) };
Expand Down
3 changes: 3 additions & 0 deletions crates/perry-runtime/src/gc/tests/helper_stores.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ unsafe fn alloc_old_test_map(
let entries = std::alloc::alloc_zeroed(layout) as *mut u64;
assert!(!entries.is_null());
(*map).size = 0;
(*map).used = 0;
(*map).capacity = capacity;
(*map).entries = entries as *mut f64;
(map, entries, layout)
Expand All @@ -33,6 +34,7 @@ unsafe fn retire_old_test_map(
layout: std::alloc::Layout,
) {
(*map).size = 0;
(*map).used = 0;
(*map).capacity = 0;
(*map).entries = std::ptr::null_mut();
std::alloc::dealloc(entries as *mut u8, layout);
Expand Down Expand Up @@ -102,6 +104,7 @@ fn map_and_set_external_helper_stores_preserve_young_children() {
let (map, entries, layout) = unsafe { alloc_old_test_map(1) };
unsafe {
(*map).size = 1;
(*map).used = 1;
crate::gc::runtime_store_external_jsvalue_slot(
map as usize,
entries as usize,
Expand Down
Loading
Loading