From fd018181ce447399cef1e92e536f101c6006cf34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 25 Aug 2026 11:34:28 +0200 Subject: [PATCH 1/3] perf(map): repair ordered-delete indexes in place --- crates/perry-runtime/src/gc/barrier/mod.rs | 2 +- crates/perry-runtime/src/map.rs | 225 ++++++++++++++++----- crates/perry-runtime/src/set.rs | 4 +- 3 files changed, 174 insertions(+), 57 deletions(-) diff --git a/crates/perry-runtime/src/gc/barrier/mod.rs b/crates/perry-runtime/src/gc/barrier/mod.rs index 067becd4fa..005edd4091 100644 --- a/crates/perry-runtime/src/gc/barrier/mod.rs +++ b/crates/perry-runtime/src/gc/barrier/mod.rs @@ -1932,7 +1932,7 @@ pub(crate) fn runtime_store_external_jsvalue_slot_with_layout( runtime_write_barrier_external_slot(parent_user, slot_addr, value_bits); } -pub(crate) fn runtime_dirty_external_slot_span( +pub(crate) fn runtime_write_barrier_external_slot_span( parent_addr: usize, first_slot_addr: usize, slot_count: usize, diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index d49184a77f..96133f40b7 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -1365,7 +1365,7 @@ unsafe fn map_set_string_key_value( let size = (*map).size; let entries = entries_ptr_mut(map); if grew && size > 0 { - crate::gc::runtime_dirty_external_slot_span( + crate::gc::runtime_write_barrier_external_slot_span( map as usize, entries as usize, size as usize * 2, @@ -1474,7 +1474,7 @@ fn map_set_resolved(map: *mut MapHeader, key: f64, value: f64) { let size = (*map).size; let entries = entries_ptr_mut(map); if grew && size > 0 { - crate::gc::runtime_dirty_external_slot_span( + crate::gc::runtime_write_barrier_external_slot_span( map as usize, entries as usize, size as usize * 2, @@ -1843,79 +1843,92 @@ unsafe fn delete_entry_at_index(map: *mut MapHeader, idx: i32) -> i32 { return 0; } let entries = entries_ptr_mut(map); + let deleted_key = ptr::read(entries.add(idx * 2)); // #2831: preserve insertion order. JS Map iteration must keep the // relative order of surviving entries after a delete (and a // delete-then-re-add appends at the end). The previous swap-and-pop - // moved the last entry into the hole, reordering iteration. Shift - // every entry after `idx` down by one slot instead. - for i in idx..(size as usize - 1) { - let next_key = ptr::read(entries.add((i + 1) * 2)); - let next_value = ptr::read(entries.add((i + 1) * 2 + 1)); - // GC_STORE_AUDIT(EXTERNAL_BARRIERED): map compaction slots use the shared external-slot helper. - crate::gc::runtime_store_external_jsvalue_slot( - map as usize, - entries.add(i * 2) as usize, - next_key.to_bits(), + // moved the last entry into the hole, reordering iteration. Compact the + // already-owned key/value pairs with one overlap-safe move. This does not + // create a new parent -> child edge: every copied value was already in + // this Map. The span mark preserves the old -> young remembered-set + // contract for the slots' new addresses without paying two full runtime + // stores per entry. + let moved_entries = size as usize - idx - 1; + if moved_entries > 0 { + // GC_STORE_AUDIT(EXTERNAL_BARRIERED): ordered compaction is followed by a dirty-span barrier for every moved slot. + ptr::copy( + entries.add((idx + 1) * 2), + entries.add(idx * 2), + moved_entries * 2, ); - crate::gc::runtime_store_external_jsvalue_slot( + crate::gc::runtime_write_barrier_external_slot_span( map as usize, - entries.add(i * 2 + 1) as usize, - next_value.to_bits(), + entries.add(idx * 2) as usize, + moved_entries * 2, ); } (*map).size = size - 1; - // The shift changes the entry index of every surviving key at or - // after `idx`, so the O(1) lookup side-tables can't be patched in - // place cheaply. Rebuild them from the compacted buffer. - rebuild_map_index(map); + // The old implementation rebuilt all three indexes from the entries + // buffer after every ordered delete. Repair their existing u32 offsets + // in place instead: removing one key and decrementing later offsets is a + // cache-linear pass over index values and does not re-hash surviving keys. + repair_map_indices_after_ordered_delete(map, deleted_key, idx as u32); 1 } -/// Rebuild the numeric + string lookup side-tables for `map` from its -/// current compacted entries buffer. Used after an order-preserving -/// `delete` shifts entry indexes (#2831). -unsafe fn rebuild_map_index(map: *mut MapHeader) { - if map.is_null() { - return; - } - let size = (*map).size as usize; - let capacity = (*map).capacity as usize; - if size > capacity || size > 16_000_000 || (*map).entries.is_null() { - return; - } - let entries = entries_ptr(map); - MAP_INDEX.with(|idx| { - let mut idx = idx.borrow_mut(); - let slot = idx - .entry(map as usize) - .or_insert_with(crate::fast_hash::new_ptr_hash_map); - slot.clear(); - for i in 0..size { - let key_bits = ptr::read(entries.add(i * 2)).to_bits(); - if is_safe_numeric_key(key_bits) { - slot.insert(NumericKey(key_bits), i as u32); +unsafe fn repair_map_indices_after_ordered_delete( + map: *mut MapHeader, + deleted_key: f64, + deleted_idx: u32, +) { + let map_addr = map as usize; + let deleted_bits = deleted_key.to_bits(); + + MAP_INDEX.with(|indexes| { + let mut indexes = indexes.borrow_mut(); + if let Some(index) = indexes.get_mut(&map_addr) { + if is_safe_numeric_key(deleted_bits) { + index.remove(&NumericKey(deleted_bits)); + } + for entry_idx in index.values_mut() { + if *entry_idx > deleted_idx { + *entry_idx -= 1; + } } } }); - MAP_STRING_INDEX.with(|idx| { - let mut idx = idx.borrow_mut(); - let slot = idx - .entry(map as usize) - .or_insert_with(std::collections::HashMap::new); - slot.clear(); - for i in 0..size { - let key_bits = ptr::read(entries.add(i * 2)).to_bits(); - if is_string_like(key_bits) { - if let Some(h) = string_content_hash(key_bits) { - slot.entry(h).or_insert_with(Vec::new).push(i as u32); + + MAP_STRING_INDEX.with(|indexes| { + let mut indexes = indexes.borrow_mut(); + if let Some(index) = indexes.get_mut(&map_addr) { + for bucket in index.values_mut() { + bucket.retain(|entry_idx| *entry_idx != deleted_idx); + for entry_idx in bucket { + if *entry_idx > deleted_idx { + *entry_idx -= 1; + } + } + } + index.retain(|_, bucket| !bucket.is_empty()); + } + }); + + MAP_PTR_INDEX.with(|indexes| { + let mut indexes = indexes.borrow_mut(); + if let Some(index) = indexes.get_mut(&map_addr) { + if is_ptr_index_key(deleted_bits) { + index.remove(&MapPtrKey(deleted_key)); + } + for entry_idx in index.values_mut() { + if *entry_idx > deleted_idx { + *entry_idx -= 1; } } } }); - rebuild_map_ptr_index(map); } /// Rebuild ONLY the pointer-key index for `map` from its current entries @@ -2687,4 +2700,108 @@ mod tests { assert_eq!(js_map_delete_number_key(map, boxed_string_key), 1); assert_eq!(js_map_has(map, boxed_string_key), 0); } + + #[test] + fn ordered_delete_repairs_mixed_side_indexes_and_preserves_order() { + let map = js_map_alloc(32); + let string_keys = (0..12) + .map(|i| { + let bytes = format!("key-{i:02}").into_bytes(); + js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) + }) + .collect::>(); + + for (i, string_key) in string_keys.iter().copied().enumerate() { + js_map_set(map, i as f64, (i * 10) as f64); + js_map_set_string_number(map, string_key, (i * 10 + 1) as f64); + } + // Keep the backing allocations alive while using their tagged + // addresses as identity keys. They deliberately are not GC objects: + // this exercises the pointer-key index without introducing an + // allocation/collection point into the ordered-delete fixture. + let pointer_owners = (0..4).map(Box::new).collect::>(); + let pointer_keys = pointer_owners + .iter() + .map(|owner| { + f64::from_bits( + crate::value::POINTER_TAG + | ((owner.as_ref() as *const i32 as u64) & crate::value::POINTER_MASK), + ) + }) + .collect::>(); + for (i, key) in pointer_keys.iter().copied().enumerate() { + js_map_set(map, key, (1_000 + i) as f64); + } + assert_eq!(js_map_size(map), 28); + + assert_eq!(js_map_delete_number_key(map, 2.0), 1); + assert_eq!(js_map_delete_string_key(map, string_keys[4]), 1); + assert_eq!(js_map_delete(map, pointer_keys[1]), 1); + assert_eq!(js_map_size(map), 25); + assert_eq!(js_map_has_number_key(map, 2.0), 0); + assert_eq!(js_map_has_string_key(map, string_keys[4]), 0); + assert_eq!(js_map_has(map, pointer_keys[1]), 0); + + for (i, string_key) in string_keys.iter().copied().enumerate() { + if i != 2 { + assert_eq!(js_map_get_number_key(map, i as f64), (i * 10) as f64); + assert!(test_map_numeric_index_contains(map, i as f64)); + } + if i != 4 { + assert_eq!(js_map_get_string_key(map, string_key), (i * 10 + 1) as f64); + assert!(test_map_string_index_contains( + map, + boxed_heap_string_key(string_key) + )); + } + } + for (i, key) in pointer_keys.iter().copied().enumerate() { + if i != 1 { + assert_eq!(js_map_get(map, key), (1_000 + i) as f64); + assert!(test_map_ptr_index_contains(map, key)); + } + } + + let mut expected_keys = (0..12) + .flat_map(|i| { + let mut keys = Vec::new(); + if i != 2 { + keys.push((i as f64).to_bits()); + } + if i != 4 { + keys.push(boxed_heap_string_key(string_keys[i]).to_bits()); + } + keys + }) + .collect::>(); + expected_keys.extend( + pointer_keys + .iter() + .enumerate() + .filter(|(i, _)| *i != 1) + .map(|(_, key)| key.to_bits()), + ); + let actual_keys = (0..js_map_size(map)) + .map(|i| js_map_entry_key_at(map, i).to_bits()) + .collect::>(); + assert_eq!( + actual_keys, expected_keys, + "delete must preserve survivor order" + ); + + js_map_set_number_key(map, 2.0, 222.0); + js_map_set_string_number(map, string_keys[4], 444.0); + js_map_set(map, pointer_keys[1], 1_111.0); + assert_eq!(js_map_size(map), 28); + assert_eq!(js_map_entry_key_at(map, 25).to_bits(), 2.0f64.to_bits()); + assert_eq!( + js_map_entry_key_at(map, 26).to_bits(), + boxed_heap_string_key(string_keys[4]).to_bits(), + "delete-then-re-add must append at the end" + ); + assert_eq!( + js_map_entry_key_at(map, 27).to_bits(), + pointer_keys[1].to_bits() + ); + } } diff --git a/crates/perry-runtime/src/set.rs b/crates/perry-runtime/src/set.rs index 8d381f900c..f83afc1169 100644 --- a/crates/perry-runtime/src/set.rs +++ b/crates/perry-runtime/src/set.rs @@ -988,7 +988,7 @@ fn set_add_resolved(set: *mut SetHeader, value: f64) { let size = (*set).size; let elements = elements_ptr_mut(set); if grew && size > 0 { - crate::gc::runtime_dirty_external_slot_span( + crate::gc::runtime_write_barrier_external_slot_span( set as usize, elements as usize, size as usize, @@ -1045,7 +1045,7 @@ fn set_add_string_resolved(set: *mut SetHeader, value: *const StringHeader) { let size = (*set).size; let elements = elements_ptr_mut(set); if grew && size > 0 { - crate::gc::runtime_dirty_external_slot_span( + crate::gc::runtime_write_barrier_external_slot_span( set as usize, elements as usize, size as usize, From 990d1f4f1a8693e414dcd001c2ad45c3c01364e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 25 Aug 2026 11:35:18 +0200 Subject: [PATCH 2/3] chore: add changelog for map delete optimization --- changelog.d/8813-map-ordered-delete.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8813-map-ordered-delete.md diff --git a/changelog.d/8813-map-ordered-delete.md b/changelog.d/8813-map-ordered-delete.md new file mode 100644 index 0000000000..392760d82c --- /dev/null +++ b/changelog.d/8813-map-ordered-delete.md @@ -0,0 +1 @@ +Ordered `Map.delete` now compacts surviving entries with one overlap-safe move and repairs numeric, string, and pointer side-index offsets in place instead of barrier-storing and rehashing every survivor. Insertion order, SameValueZero lookup, delete-then-readd ordering, moving-GC pointer-index rebuilds, and old-to-young external-slot tracking are preserved. From 2426ebf7b94779b36df27a3f950d2500298b3dfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 25 Aug 2026 12:43:57 +0200 Subject: [PATCH 3/3] test(map): root ordered-delete string keys --- crates/perry-runtime/src/map.rs | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 96133f40b7..84352f507b 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -2704,15 +2704,26 @@ mod tests { #[test] fn ordered_delete_repairs_mixed_side_indexes_and_preserves_order() { let map = js_map_alloc(32); + let scope = crate::gc::RuntimeHandleScope::new(); let string_keys = (0..12) .map(|i| { let bytes = format!("key-{i:02}").into_bytes(); - js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) + scope.root_nanbox_f64(boxed_heap_string_key(js_string_from_bytes( + bytes.as_ptr(), + bytes.len() as u32, + ))) }) .collect::>(); - for (i, string_key) in string_keys.iter().copied().enumerate() { + let string_key_ptr = |i: usize| { + (string_keys[i].get_nanbox_f64().to_bits() & crate::value::POINTER_MASK) + as *const StringHeader + }; + + for (i, string_key) in string_keys.iter().enumerate() { js_map_set(map, i as f64, (i * 10) as f64); + let string_key = (string_key.get_nanbox_f64().to_bits() & crate::value::POINTER_MASK) + as *const StringHeader; js_map_set_string_number(map, string_key, (i * 10 + 1) as f64); } // Keep the backing allocations alive while using their tagged @@ -2735,19 +2746,22 @@ mod tests { assert_eq!(js_map_size(map), 28); assert_eq!(js_map_delete_number_key(map, 2.0), 1); - assert_eq!(js_map_delete_string_key(map, string_keys[4]), 1); + assert_eq!(js_map_delete_string_key(map, string_key_ptr(4)), 1); assert_eq!(js_map_delete(map, pointer_keys[1]), 1); assert_eq!(js_map_size(map), 25); assert_eq!(js_map_has_number_key(map, 2.0), 0); - assert_eq!(js_map_has_string_key(map, string_keys[4]), 0); + assert_eq!(js_map_has_string_key(map, string_key_ptr(4)), 0); assert_eq!(js_map_has(map, pointer_keys[1]), 0); - for (i, string_key) in string_keys.iter().copied().enumerate() { + for (i, string_key) in string_keys.iter().enumerate() { if i != 2 { assert_eq!(js_map_get_number_key(map, i as f64), (i * 10) as f64); assert!(test_map_numeric_index_contains(map, i as f64)); } if i != 4 { + let string_key = (string_key.get_nanbox_f64().to_bits() + & crate::value::POINTER_MASK) + as *const StringHeader; assert_eq!(js_map_get_string_key(map, string_key), (i * 10 + 1) as f64); assert!(test_map_string_index_contains( map, @@ -2769,7 +2783,7 @@ mod tests { keys.push((i as f64).to_bits()); } if i != 4 { - keys.push(boxed_heap_string_key(string_keys[i]).to_bits()); + keys.push(string_keys[i].get_nanbox_f64().to_bits()); } keys }) @@ -2790,13 +2804,13 @@ mod tests { ); js_map_set_number_key(map, 2.0, 222.0); - js_map_set_string_number(map, string_keys[4], 444.0); + js_map_set_string_number(map, string_key_ptr(4), 444.0); js_map_set(map, pointer_keys[1], 1_111.0); assert_eq!(js_map_size(map), 28); assert_eq!(js_map_entry_key_at(map, 25).to_bits(), 2.0f64.to_bits()); assert_eq!( js_map_entry_key_at(map, 26).to_bits(), - boxed_heap_string_key(string_keys[4]).to_bits(), + string_keys[4].get_nanbox_f64().to_bits(), "delete-then-re-add must append at the end" ); assert_eq!(