diff --git a/changelog.d/9082-map-set-foreach-compaction.md b/changelog.d/9082-map-set-foreach-compaction.md new file mode 100644 index 0000000000..722921042a --- /dev/null +++ b/changelog.d/9082-map-set-foreach-compaction.md @@ -0,0 +1,2 @@ +`Map.prototype.forEach` and `Set.prototype.forEach` now continue visiting live +entries when callback-side deletes cross the backing-store compaction threshold. diff --git a/crates/perry-runtime/src/exception.rs b/crates/perry-runtime/src/exception.rs index 745311e754..55fd91d70a 100644 --- a/crates/perry-runtime/src/exception.rs +++ b/crates/perry-runtime/src/exception.rs @@ -114,6 +114,10 @@ struct ExceptionState { /// `call_method_depth_*`). Indexed by try-depth, in lockstep with /// `jump_buffers`. call_method_depths: Box<[u32]>, + /// Active Set/Map `forEach` walks. Their normal epilogues re-enable + /// backing-store compaction, but a caught throw skips those epilogues. + set_foreach_depths: Box<[usize]>, + map_foreach_depths: Box<[usize]>, /// Recorded-prototype lookup stack depth. A getter can throw while /// `resolve_inherited_field` is recursively walking; longjmp skips its /// guard drops, so restore the stack to this try-entry savepoint. @@ -157,6 +161,8 @@ impl ExceptionState { shadow_savepoints: vec![ShadowSavepoint::EMPTY; MAX_TRY_DEPTH].into_boxed_slice(), runtime_handle_savepoints: vec![0usize; MAX_TRY_DEPTH].into_boxed_slice(), call_method_depths: vec![0u32; MAX_TRY_DEPTH].into_boxed_slice(), + set_foreach_depths: vec![0usize; MAX_TRY_DEPTH].into_boxed_slice(), + map_foreach_depths: vec![0usize; MAX_TRY_DEPTH].into_boxed_slice(), prototype_resolution_depths: vec![0usize; MAX_TRY_DEPTH].into_boxed_slice(), static_private_owner_depths: vec![0usize; MAX_TRY_DEPTH].into_boxed_slice(), private_lexical_brand_depths: vec![0usize; MAX_TRY_DEPTH].into_boxed_slice(), @@ -219,6 +225,8 @@ fn try_push_with_kind(kind: HandlerKind) -> *mut i32 { // this `try` can restore it — `longjmp` skips the `CallMethodDepthGuard` // `Drop`s of the method frames it unwinds (#5591). (*s).call_method_depths[depth] = crate::object::call_method_depth_savepoint(); + (*s).set_foreach_depths[depth] = crate::set::set_foreach_stack_savepoint(); + (*s).map_foreach_depths[depth] = crate::map::map_foreach_stack_savepoint(); (*s).prototype_resolution_depths[depth] = crate::object::prototype_chain::resolution_stack_savepoint(); (*s).static_private_owner_depths[depth] = @@ -346,6 +354,8 @@ pub extern "C-unwind" fn js_throw(value: f64) -> ! { // otherwise caught throws wrap the counter below zero and wedge every // later method call into the depth-guard fallback (#5591). crate::object::call_method_depth_restore((*s).call_method_depths[depth]); + crate::set::set_foreach_stack_restore((*s).set_foreach_depths[depth]); + crate::map::map_foreach_stack_restore((*s).map_foreach_depths[depth]); crate::object::prototype_chain::resolution_stack_restore( (*s).prototype_resolution_depths[depth], ); @@ -681,6 +691,8 @@ pub(crate) fn test_unwind_innermost_shadow_restore() { let depth = (*s).try_depth - 1; shadow_stack_restore((*s).shadow_savepoints[depth]); runtime_handle_stack_restore((*s).runtime_handle_savepoints[depth]); + crate::set::set_foreach_stack_restore((*s).set_foreach_depths[depth]); + crate::map::map_foreach_stack_restore((*s).map_foreach_depths[depth]); crate::object::prototype_chain::resolution_stack_restore( (*s).prototype_resolution_depths[depth], ); diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 669b23d867..141c171542 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -16,6 +16,39 @@ const TAG_UNDEFINED: u64 = 0x7FFC_0000_0000_0001; crate::perry_thread_local! { static MAP_ITERATOR_ARRAYS: RefCell> = RefCell::new(new_ptr_hash_set()); + /// Backing Maps whose `forEach` raw-entry walk is currently active. A + /// compaction would move entries behind those cursors (#9082). + static MAP_FOREACH_STACK: RefCell> = const { RefCell::new(Vec::new()) }; +} + +#[inline] +fn map_foreach_is_active(map: *const MapHeader) -> bool { + let addr = map as usize; + MAP_FOREACH_STACK.with(|stack| stack.borrow().contains(&addr)) +} + +fn map_foreach_enter(map: *const MapHeader) { + MAP_FOREACH_STACK.with(|stack| stack.borrow_mut().push(map as usize)); +} + +/// Pop one completed walk. Returns true when this was the outermost walk of +/// this Map, at which point deferred compaction is safe again. +fn map_foreach_leave(map: *const MapHeader) -> bool { + let addr = map as usize; + MAP_FOREACH_STACK.with(|stack| { + let mut stack = stack.borrow_mut(); + let popped = stack.pop(); + debug_assert_eq!(popped, Some(addr)); + !stack.contains(&addr) + }) +} + +pub(crate) fn map_foreach_stack_savepoint() -> usize { + MAP_FOREACH_STACK.with(|stack| stack.borrow().len()) +} + +pub(crate) fn map_foreach_stack_restore(depth: usize) { + MAP_FOREACH_STACK.with(|stack| stack.borrow_mut().truncate(depth)); } fn mark_map_iterator_array(arr: *mut crate::array::ArrayHeader) { @@ -789,6 +822,13 @@ pub(crate) fn map_header_moved_for_gc(old_addr: usize, new_addr: usize) { idx.insert(new_addr, slot); } }); + MAP_FOREACH_STACK.with(|stack| { + for addr in stack.borrow_mut().iter_mut() { + if *addr == old_addr { + *addr = new_addr; + } + } + }); } pub(crate) unsafe fn finalize_map_side_allocation_for_gc(map: *mut MapHeader) { @@ -1745,7 +1785,7 @@ unsafe fn ensure_capacity(map: *mut MapHeader) -> bool { // Full by EXTENT. Squeeze tombstones out first — reclaiming holes is // cheaper than doubling, and it keeps a delete-heavy map from growing on // dead weight. - if (*map).size < (*map).used { + if (*map).size < (*map).used && !map_foreach_is_active(map) { compact_map_entries(map); if (*map).used < (*map).capacity { return false; @@ -2425,7 +2465,7 @@ unsafe fn delete_entry_at_index(map: *mut MapHeader, idx: i32) -> i32 { forget_map_index_entry(map, deleted_key, idx as u32); let used = (*map).used; - if used >= 16 && (*map).size < used / 2 { + if used >= 16 && (*map).size < used / 2 && !map_foreach_is_active(map) { compact_map_entries(map); } 1 @@ -2524,6 +2564,9 @@ pub extern "C" fn js_map_clear(map: *mut MapHeader) { let size = unsafe { (*map).size }; let used = unsafe { (*map).used }; if size == 0 { + if !map_foreach_is_active(map) { + unsafe { (*map).used = 0 }; + } return; } // The string and pointer side-tables hold an entry only for a string or @@ -2542,7 +2585,29 @@ pub extern "C" fn js_map_clear(map: *mut MapHeader) { }; unsafe { (*map).size = 0; - (*map).used = 0; + if map_foreach_is_active(map) { + // Preserve the raw [[MapData]] extent while a forEach cursor names + // it. Clearing turns each live pair into a tombstone; entries + // appended by the callback remain after the old extent and are + // visited by the continuing walk. + let entries = entries_ptr_mut(map); + for i in 0..used as usize { + if ptr::read(entries.add(i * 2)).to_bits() != MAP_HOLE_KEY_BITS { + crate::gc::runtime_store_external_jsvalue_slot( + map as usize, + entries.add(i * 2) as usize, + MAP_HOLE_KEY_BITS, + ); + crate::gc::runtime_store_external_jsvalue_slot( + map as usize, + entries.add(i * 2 + 1) as usize, + crate::value::TAG_UNDEFINED, + ); + } + } + } else { + (*map).used = 0; + } } unsafe { if let Some(index) = (*map).numeric_index.as_mut() { @@ -3039,7 +3104,9 @@ fn js_map_foreach_impl( if map.is_null() { return; } - unsafe { compact_if_holey(map as *mut MapHeader) }; + if !map_foreach_is_active(map) { + unsafe { compact_if_holey(map as *mut MapHeader) }; + } let scope = crate::gc::RuntimeHandleScope::new(); let map_handle = scope.root_raw_const_ptr(map); let callback_handle = scope.root_nanbox_f64(callback); @@ -3048,6 +3115,7 @@ fn js_map_foreach_impl( // survives a GC triggered inside the callback. let has_override = collection_override.to_bits() != crate::value::TAG_UNDEFINED; let collection_handle = scope.root_nanbox_f64(collection_override); + map_foreach_enter(map); unsafe { // The collection itself is the third callback argument and the // identity user code compares `self === m` against. @@ -3089,6 +3157,10 @@ fn js_map_foreach_impl( crate::object::js_implicit_this_set(prev_this); } } + let map = map_handle.get_raw_const_ptr::(); + if map_foreach_leave(map) { + unsafe { compact_if_holey(map as *mut MapHeader) }; + } } #[cfg(test)] diff --git a/crates/perry-runtime/src/map_tombstone_tests.rs b/crates/perry-runtime/src/map_tombstone_tests.rs index ae740c9c50..549972572f 100644 --- a/crates/perry-runtime/src/map_tombstone_tests.rs +++ b/crates/perry-runtime/src/map_tombstone_tests.rs @@ -26,30 +26,84 @@ extern "C" fn delete_current_map_entry( f64::from_bits(crate::value::TAG_UNDEFINED) } +extern "C" fn delete_earlier_map_entry( + _closure: *const crate::closure::ClosureHeader, + value: f64, + key: f64, + collection: f64, +) -> f64 { + FOREACH_DELETE_VISITS.with(|visits| visits.borrow_mut().push((key.to_bits(), value.to_bits()))); + if key > 0.0 { + let map = crate::value::js_nanbox_get_pointer(collection) as *mut MapHeader; + js_map_delete(map, key - 1.0); + } + f64::from_bits(crate::value::TAG_UNDEFINED) +} + +fn take_foreach_delete_visits() -> Vec<(f64, f64)> { + FOREACH_DELETE_VISITS.with(|visits| { + visits + .borrow_mut() + .drain(..) + .map(|(key, value)| (f64::from_bits(key), f64::from_bits(value))) + .collect() + }) +} + +fn foreach_callback(func: *const u8) -> f64 { + FOREACH_DELETE_VISITS.with(|visits| visits.borrow_mut().clear()); + let callback = crate::closure::js_closure_alloc(func, 0); + crate::value::js_nanbox_pointer(callback as i64) +} + #[test] -fn foreach_skips_tombstones_created_by_callback_deletes() { +fn foreach_survives_delete_compaction_threshold() { let map = js_map_alloc(4); - for (key, value) in [(1.0, 10.0), (2.0, 20.0), (3.0, 30.0)] { + let expected = (0..20) + .map(|key| (key as f64, (key * 10) as f64)) + .collect::>(); + for &(key, value) in &expected { js_map_set(map, key, value); } - FOREACH_DELETE_VISITS.with(|visits| visits.borrow_mut().clear()); - let callback = crate::closure::js_closure_alloc(delete_current_map_entry as *const u8, 0); + js_map_foreach( + map, + foreach_callback(delete_current_map_entry as *const u8), + f64::from_bits(crate::value::TAG_UNDEFINED), + ); + assert_eq!(take_foreach_delete_visits(), expected); + assert_eq!(js_map_size(map), 0); + unsafe { + assert_eq!( + (*map).used, + 0, + "the completed walk runs deferred compaction" + ) + }; + let map = js_map_alloc(4); + for &(key, value) in &expected { + js_map_set(map, key, value); + } js_map_foreach( map, - crate::value::js_nanbox_pointer(callback as i64), + foreach_callback(delete_earlier_map_entry as *const u8), f64::from_bits(crate::value::TAG_UNDEFINED), ); + assert_eq!(take_foreach_delete_visits(), expected); + assert_eq!(js_map_size(map), 1); +} - let visits = FOREACH_DELETE_VISITS.with(|visits| { - visits - .borrow_mut() - .drain(..) - .map(|(key, value)| (f64::from_bits(key), f64::from_bits(value))) - .collect::>() - }); - assert_eq!(visits, vec![(1.0, 10.0), (2.0, 20.0), (3.0, 30.0)]); - assert_eq!(js_map_size(map), 0); +#[test] +fn caught_throw_restores_map_foreach_compaction_state() { + let map = js_map_alloc(4); + let base = map_foreach_stack_savepoint(); + let _ = crate::exception::js_try_push(); + map_foreach_enter(map); + assert!(map_foreach_is_active(map)); + crate::exception::test_unwind_innermost_shadow_restore(); + assert_eq!(map_foreach_stack_savepoint(), base); + assert!(!map_foreach_is_active(map)); + crate::exception::js_try_end(); } #[test] diff --git a/crates/perry-runtime/src/set.rs b/crates/perry-runtime/src/set.rs index 1ecf3e8fd2..3445ef7e94 100644 --- a/crates/perry-runtime/src/set.rs +++ b/crates/perry-runtime/src/set.rs @@ -13,6 +13,41 @@ use std::ptr; crate::perry_thread_local! { static SET_ITERATOR_ARRAYS: RefCell> = RefCell::new(new_ptr_hash_set()); + /// Backing Sets whose `forEach` raw-index walk is currently in flight. + /// Deletes may leave holes but must not compact them while a cursor still + /// names the old raw layout (#9082). Nested walks are kept as a stack so + /// exception savepoints can discard precisely the frames a throw skips. + static SET_FOREACH_STACK: RefCell> = const { RefCell::new(Vec::new()) }; +} + +#[inline] +fn set_foreach_is_active(set: *const SetHeader) -> bool { + let addr = set as usize; + SET_FOREACH_STACK.with(|stack| stack.borrow().contains(&addr)) +} + +fn set_foreach_enter(set: *const SetHeader) { + SET_FOREACH_STACK.with(|stack| stack.borrow_mut().push(set as usize)); +} + +/// Pop one completed walk. Returns true when this was the outermost walk of +/// this Set, at which point deferred compaction is safe again. +fn set_foreach_leave(set: *const SetHeader) -> bool { + let addr = set as usize; + SET_FOREACH_STACK.with(|stack| { + let mut stack = stack.borrow_mut(); + let popped = stack.pop(); + debug_assert_eq!(popped, Some(addr)); + !stack.contains(&addr) + }) +} + +pub(crate) fn set_foreach_stack_savepoint() -> usize { + SET_FOREACH_STACK.with(|stack| stack.borrow().len()) +} + +pub(crate) fn set_foreach_stack_restore(depth: usize) { + SET_FOREACH_STACK.with(|stack| stack.borrow_mut().truncate(depth)); } fn mark_set_iterator_array(arr: *mut crate::array::ArrayHeader) { @@ -376,6 +411,13 @@ pub(crate) fn set_header_moved_for_gc(old_addr: usize, new_addr: usize) { idx.insert(new_addr, slot); } }); + SET_FOREACH_STACK.with(|stack| { + for addr in stack.borrow_mut().iter_mut() { + if *addr == old_addr { + *addr = new_addr; + } + } + }); } pub(crate) unsafe fn finalize_set_side_allocation_for_gc(set: *mut SetHeader) { @@ -918,12 +960,24 @@ unsafe fn find_value_index_cold(set: *const SetHeader, value: f64) -> i32 { /// Grow the elements array if needed (header stays at same address) unsafe fn ensure_capacity(set: *mut SetHeader) -> bool { let size = (*set).size; + let used = (*set).used; let capacity = (*set).capacity; - if size < capacity { + if used < capacity { return false; } + // A full raw extent can still contain holes. Reclaim them before growing, + // except while `forEach` has a cursor into this layout: moving survivors + // then would skip entries. The active walk instead grows the buffer and + // compacts once its outermost frame finishes. + if size < used && !set_foreach_is_active(set) { + compact_set_elements(set); + if (*set).used < capacity { + return false; + } + } + // Double the capacity let new_capacity = capacity * 2; let old_layout = elements_layout(capacity as usize); @@ -1408,7 +1462,7 @@ pub extern "C" fn js_set_delete(set: *mut SetHeader, value: f64) -> i32 { }); let used = (*set).used; - if used >= 16 && (*set).size < used / 2 { + if used >= 16 && (*set).size < used / 2 && !set_foreach_is_active(set) { compact_set_elements(set); } 1 @@ -1547,15 +1601,36 @@ pub extern "C" fn js_set_clear(set: *mut SetHeader) { return; } unsafe { + let active_foreach = set_foreach_is_active(set); // The side-table mirrors the elements exactly, so an already-empty // set has nothing to reset — half of a change set's per-entity // `adds.clear(); removes.clear()` — and skips the table probe. if (*set).size == 0 { - (*set).used = 0; + if !active_foreach { + (*set).used = 0; + } return; } (*set).size = 0; - (*set).used = 0; + if active_foreach { + // ECMA-262 keeps the current [[SetData]] list in place while a + // forEach is walking it. Mark every live slot empty so the cursor + // skips them; values appended after clear remain after this raw + // extent and are therefore still visited. + let used = (*set).used as usize; + let elements = elements_ptr_mut(set); + for i in 0..used { + if ptr::read(elements.add(i)).to_bits() != SET_HOLE_VALUE_BITS { + crate::gc::runtime_store_external_jsvalue_slot( + set as usize, + elements.add(i) as usize, + SET_HOLE_VALUE_BITS, + ); + } + } + } else { + (*set).used = 0; + } } SET_INDEX.with(|idx| { let mut idx = idx.borrow_mut(); @@ -1818,7 +1893,7 @@ fn js_set_foreach_impl( // the raw marker AND the walk ended early, dropping live elements past it. unsafe { let resolved = clean_set_ptr(set); - if !resolved.is_null() { + if !resolved.is_null() && !set_foreach_is_active(resolved) { compact_if_holey_set(resolved as *mut SetHeader); } } @@ -1835,6 +1910,7 @@ fn js_set_foreach_impl( let this_handle = scope.root_nanbox_f64(this_arg); let has_override = collection_override.to_bits() != crate::value::TAG_UNDEFINED; let collection_handle = scope.root_nanbox_f64(collection_override); + set_foreach_enter(set); unsafe { // ECMA-262 24.2.3.6: Set.prototype.forEach iterates [[SetData]] in // insertion order. `used` is the raw [[SetData]] extent: unlike `size`, @@ -1870,6 +1946,10 @@ fn js_set_foreach_impl( crate::object::js_implicit_this_set(prev_this); } } + let set = set_handle.get_raw_const_ptr::(); + if set_foreach_leave(set) { + unsafe { compact_if_holey_set(set as *mut SetHeader) }; + } } // ===================================================================== diff --git a/crates/perry-runtime/src/set_tombstone_tests.rs b/crates/perry-runtime/src/set_tombstone_tests.rs index 83504a03c5..b8a810fcb7 100644 --- a/crates/perry-runtime/src/set_tombstone_tests.rs +++ b/crates/perry-runtime/src/set_tombstone_tests.rs @@ -26,9 +26,9 @@ extern "C" fn delete_earlier_set_value( collection: f64, ) -> f64 { FOREACH_DELETE_VISITS.with(|visits| visits.borrow_mut().push(value.to_bits())); - if value == 2.0 { + if value > 0.0 { let set = crate::value::js_nanbox_get_pointer(collection) as *mut SetHeader; - js_set_delete(set, 1.0); + js_set_delete(set, value - 1.0); } f64::from_bits(crate::value::TAG_UNDEFINED) } @@ -44,9 +44,10 @@ fn foreach_callback(func: *const u8) -> f64 { } #[test] -fn foreach_skips_tombstones_created_by_callback_deletes() { +fn foreach_survives_delete_compaction_threshold() { + let expected = (0..20).map(|value| value as f64).collect::>(); let set = js_set_alloc(4); - for value in [1.0, 2.0] { + for &value in &expected { js_set_add(set, value); } js_set_foreach( @@ -54,20 +55,40 @@ fn foreach_skips_tombstones_created_by_callback_deletes() { foreach_callback(delete_current_set_value as *const u8), f64::from_bits(crate::value::TAG_UNDEFINED), ); - assert_eq!(take_foreach_delete_visits(), vec![1.0, 2.0]); + assert_eq!(take_foreach_delete_visits(), expected); assert_eq!(js_set_size(set), 0); + unsafe { + assert_eq!( + (*set).used, + 0, + "the completed walk runs deferred compaction" + ) + }; let set = js_set_alloc(4); - for value in [1.0, 2.0, 3.0] { - js_set_add(set, value); + for value in 0..20 { + js_set_add(set, value as f64); } js_set_foreach( set, foreach_callback(delete_earlier_set_value as *const u8), f64::from_bits(crate::value::TAG_UNDEFINED), ); - assert_eq!(take_foreach_delete_visits(), vec![1.0, 2.0, 3.0]); - assert_eq!(js_set_size(set), 2); + assert_eq!(take_foreach_delete_visits(), expected); + assert_eq!(js_set_size(set), 1); +} + +#[test] +fn caught_throw_restores_set_foreach_compaction_state() { + let set = js_set_alloc(4); + let base = set_foreach_stack_savepoint(); + let _ = crate::exception::js_try_push(); + set_foreach_enter(set); + assert!(set_foreach_is_active(set)); + crate::exception::test_unwind_innermost_shadow_restore(); + assert_eq!(set_foreach_stack_savepoint(), base); + assert!(!set_foreach_is_active(set)); + crate::exception::js_try_end(); } #[test]