diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index cfdbfa9fa0..af3ef43aad 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -331,6 +331,75 @@ pub(crate) fn object_has_descriptors(obj: usize) -> bool { false } +/// #6084 (item 6): can anything intercept a plain-data write of `key` to the +/// `GC_TYPE_OBJECT` at `addr` (own accessor / non-writable descriptor, or an +/// inherited setter / non-writable data property), so the dynamic-write +/// transition-cache fast path must be skipped for THIS write? +/// +/// Replaces the process-global `GLOBAL_DESCRIPTORS_IN_USE` latch that used to +/// gate both dynamic-write fast paths. That latch flips on *any* descriptor +/// install anywhere — so a single `Object.freeze` on a completely unrelated +/// object (or any library that freezes one config object at import time) +/// permanently pushed EVERY dynamic property write in the process onto the +/// O(own-key-count) slow walk. Measured: 1M objects × 3 new props = 5281 ms; +/// the identical loop after one unrelated `Object.freeze` = 6807 ms (+29%, +/// and it never recovers). +/// +/// The vetting here is the same predicate `ordinary_set`'s #5054 fast path +/// (`proxy.rs`) already applies per receiver, and the same receiver-level / +/// prototype-level split as the #5654 read-side guard: +/// - own descriptors are visible per-object in `OBJ_FLAG_HAS_DESCRIPTORS` +/// (set by [`note_descriptor_target`], travels with the object on +/// evacuation, and is clear on every fresh allocation); +/// - only *prototype*-level installs can intercept a write to an object whose +/// own flag is clear, and those are checked against the actual prototype +/// chain — `Object.prototype` per-key via [`object_proto_may_intercept_key`] +/// (a blanket check made wide dynamic builds O(n²), see #5054), a recorded +/// `setPrototypeOf` target, or the class chain via +/// [`class_instance_set_may_intercept`]. +/// +/// Conservative in every uncertain case (returns `true` = take the slow path). +/// `caller` must have already established that `addr` is a `GC_TYPE_OBJECT` +/// whose frozen/sealed/non-extensible flags are clear. +pub(crate) unsafe fn plain_data_write_may_intercept(addr: usize, class_id: u32, key: f64) -> bool { + // Nothing has ever installed a descriptor or accessor: no per-object work at + // all, just the one relaxed load the old gate did. + if !descriptors_in_use() { + return false; + } + + // A descriptor exists SOMEWHERE. Vet this receiver and its prototype chain + // instead of latching the whole process onto the slow path. + + // Own accessor / non-writable descriptor on this exact object. + if object_has_descriptors(addr) { + return true; + } + + // `note_descriptor_target` cannot record the per-object flag for typed + // arrays (small ones are plain-alloc'd without a GcHeader) or for exotic + // expando hosts, so their descriptors are invisible to the flag check + // above — never fast-path them once any descriptor exists. + if crate::typedarray::lookup_typed_array_kind(addr).is_some() { + return true; + } + let value = crate::value::js_nanbox_pointer(addr as i64); + if super::exotic_expando::exotic_expando_kind_of_value(value).is_some() { + return true; + } + + if class_id == 0 { + // Plain object. Its prototype is exactly `Object.prototype` unless a + // `setPrototypeOf` target was recorded for it. + super::prototype_chain::object_static_prototype(addr).is_some() + || object_proto_may_intercept_key(key) + } else { + // Class instance: an inherited accessor / non-writable data property + // anywhere in the chain intercepts the write. + class_instance_set_may_intercept(addr, class_id, key) + } +} + /// Store a property descriptor for (obj, key). pub(crate) fn set_property_attrs(obj: usize, key: String, attrs: PropertyAttrs) { note_descriptor_target(obj); diff --git a/crates/perry-runtime/src/object/field_set_by_name.rs b/crates/perry-runtime/src/object/field_set_by_name.rs index cc387c140c..b6367ac360 100644 --- a/crates/perry-runtime/src/object/field_set_by_name.rs +++ b/crates/perry-runtime/src/object/field_set_by_name.rs @@ -51,9 +51,6 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast( if obj.is_null() || (obj as usize) < crate::gc::GC_HEADER_SIZE + 0x1000 { return 0; } - if GLOBAL_DESCRIPTORS_IN_USE.load(Ordering::Relaxed) { - return 0; - } let scope = crate::gc::RuntimeHandleScope::new(); let obj_handle = scope.root_raw_mut_ptr(obj); @@ -80,7 +77,10 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast( if object_flags & (crate::gc::OBJ_FLAG_FROZEN | crate::gc::OBJ_FLAG_SEALED - | crate::gc::OBJ_FLAG_NO_EXTEND) + | crate::gc::OBJ_FLAG_NO_EXTEND + // #6084 item 6: an own descriptor on THIS object (accessor or + // non-writable) must route through the full setter semantics. + | crate::gc::OBJ_FLAG_HAS_DESCRIPTORS) != 0 { return 0; @@ -89,6 +89,18 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast( return 0; } + // #6084 item 6: this used to be a `GLOBAL_DESCRIPTORS_IN_USE` check at + // the top of the function — one `Object.freeze` anywhere in the process + // (even on an unrelated object) permanently disabled this fast path for + // every object. Vet the receiver's own flag (above) and its prototype + // chain (here) instead. `class_id` is 0 at this point, so the only + // inherited interceptor is `Object.prototype` (or a recorded + // `setPrototypeOf` target). + let key_f64 = f64::from_bits(JSValue::string_ptr(key as *mut _).bits()); + if super::plain_data_write_may_intercept(obj as usize, 0, key_f64) { + return 0; + } + let key_gc = (key as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; if (*key_gc).obj_type != crate::gc::GC_TYPE_STRING { @@ -910,10 +922,24 @@ pub extern "C" fn js_object_set_field_by_name( } // FAST PATH: shape-transition cache with interned string pointer identity. + // + // #6084 item 6: the descriptor gate here used to be the process-global + // `GLOBAL_DESCRIPTORS_IN_USE` latch, so ONE `Object.freeze` anywhere + // (even on an object never written to again) permanently forced every + // dynamic write in the process down the O(own-key-count) slow walk + // below. It is now vetted per receiver: an own descriptor is visible in + // this object's `OBJ_FLAG_HAS_DESCRIPTORS`, and only prototype-level + // interceptors need a chain walk. + let has_own_descriptors = obj_flags & crate::gc::OBJ_FLAG_HAS_DESCRIPTORS != 0; if !key.is_null() && !is_frozen && !is_sealed_or_no_extend - && !GLOBAL_DESCRIPTORS_IN_USE.load(Ordering::Relaxed) + && !has_own_descriptors + && !super::plain_data_write_may_intercept( + obj as usize, + (*obj).class_id, + f64::from_bits(JSValue::string_ptr(key as *mut _).bits()), + ) { if let Some((next_keys, slot_idx)) = transition_cache_lookup(prev_keys_usize, interned_key) diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index f3b17b9811..45843e39f1 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -157,10 +157,11 @@ pub(crate) use descriptor_state::{ descriptors_in_use, disable_class_field_inline_guard, get_accessor_descriptor, get_property_attrs, json_object_getter_value, mark_all_keys, note_descriptor_target, object_has_descriptors, object_proto_descriptors_in_use, object_proto_may_intercept_key, - prune_dead_descriptor_owner_entries, reflect_getter_closure_bits, set_accessor_descriptor, - set_builtin_accessor_descriptor, set_builtin_property_attrs, set_property_attrs, - AccessorDescriptor, PropertyAttrs, ACCESSORS_IN_USE, ACCESSOR_DESCRIPTORS, - GLOBAL_DESCRIPTORS_IN_USE, PROPERTY_ATTRS_IN_USE, PROPERTY_DESCRIPTORS, + plain_data_write_may_intercept, prune_dead_descriptor_owner_entries, + reflect_getter_closure_bits, set_accessor_descriptor, set_builtin_accessor_descriptor, + set_builtin_property_attrs, set_property_attrs, AccessorDescriptor, PropertyAttrs, + ACCESSORS_IN_USE, ACCESSOR_DESCRIPTORS, GLOBAL_DESCRIPTORS_IN_USE, PROPERTY_ATTRS_IN_USE, + PROPERTY_DESCRIPTORS, }; pub use this_binding::{ js_implicit_this_get, js_implicit_this_get_sloppy, js_implicit_this_set, js_new_target_get, diff --git a/crates/perry-runtime/src/promise/combinators.rs b/crates/perry-runtime/src/promise/combinators.rs index e22190114b..cd829dbcd2 100644 --- a/crates/perry-runtime/src/promise/combinators.rs +++ b/crates/perry-runtime/src/promise/combinators.rs @@ -5,6 +5,8 @@ use super::*; use std::os::raw::c_int; +use super::keyed_table::PromiseKeyedTable; + use super::assimilate::{ assimilate_via_then_property, enqueue_thenable_job, get_then_action, promise_resolve_assimilating, thenable_job_reject_fn, thenable_job_resolve_fn, @@ -19,8 +21,12 @@ pub(super) struct PromiseAllState { } thread_local! { - pub(super) static PROMISE_ALL_STATES: RefCell> = - const { RefCell::new(Vec::new()) }; + /// Keyed by input-promise address. See `keyed_table.rs`: a dense `Vec` is + /// still the GC scanners' traversal/rewrite surface, with an O(1) key index + /// layered on top (#6084 item 2 — this used to be a raw `Vec` that every + /// settlement scanned end to end). + pub(super) static PROMISE_ALL_STATES: RefCell> = + const { RefCell::new(PromiseKeyedTable::new()) }; } /// Drain ALL `PromiseAllState` entries associated with `promise`. @@ -42,20 +48,7 @@ pub(super) fn promise_all_take_all_handlers(promise: *mut Promise) -> Vec) { PROMISE_ALL_STATES.with(|states| { let mut states = states.borrow_mut(); - for (key, state) in states.iter_mut() { - visitor.visit_metadata_usize_slot(key); - visitor.visit_raw_mut_ptr_slot(&mut state.result_promise); - visitor.visit_raw_mut_ptr_slot(&mut state.results_arr); - visitor.visit_raw_mut_ptr_slot(&mut state.state_arr); + let mut rekeyed = false; + for entry in states.iter_mut() { + // Evacuation rewrites the key IN PLACE — the position stays valid, + // but the key → position index no longer does. Rebuild it lazily. + rekeyed |= visitor.visit_metadata_usize_slot(&mut entry.key); + visitor.visit_raw_mut_ptr_slot(&mut entry.value.result_promise); + visitor.visit_raw_mut_ptr_slot(&mut entry.value.results_arr); + visitor.visit_raw_mut_ptr_slot(&mut entry.value.state_arr); + } + if rekeyed { + states.note_key_rewritten(); } }); } @@ -88,12 +87,7 @@ pub(super) fn remove_all_states_for_dead_promise(promise: *mut Promise) { return; } let key = promise as usize; - PROMISE_ALL_STATES.with(|states| { - let mut states = states.borrow_mut(); - if !states.is_empty() { - states.retain(|(k, _)| *k != key); - } - }); + PROMISE_ALL_STATES.with(|states| states.borrow_mut().remove_key(key)); } /// Copied-minor from-space cleanup for `PROMISE_ALL_STATES` — see @@ -101,11 +95,11 @@ pub(super) fn remove_all_states_for_dead_promise(promise: *mut Promise) { pub(super) fn cleanup_copied_minor_all_states_for_gc() { use super::CopiedMinorPromiseKeyFate::*; PROMISE_ALL_STATES.with(|states| { - states.borrow_mut().retain_mut(|(key, _)| { - match super::copied_minor_promise_key_fate(*key) { + states.borrow_mut().retain_mut(|entry| { + match super::copied_minor_promise_key_fate(entry.key) { Keep => true, Rekey(new_key) => { - *key = new_key; + entry.key = new_key; true } Drop => false, @@ -891,7 +885,7 @@ pub extern "C" fn js_promise_all(promises_arr: *const crate::array::ArrayHeader) } PromiseState::Pending => { PROMISE_ALL_STATES.with(|states| { - states.borrow_mut().push((promise_ptr as usize, state)); + states.borrow_mut().push(promise_ptr as usize, state); }); set_promise_callback_context(promise_ptr); } @@ -1721,12 +1715,8 @@ mod tests { assert_eq!((*all_b).state, PromiseState::Pending); // PROMISE_ALL_STATES must hold TWO entries keyed on `shared`. - let registered = PROMISE_ALL_STATES.with(|s| { - s.borrow() - .iter() - .filter(|(k, _)| *k == shared as usize) - .count() - }); + let registered = + PROMISE_ALL_STATES.with(|s| s.borrow_mut().count_for_key(shared as usize)); assert_eq!( registered, 2, "expected two Promise.all states keyed on the shared pending promise" diff --git a/crates/perry-runtime/src/promise/keyed_table.rs b/crates/perry-runtime/src/promise/keyed_table.rs new file mode 100644 index 0000000000..ebf35197f1 --- /dev/null +++ b/crates/perry-runtime/src/promise/keyed_table.rs @@ -0,0 +1,604 @@ +//! `PromiseKeyedTable` — the shared backing store for the three +//! promise-pointer-keyed reaction side tables (`PROMISE_ALL_STATES`, +//! `PROMISE_SETTLE_LISTENERS`, `PROMISE_OVERFLOW_REACTIONS`). +//! +//! # Why this exists (#6084 item 2) +//! +//! All three were `RefCell>` and every promise settlement ran a +//! FULL LINEAR SCAN of each table to collect the entries keyed by the settling +//! promise — `js_promise_resolve`/`reject` call +//! `promise_take_settle_listeners` + `promise_all_take_all_handlers` + +//! `promise_take_overflow_reactions` unconditionally. With N promises parked in +//! a table (the `Promise.all([...N])` shape), settling all N is O(N²). Measured +//! settle time for `Promise.all` over N pending promises: 5k → 14ms, 10k → 26ms, +//! 20k → 329ms, 40k → 595ms (node: 1/0/2/4ms). The per-sweep death hook +//! (`remove_*_for_dead_promise`) had the same shape: O(table) per dead promise. +//! +//! # Why it is not just a `HashMap` +//! +//! These tables are **GC root side tables**, and two GC mechanisms depend on the +//! backing `Vec`'s layout: +//! +//! 1. **The incremental root scanner resumes by position.** +//! `PromiseRootScanState { index, slot }` (`scanners.rs`) stops mid-table +//! when its step budget runs out and resumes at `entries[index]` on the next +//! increment. A `HashMap` has no stable positional cursor. +//! 2. **Evacuation rewrites the key in place.** The scanners call +//! `visitor.visit_metadata_usize_slot(&mut entry.key)`: when a moving GC +//! relocates a promise, the GC *edits the key of a live entry*. Doing that +//! inside a `HashMap` leaves the entry in the bucket for its OLD hash — +//! silently unfindable. +//! +//! So the dense `Vec>` is kept EXACTLY as the traversal/rewrite surface +//! (the scanners' access pattern and the mutation discipline — append + swap +//! remove — are unchanged from the old raw `Vec`), and an O(1) key → positions +//! index is layered on top as *derived* state: +//! +//! * `index` is a plain `HashMap` of positions into `entries`. +//! * `Entry::slot` is the *reverse* of that: an entry's own offset within its +//! key's slot list. Draining key A `swap_remove`s A's entries, and each +//! removal displaces one FOREIGN entry (the table's last) into the vacated +//! slot, whose key's slot list must then be repointed. Searching that list +//! for the old position is Θ(M) for a key with M parked entries — so +//! `a.then(f)×M; b.then(f)×M; resolveA()` drains A in Θ(M²), the very defect +//! this table exists to remove, just moved from the table to a slot list. +//! With `slot` the displaced entry names its own offset and the repoint is a +//! single indexed store. +//! * Any GC action that can invalidate them — a key rewritten by evacuation +//! (`note_key_rewritten`), or the copied-minor cleanup dropping/rekeying +//! entries (`retain_mut`) — just sets `index_dirty`. Nothing tries to patch +//! the index (or `slot`) incrementally from inside a GC path. +//! * The next lookup rebuilds both from `entries` alone (`ensure_index`). That +//! is O(n log n) but happens at most once per GC cycle that actually moved a +//! promise, and such a cycle already pays O(n) walking this table. +//! +//! This is the same "keys vector stays the GC traversal surface, map gives O(1)" +//! split that `PromiseContextStore` (#6267) uses for the 1:1 `PROMISE_CONTEXTS` +//! table; `PromiseKeyedTable` is its 1:N (multimap) analogue — a promise can be +//! an input to several `Promise.all` calls, and can carry several settle +//! listeners / overflow reactions. +//! +//! # Ordering +//! +//! Per-key registration (FIFO) order is observable: overflow reactions +//! (`p.then(a); p.then(b)`) must replay in registration order. The dense `Vec` +//! no longer preserves it (`swap_remove` reorders), so each entry carries a +//! monotonic `seq` and `take_all` sorts the drained entries by it. `seq` also +//! makes the index fully reconstructible from `entries` alone, which is what +//! lets the GC paths get away with a blunt "mark dirty". + +use std::collections::HashMap; + +use crate::fast_hash::{PtrHashMap, PtrHasher}; + +/// One parked reaction. `key` is the pending promise's address — a GC-visible +/// metadata slot, rewritten in place by evacuation. +pub(super) struct Entry { + pub(super) key: usize, + /// Monotonic registration counter — restores FIFO order after `swap_remove` + /// shuffles `entries`, and is the sort key when the index is rebuilt. + seq: u64, + /// Reverse position index: this entry's own offset within `index[key]`'s + /// slot list, i.e. `index[key].as_slice()[slot]` is this entry's position in + /// `entries`. Lets a displaced entry repoint its key's slot list with one + /// indexed store instead of scanning it (see `take_all`). + /// + /// Derived state, exactly like `index` itself: meaningful only while + /// `!index_dirty`, and rebuilt wholesale by `ensure_index`. No GC path + /// maintains it. + slot: u32, + pub(super) value: T, +} + +/// Positions of one key's entries, in registration order. Inline for the +/// overwhelmingly common single-entry case so a `Promise.all` over N distinct +/// promises does not allocate N side `Vec`s. +enum Slots { + One(u32), + Many(Vec), +} + +impl Slots { + /// Append `position` and return the offset it landed at — the caller stores + /// that in `Entry::slot` so the entry can find itself again in O(1). + #[inline] + fn push(&mut self, position: u32) -> u32 { + match self { + Slots::One(first) => { + *self = Slots::Many(vec![*first, position]); + 1 + } + Slots::Many(positions) => { + positions.push(position); + (positions.len() - 1) as u32 + } + } + } + + #[inline] + fn as_slice(&self) -> &[u32] { + match self { + Slots::One(first) => std::slice::from_ref(first), + Slots::Many(positions) => positions, + } + } + + /// Repoint the slot at `offset` — the displaced entry's own `Entry::slot` — + /// at its new home in `entries`. O(1): no scan of the slot list, which is + /// what made draining one heavily-populated key ahead of another Θ(M²). + #[inline] + fn relocate(&mut self, offset: u32, new_position: u32) { + match self { + Slots::One(first) => { + debug_assert_eq!(offset, 0, "a One slot list has only offset 0"); + *first = new_position; + } + Slots::Many(positions) => { + debug_assert!((offset as usize) < positions.len(), "slot offset in range"); + if let Some(position) = positions.get_mut(offset as usize) { + *position = new_position; + } + } + } + } +} + +pub(super) struct PromiseKeyedTable { + /// Dense storage. This is the GC scanners' traversal surface: they walk it + /// positionally and rewrite `Entry::key` in place on evacuation. + entries: Vec>, + /// Derived: promise address → positions in `entries`. Rebuilt wholesale + /// whenever a GC path touches `entries` (see `index_dirty`). Keys are raw + /// promise pointers, so `PtrHasher` (one multiply) rather than SipHash. + index: PtrHashMap, + /// Set by every GC path that can invalidate `index` (key rewritten by + /// evacuation, entries dropped/rekeyed by the copied-minor cleanup). The + /// next lookup rebuilds the index from `entries`. + index_dirty: bool, + next_seq: u64, +} + +impl Default for PromiseKeyedTable { + fn default() -> Self { + Self::new() + } +} + +impl PromiseKeyedTable { + pub(super) const fn new() -> Self { + Self { + entries: Vec::new(), + // `HashMap::with_hasher` is const; `HashMap::new` (RandomState) is + // not — and these thread_locals want const init. + index: HashMap::with_hasher(PtrHasher), + index_dirty: false, + next_seq: 0, + } + } + + #[inline] + pub(super) fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + #[inline] + pub(super) fn len(&self) -> usize { + self.entries.len() + } + + /// Park `value` against `key`. O(1). + pub(super) fn push(&mut self, key: usize, value: T) { + let position = self.entries.len() as u32; + let seq = self.next_seq; + self.next_seq += 1; + // While the index is dirty it is not maintained at all, and neither is + // `slot`: `ensure_index` rebuilds both together on the next lookup. + let slot = if self.index_dirty { + 0 + } else { + self.index_slot_for(key, position) + }; + self.entries.push(Entry { + key, + seq, + slot, + value, + }); + } + + /// Record `position` under `key` in the index and return the entry's offset + /// within that key's slot list. + #[inline] + fn index_slot_for(&mut self, key: usize, position: u32) -> u32 { + match self.index.get_mut(&key) { + Some(slots) => slots.push(position), + None => { + self.index.insert(key, Slots::One(position)); + 0 + } + } + } + + /// Positional accessor for the incremental GC scanners. + #[inline] + pub(super) fn entry_at_mut(&mut self, index: usize) -> Option<&mut Entry> { + self.entries.get_mut(index) + } + + /// Iterate every entry — for the non-incremental (`_mut`) root scanners. + #[inline] + pub(super) fn iter_mut(&mut self) -> impl Iterator> { + self.entries.iter_mut() + } + + /// A GC pass rewrote at least one `Entry::key` (evacuation moved a promise), + /// so every recorded position is still valid but the key → position mapping + /// is not. Rebuild lazily rather than patching a HashMap whose hashes just + /// changed underneath it. + #[inline] + pub(super) fn note_key_rewritten(&mut self) { + self.index_dirty = true; + } + + /// Drain every entry parked against `key`, in registration (FIFO) order. + /// O(k + k·log k) in the number of entries for THAT key — no longer O(table). + pub(super) fn take_all(&mut self, key: usize) -> Vec { + if self.entries.is_empty() { + return Vec::new(); + } + self.ensure_index(); + let Some(slots) = self.index.remove(&key) else { + return Vec::new(); + }; + + let mut positions: Vec = slots.as_slice().to_vec(); + // Remove from the highest position down. `swap_remove(p)` backfills `p` + // with the element that was last; taking the largest of OUR positions + // first guarantees that element is never another entry we still have to + // remove (all of ours that remain are at strictly lower positions), so + // each removal either pops the tail or relocates exactly one FOREIGN + // entry, which we fix up through its own key's slot list. + positions.sort_unstable_by(|a, b| b.cmp(a)); + + let mut drained: Vec> = Vec::with_capacity(positions.len()); + for position in positions { + let entry = self.entries.swap_remove(position as usize); + debug_assert_eq!(entry.key, key); + let vacated = position as usize; + if vacated < self.entries.len() { + // `swap_remove` moved what was the LAST entry into `vacated`. + // Per the argument above it is always a FOREIGN entry, so its + // key still has a slot list in the index (ours was removed) and + // that list has to point at `position` now. The entry knows its + // own offset within that list, so this is one indexed store — + // scanning the list instead is Θ(M) per removal and turns + // draining one heavily-populated key that sits ahead of another + // into Θ(M²). + let moved = &self.entries[vacated]; + debug_assert_ne!(moved.key, key, "displaced entry must be foreign"); + let moved_key = moved.key; + let moved_slot = moved.slot; + if let Some(slots) = self.index.get_mut(&moved_key) { + slots.relocate(moved_slot, position); + } + } + drained.push(entry); + } + + // `entries` is not in registration order (swap_remove reorders it), so + // restore FIFO from the monotonic seq. Observable for overflow + // reactions: `p.then(a); p.then(b)` must run a before b. + drained.sort_unstable_by_key(|entry| entry.seq); + drained.into_iter().map(|entry| entry.value).collect() + } + + /// Drop every entry parked against `key` (GC death hook: the promise was + /// swept, so nothing parked against it can ever fire). O(k), was O(table). + pub(super) fn remove_key(&mut self, key: usize) { + if self.entries.is_empty() { + return; + } + drop(self.take_all(key)); + } + + /// Copied-minor from-space cleanup: `keep` may mutate `Entry::key` (rekey a + /// promise the scanners missed) and returns false to drop the entry. Both + /// invalidate the index, so it is rebuilt on the next lookup. + pub(super) fn retain_mut(&mut self, mut keep: impl FnMut(&mut Entry) -> bool) { + if self.entries.is_empty() { + return; + } + self.entries.retain_mut(|entry| keep(entry)); + self.index_dirty = true; + } + + #[cfg(test)] + pub(super) fn clear(&mut self) { + self.entries.clear(); + self.index.clear(); + self.index_dirty = false; + } + + #[cfg(test)] + pub(super) fn count_for_key(&mut self, key: usize) -> usize { + self.ensure_index(); + self.index + .get(&key) + .map(|slots| slots.as_slice().len()) + .unwrap_or(0) + } + + /// Rebuild `index` — and every entry's reverse `slot` — from `entries` + /// alone. Positions are grouped per key in ascending `seq` order so each + /// key's slot list stays registration-ordered. + fn ensure_index(&mut self) { + if !self.index_dirty { + return; + } + self.index.clear(); + let mut order: Vec = (0..self.entries.len() as u32).collect(); + order.sort_unstable_by_key(|&position| self.entries[position as usize].seq); + for position in order { + let key = self.entries[position as usize].key; + let slot = self.index_slot_for(key, position); + self.entries[position as usize].slot = slot; + } + self.index_dirty = false; + } + + /// Debug-only: the index must describe `entries` exactly, each key's slot + /// list must be in registration (seq) order, and every entry's reverse + /// `slot` must point back at the slot that points at it. + #[cfg(test)] + fn assert_invariants(&self) { + if self.index_dirty { + return; + } + let indexed: usize = self.index.values().map(|s| s.as_slice().len()).sum(); + assert_eq!(indexed, self.entries.len(), "index must cover every entry"); + for (key, slots) in self.index.iter() { + let mut last_seq = None; + for (offset, &position) in slots.as_slice().iter().enumerate() { + let entry = &self.entries[position as usize]; + assert_eq!(entry.key, *key, "indexed position must hold that key"); + assert_eq!( + entry.slot as usize, offset, + "entry must record its own offset in its key's slot list" + ); + if let Some(previous) = last_seq { + assert!(entry.seq > previous, "slot list must be seq-ordered"); + } + last_seq = Some(entry.seq); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::PromiseKeyedTable; + + #[test] + fn take_all_returns_registration_order_and_leaves_others_intact() { + let mut table: PromiseKeyedTable<&str> = PromiseKeyedTable::new(); + table.push(1, "a1"); + table.push(2, "b1"); + table.push(1, "a2"); + table.push(3, "c1"); + table.push(1, "a3"); + table.assert_invariants(); + + assert_eq!(table.take_all(1), vec!["a1", "a2", "a3"]); + table.assert_invariants(); + assert_eq!(table.len(), 2); + assert_eq!(table.take_all(1), Vec::<&str>::new()); + assert_eq!(table.take_all(2), vec!["b1"]); + assert_eq!(table.take_all(3), vec!["c1"]); + assert!(table.is_empty()); + } + + #[test] + fn take_all_of_absent_key_is_a_no_op() { + let mut table: PromiseKeyedTable = PromiseKeyedTable::new(); + table.push(7, 1); + assert_eq!(table.take_all(9), Vec::::new()); + assert_eq!(table.len(), 1); + table.assert_invariants(); + } + + #[test] + fn rekey_then_lookup_rebuilds_the_index() { + // Mirrors evacuation: the scanner rewrites Entry::key in place and calls + // note_key_rewritten(); the entry must be findable under its NEW key and + // gone from the old one. + let mut table: PromiseKeyedTable<&str> = PromiseKeyedTable::new(); + table.push(10, "x"); + table.push(20, "y"); + table.push(10, "z"); + + for entry in table.iter_mut() { + if entry.key == 10 { + entry.key = 99; + } + } + table.note_key_rewritten(); + + assert_eq!(table.take_all(10), Vec::<&str>::new()); + assert_eq!(table.take_all(99), vec!["x", "z"]); + table.assert_invariants(); + assert_eq!(table.take_all(20), vec!["y"]); + } + + #[test] + fn retain_mut_drop_and_rekey_rebuilds_the_index() { + let mut table: PromiseKeyedTable = PromiseKeyedTable::new(); + for i in 0..6u32 { + table.push((i % 3) as usize, i); + } + // Drop key 1 entirely, rekey key 0 -> key 5. + table.retain_mut(|entry| { + if entry.key == 1 { + return false; + } + if entry.key == 0 { + entry.key = 5; + } + true + }); + + assert_eq!(table.take_all(1), Vec::::new()); + assert_eq!(table.take_all(0), Vec::::new()); + assert_eq!(table.take_all(5), vec![0, 3]); + table.assert_invariants(); + assert_eq!(table.take_all(2), vec![2, 5]); + assert!(table.is_empty()); + } + + #[test] + fn remove_key_drops_only_that_key() { + let mut table: PromiseKeyedTable = PromiseKeyedTable::new(); + table.push(1, 10); + table.push(2, 20); + table.push(1, 11); + table.remove_key(1); + table.assert_invariants(); + assert_eq!(table.len(), 1); + assert_eq!(table.take_all(2), vec![20]); + } + + /// Randomized differential test against the naive `Vec<(usize, T)>` the + /// three tables used before #6084: same push/take/remove sequence must + /// produce the same drained values in the same order. + #[test] + fn matches_naive_vec_model_under_random_operations() { + let mut table: PromiseKeyedTable = PromiseKeyedTable::new(); + let mut model: Vec<(usize, u64)> = Vec::new(); + + // Deterministic xorshift — no rand dependency in perry-runtime. + let mut state: u64 = 0x243f_6a88_85a3_08d3; + let mut next = || { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + state + }; + + for step in 0..20_000u64 { + let key = (next() % 16) as usize; + match next() % 4 { + 0 | 1 => { + table.push(key, step); + model.push((key, step)); + } + 2 => { + let drained = table.take_all(key); + let expected: Vec = model + .iter() + .filter(|(k, _)| *k == key) + .map(|(_, v)| *v) + .collect(); + model.retain(|(k, _)| *k != key); + assert_eq!(drained, expected, "FIFO drain must match the naive model"); + } + _ => { + table.remove_key(key); + model.retain(|(k, _)| *k != key); + } + } + assert_eq!(table.len(), model.len()); + // The reverse `slot` index is only observable through a later + // relocation, so check it as we go rather than once at the end. + if step % 128 == 0 { + table.assert_invariants(); + } + } + table.assert_invariants(); + } + + /// The regression this exists to prevent: settling N distinct promises must + /// not be O(N²). With the old full-table scan this is ~N²/2 comparisons. + /// + /// Shape: MANY keys, ONE entry each (`Promise.all([...N])`). This exercises + /// the *lookup* path but NOT the *relocation* path — see + /// `draining_a_heavily_populated_key_ahead_of_another_is_not_quadratic`. + #[test] + fn settling_many_keys_is_not_quadratic() { + use std::time::Instant; + + fn drain_all(n: usize) -> std::time::Duration { + let mut table: PromiseKeyedTable = PromiseKeyedTable::new(); + for i in 0..n { + table.push(i * 64, i); + } + let start = Instant::now(); + for i in 0..n { + assert_eq!(table.take_all(i * 64), vec![i]); + } + start.elapsed() + } + + // Warm the allocator so the small run isn't paying first-touch costs. + drain_all(4_000); + let small = drain_all(20_000).as_secs_f64(); + let large = drain_all(80_000).as_secs_f64(); + // 4x the entries. Linear ⇒ ~4x. Quadratic ⇒ ~16x. Allow a wide margin + // for timer noise on loaded CI hosts but still fail the O(n²) shape. + assert!( + large < small * 8.0 + 0.05, + "drain looks super-linear: 20k took {small:.4}s, 80k took {large:.4}s" + ); + } + + /// The OTHER quadratic — the one the many-keys/one-entry benchmark above + /// cannot see (CodeRabbit review on #6327). + /// + /// Shape: a FEW keys, each with MANY parked entries. In TS: + /// `for (…M…) a.then(f); for (…M…) b.then(f); resolveA()`. All of A's + /// entries sit at the FRONT of `entries`, so every `swap_remove` of one + /// backfills the vacated slot with one of B's entries and has to repoint + /// B's slot list at the moved entry's new home. Finding the moved entry's + /// offset in B's slot list by scanning is Θ(M) per removal ⇒ Θ(M²) for the + /// drain. Each entry therefore records its own offset (`Entry::slot`), so + /// the relocation is a single indexed store. + #[test] + fn draining_a_heavily_populated_key_ahead_of_another_is_not_quadratic() { + use std::time::Instant; + + const A: usize = 0x1000; + const B: usize = 0x2000; + + fn drain_first_of_two(m: usize) -> std::time::Duration { + let mut table: PromiseKeyedTable = PromiseKeyedTable::new(); + for i in 0..m { + table.push(A, i); + } + for i in 0..m { + table.push(B, m + i); + } + + let start = Instant::now(); + let drained = table.take_all(A); + let elapsed = start.elapsed(); + + // Correctness, not just speed: A drains in registration order and + // B is left whole (and still in registration order). + assert_eq!(drained, (0..m).collect::>()); + assert_eq!(table.len(), m); + table.assert_invariants(); + assert_eq!(table.take_all(B), (m..2 * m).collect::>()); + assert!(table.is_empty()); + elapsed + } + + drain_first_of_two(1_000); + let small = drain_first_of_two(20_000).as_secs_f64(); + let large = drain_first_of_two(80_000).as_secs_f64(); + eprintln!("two-key drain: M=20k {small:.4}s, M=80k {large:.4}s"); + // 4x the entries per key. Linear ⇒ ~4x. Quadratic ⇒ ~16x. + assert!( + large < small * 8.0 + 0.05, + "displaced-entry relocation looks super-linear: \ + M=20k took {small:.4}s, M=80k took {large:.4}s" + ); + } +} diff --git a/crates/perry-runtime/src/promise/mod.rs b/crates/perry-runtime/src/promise/mod.rs index 8474d3d58b..33cfb5e6d3 100644 --- a/crates/perry-runtime/src/promise/mod.rs +++ b/crates/perry-runtime/src/promise/mod.rs @@ -22,6 +22,7 @@ pub mod assimilate; pub mod async_step; pub mod checked_dispatch; pub mod combinators; +pub(crate) mod keyed_table; pub mod microtasks; pub mod native_async; pub mod reactions; diff --git a/crates/perry-runtime/src/promise/reactions.rs b/crates/perry-runtime/src/promise/reactions.rs index 95b576f0dd..d33fb05129 100644 --- a/crates/perry-runtime/src/promise/reactions.rs +++ b/crates/perry-runtime/src/promise/reactions.rs @@ -8,6 +8,8 @@ use super::*; +use super::keyed_table::PromiseKeyedTable; + pub(super) struct PromiseSettleListener { pub(super) on_fulfilled: ClosurePtr, pub(super) on_rejected: ClosurePtr, @@ -15,8 +17,10 @@ pub(super) struct PromiseSettleListener { } thread_local! { - pub(super) static PROMISE_SETTLE_LISTENERS: RefCell> = - const { RefCell::new(Vec::new()) }; + /// Keyed by pending-promise address — see `keyed_table.rs` (#6084 item 2: + /// this used to be a raw `Vec` that every settlement scanned end to end). + pub(super) static PROMISE_SETTLE_LISTENERS: RefCell> = + const { RefCell::new(PromiseKeyedTable::new()) }; } pub(crate) fn js_promise_attach_settle_listener( @@ -37,14 +41,14 @@ pub(crate) fn js_promise_attach_settle_listener( crate::gc::runtime_write_barrier_root_raw_ptr(on_fulfilled); crate::gc::runtime_write_barrier_root_raw_ptr(on_rejected); PROMISE_SETTLE_LISTENERS.with(|listeners| { - listeners.borrow_mut().push(( + listeners.borrow_mut().push( promise as usize, PromiseSettleListener { on_fulfilled, on_rejected, context, }, - )); + ); }); } PromiseState::Fulfilled => { @@ -61,20 +65,7 @@ pub(super) fn promise_take_settle_listeners(promise: *mut Promise) -> Vec) { PROMISE_SETTLE_LISTENERS.with(|listeners| { - for (key, listener) in listeners.borrow_mut().iter_mut() { - visitor.visit_metadata_usize_slot(key); - visitor.visit_raw_const_ptr_slot(&mut listener.on_fulfilled); - visitor.visit_raw_const_ptr_slot(&mut listener.on_rejected); - scan_snapshot_roots_mut(&mut listener.context, visitor); + let mut listeners = listeners.borrow_mut(); + let mut rekeyed = false; + for entry in listeners.iter_mut() { + rekeyed |= visitor.visit_metadata_usize_slot(&mut entry.key); + visitor.visit_raw_const_ptr_slot(&mut entry.value.on_fulfilled); + visitor.visit_raw_const_ptr_slot(&mut entry.value.on_rejected); + scan_snapshot_roots_mut(&mut entry.value.context, visitor); + } + if rekeyed { + listeners.note_key_rewritten(); } }); } @@ -116,12 +112,7 @@ pub(super) fn remove_settle_listeners_for_dead_promise(promise: *mut Promise) { return; } let key = promise as usize; - PROMISE_SETTLE_LISTENERS.with(|listeners| { - let mut listeners = listeners.borrow_mut(); - if !listeners.is_empty() { - listeners.retain(|(k, _)| *k != key); - } - }); + PROMISE_SETTLE_LISTENERS.with(|listeners| listeners.borrow_mut().remove_key(key)); } /// Copied-minor from-space cleanup for the settle-listener table: drop entries @@ -130,11 +121,11 @@ pub(super) fn remove_settle_listeners_for_dead_promise(promise: *mut Promise) { pub(super) fn cleanup_copied_minor_settle_listeners_for_gc() { use super::CopiedMinorPromiseKeyFate::*; PROMISE_SETTLE_LISTENERS.with(|listeners| { - listeners.borrow_mut().retain_mut(|(key, _)| { - match super::copied_minor_promise_key_fate(*key) { + listeners.borrow_mut().retain_mut(|entry| { + match super::copied_minor_promise_key_fate(entry.key) { Keep => true, Rekey(new_key) => { - *key = new_key; + entry.key = new_key; true } Drop => false, @@ -169,8 +160,10 @@ pub(super) struct OverflowReaction { } thread_local! { - pub(super) static PROMISE_OVERFLOW_REACTIONS: RefCell> = - const { RefCell::new(Vec::new()) }; + /// Keyed by pending-promise address — see `keyed_table.rs` (#6084 item 2: + /// this used to be a raw `Vec` that every settlement scanned end to end). + pub(super) static PROMISE_OVERFLOW_REACTIONS: RefCell> = + const { RefCell::new(PromiseKeyedTable::new()) }; } /// Park a 2nd+ reaction on a still-pending `promise`. @@ -186,7 +179,7 @@ pub(super) fn push_overflow_reaction( crate::gc::runtime_write_barrier_root_raw_ptr(on_rejected); crate::gc::runtime_write_barrier_root_raw_ptr(next); PROMISE_OVERFLOW_REACTIONS.with(|r| { - r.borrow_mut().push(( + r.borrow_mut().push( promise as usize, OverflowReaction { on_fulfilled, @@ -194,7 +187,7 @@ pub(super) fn push_overflow_reaction( next, context, }, - )); + ); }); } @@ -202,30 +195,11 @@ pub(super) fn push_overflow_reaction( /// `promise`. Returns `Vec::new()` for the overwhelmingly common no-overflow /// case without touching the table's allocation. pub(super) fn promise_take_overflow_reactions(promise: *mut Promise) -> Vec { - PROMISE_OVERFLOW_REACTIONS.with(|r| { - let mut r = r.borrow_mut(); - if r.is_empty() { - return Vec::new(); - } - let key = promise as usize; - let mut drained = Vec::new(); - // Preserve FIFO order (a plain filter keeps relative order; swap_remove - // would not — reaction ordering is observable, see resolved-sequence). - r.retain(|(k, reaction)| { - if *k == key { - drained.push(OverflowReaction { - on_fulfilled: reaction.on_fulfilled, - on_rejected: reaction.on_rejected, - next: reaction.next, - context: reaction.context.clone(), - }); - false - } else { - true - } - }); - drained - }) + // FIFO order is observable (`p.then(a); p.then(b)` must run a before b). + // `take_all` restores it from each entry's registration seq — the old + // order-preserving `retain` had to touch every entry in the table, which is + // what made settling N promises O(N²) (#6084 item 2). + PROMISE_OVERFLOW_REACTIONS.with(|r| r.borrow_mut().take_all(promise as usize)) } /// Push the `Task::Inline` jobs for a settled promise's drained overflow @@ -251,12 +225,17 @@ pub(super) fn enqueue_overflow_reactions( pub(super) fn scan_promise_overflow_reactions_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) { PROMISE_OVERFLOW_REACTIONS.with(|reactions| { - for (key, reaction) in reactions.borrow_mut().iter_mut() { - visitor.visit_metadata_usize_slot(key); - visitor.visit_raw_const_ptr_slot(&mut reaction.on_fulfilled); - visitor.visit_raw_const_ptr_slot(&mut reaction.on_rejected); - visitor.visit_raw_mut_ptr_slot(&mut reaction.next); - scan_snapshot_roots_mut(&mut reaction.context, visitor); + let mut reactions = reactions.borrow_mut(); + let mut rekeyed = false; + for entry in reactions.iter_mut() { + rekeyed |= visitor.visit_metadata_usize_slot(&mut entry.key); + visitor.visit_raw_const_ptr_slot(&mut entry.value.on_fulfilled); + visitor.visit_raw_const_ptr_slot(&mut entry.value.on_rejected); + visitor.visit_raw_mut_ptr_slot(&mut entry.value.next); + scan_snapshot_roots_mut(&mut entry.value.context, visitor); + } + if rekeyed { + reactions.note_key_rewritten(); } }); } @@ -270,12 +249,7 @@ pub(super) fn remove_overflow_reactions_for_dead_promise(promise: *mut Promise) return; } let key = promise as usize; - PROMISE_OVERFLOW_REACTIONS.with(|reactions| { - let mut reactions = reactions.borrow_mut(); - if !reactions.is_empty() { - reactions.retain(|(k, _)| *k != key); - } - }); + PROMISE_OVERFLOW_REACTIONS.with(|reactions| reactions.borrow_mut().remove_key(key)); } /// Copied-minor from-space cleanup for the overflow-reaction table — see @@ -284,11 +258,11 @@ pub(super) fn remove_overflow_reactions_for_dead_promise(promise: *mut Promise) pub(super) fn cleanup_copied_minor_overflow_reactions_for_gc() { use super::CopiedMinorPromiseKeyFate::*; PROMISE_OVERFLOW_REACTIONS.with(|reactions| { - reactions.borrow_mut().retain_mut(|(key, _)| { - match super::copied_minor_promise_key_fate(*key) { + reactions.borrow_mut().retain_mut(|entry| { + match super::copied_minor_promise_key_fate(entry.key) { Keep => true, Rekey(new_key) => { - *key = new_key; + entry.key = new_key; true } Drop => false, diff --git a/crates/perry-runtime/src/promise/scanners.rs b/crates/perry-runtime/src/promise/scanners.rs index 14bc7cb15e..6c084d9641 100644 --- a/crates/perry-runtime/src/promise/scanners.rs +++ b/crates/perry-runtime/src/promise/scanners.rs @@ -563,14 +563,22 @@ fn scan_promise_all_states_step( if !consume_root_work(remaining) { return false; } - let (key, promise_state) = &mut states[state.index]; - match state.slot { - 0 => visitor.visit_metadata_usize_slot(key), - 1 => visitor.visit_raw_mut_ptr_slot(&mut promise_state.result_promise), - 2 => visitor.visit_raw_mut_ptr_slot(&mut promise_state.results_arr), - 3 => visitor.visit_raw_mut_ptr_slot(&mut promise_state.state_arr), + let slot = state.slot; + let Some(entry) = states.entry_at_mut(state.index) else { + break; + }; + let rekeyed = match slot { + 0 => visitor.visit_metadata_usize_slot(&mut entry.key), + 1 => visitor.visit_raw_mut_ptr_slot(&mut entry.value.result_promise), + 2 => visitor.visit_raw_mut_ptr_slot(&mut entry.value.results_arr), + 3 => visitor.visit_raw_mut_ptr_slot(&mut entry.value.state_arr), _ => false, }; + // Evacuation rewrote the key in place: the position is still + // valid, the key → position index is not. + if slot == 0 && rekeyed { + states.note_key_rewritten(); + } state.slot += 1; } state.index += 1; @@ -592,17 +600,26 @@ fn scan_promise_settle_listeners_step( if !consume_root_work(remaining) { return false; } - let (key, listener) = &mut listeners[state.index]; - match state.slot { - 0 => visitor.visit_metadata_usize_slot(key), - 1 => visitor.visit_raw_const_ptr_slot(&mut listener.on_fulfilled), - 2 => visitor.visit_raw_const_ptr_slot(&mut listener.on_rejected), + let slot = state.slot; + let Some(entry) = listeners.entry_at_mut(state.index) else { + break; + }; + let rekeyed = match slot { + 0 => visitor.visit_metadata_usize_slot(&mut entry.key), + 1 => visitor.visit_raw_const_ptr_slot(&mut entry.value.on_fulfilled), + 2 => visitor.visit_raw_const_ptr_slot(&mut entry.value.on_rejected), _ => false, }; + if slot == 0 && rekeyed { + listeners.note_key_rewritten(); + } state.slot += 1; } + let Some(entry) = listeners.entry_at_mut(state.index) else { + break; + }; if !crate::async_context::scan_snapshot_roots_mut_step( - &mut listeners[state.index].1.context, + &mut entry.value.context, visitor, &mut state.context_entry, &mut state.context_store, @@ -635,18 +652,27 @@ fn scan_promise_overflow_reactions_step( if !consume_root_work(remaining) { return false; } - let (key, reaction) = &mut reactions[state.index]; - match state.slot { - 0 => visitor.visit_metadata_usize_slot(key), - 1 => visitor.visit_raw_const_ptr_slot(&mut reaction.on_fulfilled), - 2 => visitor.visit_raw_const_ptr_slot(&mut reaction.on_rejected), - 3 => visitor.visit_raw_mut_ptr_slot(&mut reaction.next), + let slot = state.slot; + let Some(entry) = reactions.entry_at_mut(state.index) else { + break; + }; + let rekeyed = match slot { + 0 => visitor.visit_metadata_usize_slot(&mut entry.key), + 1 => visitor.visit_raw_const_ptr_slot(&mut entry.value.on_fulfilled), + 2 => visitor.visit_raw_const_ptr_slot(&mut entry.value.on_rejected), + 3 => visitor.visit_raw_mut_ptr_slot(&mut entry.value.next), _ => false, }; + if slot == 0 && rekeyed { + reactions.note_key_rewritten(); + } state.slot += 1; } + let Some(entry) = reactions.entry_at_mut(state.index) else { + break; + }; if !crate::async_context::scan_snapshot_roots_mut_step( - &mut reactions[state.index].1.context, + &mut entry.value.context, visitor, &mut state.context_entry, &mut state.context_store, @@ -932,17 +958,17 @@ pub(crate) fn test_clear_promise_scanner_roots() { pub(crate) fn test_park_promise_side_table_entries(promise: *mut Promise) { let key = promise as usize; super::reactions::PROMISE_SETTLE_LISTENERS.with(|listeners| { - listeners.borrow_mut().push(( + listeners.borrow_mut().push( key, super::reactions::PromiseSettleListener { on_fulfilled: std::ptr::null(), on_rejected: std::ptr::null(), context: capture_context(), }, - )); + ); }); super::reactions::PROMISE_OVERFLOW_REACTIONS.with(|reactions| { - reactions.borrow_mut().push(( + reactions.borrow_mut().push( key, super::reactions::OverflowReaction { on_fulfilled: std::ptr::null(), @@ -950,10 +976,10 @@ pub(crate) fn test_park_promise_side_table_entries(promise: *mut Promise) { next: std::ptr::null_mut(), context: capture_context(), }, - )); + ); }); super::combinators::PROMISE_ALL_STATES.with(|states| { - states.borrow_mut().push(( + states.borrow_mut().push( key, super::combinators::PromiseAllState { result_promise: std::ptr::null_mut(), @@ -961,7 +987,7 @@ pub(crate) fn test_park_promise_side_table_entries(promise: *mut Promise) { state_arr: std::ptr::null_mut(), index: 0, }, - )); + ); }); } @@ -969,12 +995,11 @@ pub(crate) fn test_park_promise_side_table_entries(promise: *mut Promise) { /// (settle listeners, overflow reactions, Promise.all states). #[cfg(test)] pub(crate) fn test_promise_side_table_counts_for(key: usize) -> (usize, usize, usize) { - let listeners = super::reactions::PROMISE_SETTLE_LISTENERS - .with(|l| l.borrow().iter().filter(|(k, _)| *k == key).count()); - let reactions = super::reactions::PROMISE_OVERFLOW_REACTIONS - .with(|r| r.borrow().iter().filter(|(k, _)| *k == key).count()); - let states = super::combinators::PROMISE_ALL_STATES - .with(|s| s.borrow().iter().filter(|(k, _)| *k == key).count()); + let listeners = + super::reactions::PROMISE_SETTLE_LISTENERS.with(|l| l.borrow_mut().count_for_key(key)); + let reactions = + super::reactions::PROMISE_OVERFLOW_REACTIONS.with(|r| r.borrow_mut().count_for_key(key)); + let states = super::combinators::PROMISE_ALL_STATES.with(|s| s.borrow_mut().count_for_key(key)); (listeners, reactions, states) } diff --git a/crates/perry-runtime/src/typed_feedback/tests.rs b/crates/perry-runtime/src/typed_feedback/tests.rs index 064452adee..b61c2bf1be 100644 --- a/crates/perry-runtime/src/typed_feedback/tests.rs +++ b/crates/perry-runtime/src/typed_feedback/tests.rs @@ -1761,13 +1761,21 @@ fn typed_feedback_object_set_fast_hits_learned_dynamic_key_transition() { let stored = crate::object::js_object_get_field_by_name_f64(second_obj, key); assert_eq!(stored.to_bits(), 12.0f64.to_bits()); + // #6084 item 6: the dynamic-write fast path is vetted PER RECEIVER, not by + // the process-global `GLOBAL_DESCRIPTORS_IN_USE` latch. That latch flips on + // any descriptor install anywhere — including ones the runtime itself + // performs and ones earlier tests in this process performed — and it used to + // force this learned transition onto the fallback path (the old expectation + // here was `fallback_calls == 2` whenever `descriptors_in_use()`). + // + // `second_obj` is a fresh plain object: no own descriptor + // (`OBJ_FLAG_HAS_DESCRIPTORS` clear), no recorded `setPrototypeOf` target, + // and `Object.prototype` owns no `dyn_fast_key_34` — so nothing can + // intercept the write and it must hit the learned transition regardless of + // what descriptors exist elsewhere in the process. let site = &typed_feedback_snapshot().sites[0]; - if crate::object::descriptors_in_use() { - assert_eq!(site.fallback_calls, 2); - } else { - assert_eq!(site.fallback_calls, 1); - assert!(site.guard_passes >= 1); - } + assert_eq!(site.fallback_calls, 1); + assert!(site.guard_passes >= 1); } #[test] diff --git a/crates/perry/tests/issue_6084_promise_settle_scaling.rs b/crates/perry/tests/issue_6084_promise_settle_scaling.rs new file mode 100644 index 0000000000..2df223b186 --- /dev/null +++ b/crates/perry/tests/issue_6084_promise_settle_scaling.rs @@ -0,0 +1,159 @@ +//! #6084 item 2: settling a promise must not cost O(table) in the promise +//! reaction side tables. +//! +//! `js_promise_resolve`/`reject` unconditionally drain three promise-pointer-keyed +//! side tables — `PROMISE_SETTLE_LISTENERS`, `PROMISE_OVERFLOW_REACTIONS` and +//! `PROMISE_ALL_STATES`. All three were `Vec<(usize, T)>` scanned END TO END on +//! every settle, so settling N promises that are parked in a table is O(N²). +//! Measured on `main` (2nd `.then` per promise, so each parks an overflow +//! reaction), settle time for N promises: 5k=18ms, 10k=73ms, 20k=779ms, +//! 40k=3503ms — against node's ~1ms. They are now backed by `PromiseKeyedTable` +//! (dense `Vec` for the GC scanners + an O(1) key index), so a settle drains +//! only its own key's entries. +//! +//! Pinned as a SCALING check rather than an absolute time: 4x the promises must +//! not cost ~16x the time. The margin is wide enough for a loaded CI host but +//! still fails the quadratic shape by a mile (on `main` the 40k/10k ratio is +//! ~48x, and the absolute numbers are seconds). + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(src: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + std::fs::write(&entry, src).expect("write entry"); + let output = dir.path().join("main_bin"); + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + let run = Command::new(&output).output().expect("run compiled binary"); + let stdout = String::from_utf8_lossy(&run.stdout).to_string(); + assert!( + run.status.success(), + "binary failed\nstdout:\n{stdout}\nstderr:\n{}", + String::from_utf8_lossy(&run.stderr) + ); + stdout +} + +/// Overflow reactions (2nd+ `.then` on a pending promise) — the table that +/// dominated the measured regression. +#[test] +fn settling_many_promises_with_overflow_reactions_is_not_quadratic() { + let stdout = compile_and_run( + r#" +function runOne(N: number): Promise { + const resolvers: ((v: number) => void)[] = [] + const ps: Promise[] = [] + for (let i = 0; i < N; i++) { + ps.push(new Promise((res) => { resolvers.push(res) })) + } + let sum = 0 + const tails: Promise[] = [] + for (const p of ps) { + p.then(() => {}) // inline reaction slot + tails.push(p.then((v: number) => { sum += v })) // -> overflow table + } + const done = Promise.all(tails) + const t0 = Date.now() + for (let i = 0; i < N; i++) resolvers[i](1) + const settleMs = Date.now() - t0 + return done.then(() => { + if (sum !== N) throw new Error("lost reactions: sum=" + sum + " want " + N) + return settleMs + }) +} + +// Warm, then compare 10k against 40k (4x the promises). +runOne(2000).then(() => runOne(10000)).then((small: number) => { + runOne(40000).then((large: number) => { + console.log("small_ms=" + small + " large_ms=" + large) + // Linear => ~4x. Quadratic => ~16x (main: 73ms -> 3503ms = 48x). + const quadratic = small >= 4 && large > small * 10 + console.log("QUADRATIC:" + quadratic) + console.log("DONE") + }) +}) +"#, + ); + + assert!( + stdout.contains("QUADRATIC:false") && stdout.contains("DONE"), + "settle scaling looks quadratic:\n{stdout}" + ); +} + +/// Reaction ORDER is observable and must survive the switch from a linear +/// order-preserving scan to the keyed table (which restores FIFO from each +/// entry's registration seq). Also covers a promise parked in several +/// `Promise.all` calls at once, plus rejection paths. +#[test] +fn reaction_order_and_multi_registration_are_preserved() { + let stdout = compile_and_run( + r#" +const order: string[] = [] + +// Many reactions on ONE promise: 1st uses the inline slot, the rest overflow. +// They must replay in registration (FIFO) order. +let resolveShared: (v: number) => void = () => {} +const shared = new Promise((res) => { resolveShared = res }) +for (let i = 0; i < 6; i++) { + const label = "r" + i + shared.then(() => { order.push(label) }) +} + +// The same pending promise feeding two Promise.all calls. +let resolveOther: (v: number) => void = () => {} +const other = new Promise((res) => { resolveOther = res }) +const allA = Promise.all([shared, other]) +const allB = Promise.all([shared, Promise.resolve(9)]) + +// Rejection side: catch must still fire for overflow reactions. (The derived +// promise of the first `.then` rejects, so give it a handler — an unhandled +// rejection would abort the process, not exercise the table.) +let rejectMe: (e: any) => void = () => {} +const bad = new Promise((_res, rej) => { rejectMe = rej }) +bad.then(() => { order.push("bad-then") }).catch(() => {}) +bad.catch(() => { order.push("bad-catch") }) + +resolveShared(1) +resolveOther(2) +rejectMe(new Error("x")) + +Promise.all([allA, allB]).then((rs: number[][]) => { + console.log("order:" + order.join(",")) + console.log("allA:" + rs[0].join(",")) + console.log("allB:" + rs[1].join(",")) + console.log("DONE") +}) +"#, + ); + + for needle in [ + "order:r0,r1,r2,r3,r4,r5,bad-catch", + "allA:1,2", + "allB:1,9", + "DONE", + ] { + assert!( + stdout.contains(needle), + "expected `{needle}` in output:\n{stdout}" + ); + } +} diff --git a/crates/perry/tests/issue_6084_write_fast_path_per_object.rs b/crates/perry/tests/issue_6084_write_fast_path_per_object.rs new file mode 100644 index 0000000000..cbf54f0f27 --- /dev/null +++ b/crates/perry/tests/issue_6084_write_fast_path_per_object.rs @@ -0,0 +1,300 @@ +//! #6084 item 6: one `Object.freeze` must not disable the dynamic-write fast +//! path process-wide. +//! +//! Both dynamic-write fast paths (`js_object_set_field_by_name_transition_fast` +//! and the transition-cache branch inside `js_object_set_field_by_name`) used to +//! be gated on the process-global `GLOBAL_DESCRIPTORS_IN_USE` latch, which flips +//! on ANY descriptor install anywhere — including the ones the runtime itself +//! performs (an Error's `stack` accessor, `arguments` objects, tagged-template +//! `raw`, typed-array props) and a userland `Object.freeze` on a completely +//! unrelated object. Once flipped it never reverts, so every dynamic property +//! write in the process fell back to the O(own-key-count) `[[Set]]` walk. +//! +//! The gate is now per-receiver: an own descriptor is visible in the object's +//! `OBJ_FLAG_HAS_DESCRIPTORS` GcHeader bit, and only prototype-level installs +//! (`Object.prototype`, a recorded `setPrototypeOf` target, a class prototype) +//! need a chain walk — mirroring what `ordinary_set`'s #5054 fast path already +//! did per receiver. +//! +//! These tests pin the CORRECTNESS half: every interception source must still +//! intercept a plain-data write after the fast path is re-enabled. The write +//! fast path was effectively dead on `main` (the latch is set before user code +//! runs), so this re-enables a path that needs its semantics nailed down. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(src: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + std::fs::write(&entry, src).expect("write entry"); + let output = dir.path().join("main_bin"); + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + let run = Command::new(&output).output().expect("run compiled binary"); + let stdout = String::from_utf8_lossy(&run.stdout).to_string(); + assert!( + run.status.success(), + "binary failed\nstdout:\n{stdout}\nstderr:\n{}", + String::from_utf8_lossy(&run.stderr) + ); + stdout +} + +fn assert_lines(stdout: &str, needles: &[&str]) { + for needle in needles { + assert!( + stdout.contains(needle), + "expected `{needle}` in output:\n{stdout}" + ); + } +} + +/// An unrelated `Object.freeze` must not change the semantics of writes to +/// other objects — and every interception source must still fire. +#[test] +fn unrelated_freeze_keeps_write_semantics_for_every_interception_source() { + let stdout = compile_and_run( + r#" +// Flip the process-global descriptor latch via an object nobody writes to. +Object.freeze({ unrelated: 1 }) + +// 1. Plain fresh object: own data writes still land. +const plain: any = {} +plain.a = 1 +plain.b = 2 +plain.a = 3 +console.log("plain:", plain.a, plain.b, Object.keys(plain).join(",")) + +// 2. Inherited SETTER on Object.prototype must still intercept. +Object.defineProperty(Object.prototype, "viaProto", { + set(v: any) { (this as any)._proto_set = v }, + get() { return (this as any)._proto_set }, + configurable: true, +}) +const p2: any = {} +p2.viaProto = 42 +console.log("protoSetter:", p2._proto_set, Object.prototype.hasOwnProperty.call(p2, "viaProto")) + +// 3. Inherited NON-WRITABLE data on Object.prototype must still block. +Object.defineProperty(Object.prototype, "roProto", { value: 7, writable: false, configurable: true }) +const p3: any = {} +p3.roProto = 99 +console.log("protoRO:", p3.roProto, Object.prototype.hasOwnProperty.call(p3, "roProto")) + +// 4. OWN accessor on the receiver must still intercept. +const p4: any = {} +Object.defineProperty(p4, "own", { + set(v: any) { (this as any)._own = v * 2 }, + get() { return (this as any)._own }, + configurable: true, +}) +p4.own = 21 +console.log("ownSetter:", p4._own) + +// 5. OWN non-writable data on the receiver must still block. +const p5: any = {} +Object.defineProperty(p5, "ro", { value: 5, writable: false, configurable: true }) +p5.ro = 123 +console.log("ownRO:", p5.ro) + +// 6. A frozen object itself still rejects writes (own + new keys). +const frozen: any = { x: 1 } +Object.freeze(frozen) +try { frozen.x = 2 } catch (e) {} +try { frozen.fresh = 1 } catch (e) {} +console.log("frozen:", frozen.x, frozen.fresh === undefined) + +// 7. A sealed object rejects NEW keys but allows existing ones. +const sealed: any = { y: 1 } +Object.seal(sealed) +sealed.y = 2 +try { sealed.brand = 1 } catch (e) {} +console.log("sealed:", sealed.y, sealed.brand === undefined) + +console.log("DONE") +"#, + ); + + assert_lines( + &stdout, + &[ + "plain: 3 2 a,b", + "protoSetter: 42 false", + "protoRO: 7 false", + "ownSetter: 42", + "ownRO: 5", + "frozen: 1 true", + "sealed: 2 true", + "DONE", + ], + ); +} + +/// Prototype-level interception reached through `setPrototypeOf` / `Object.create` +/// and through a class prototype — the per-receiver flag cannot see these, so the +/// gate has to walk the chain. +#[test] +fn prototype_chain_setters_still_intercept_after_unrelated_freeze() { + let stdout = compile_and_run( + r#" +Object.freeze({ unrelated: 1 }) + +// setPrototypeOf onto a proto carrying a setter. +const protoA: any = {} +Object.defineProperty(protoA, "hooked", { + set(v: any) { (this as any)._a = v + 1 }, + get() { return (this as any)._a }, + configurable: true, +}) +const viaSet: any = {} +Object.setPrototypeOf(viaSet, protoA) +viaSet.hooked = 10 +console.log("setProtoOf:", viaSet._a, Object.prototype.hasOwnProperty.call(viaSet, "hooked")) + +// Object.create with the same proto. +const viaCreate: any = Object.create(protoA) +viaCreate.hooked = 20 +console.log("objectCreate:", viaCreate._a, Object.prototype.hasOwnProperty.call(viaCreate, "hooked")) + +// Class accessor on the prototype (vtable, not the address-keyed tables). +class Base { + _v = 0 + set tracked(v: number) { this._v = v * 3 } + get tracked(): number { return this._v } +} +const inst: any = new Base() +inst.tracked = 5 +console.log("classSetter:", inst._v) + +// Object.defineProperty on a class prototype intercepts instance writes. +class Other { z = 0 } +Object.defineProperty(Other.prototype, "onProto", { + set(v: any) { (this as any)._z = v - 1 }, + get() { return (this as any)._z }, + configurable: true, +}) +const o2: any = new Other() +o2.onProto = 8 +console.log("classProtoDesc:", o2._z, Object.prototype.hasOwnProperty.call(o2, "onProto")) + +// A plain data write on the same class instance must still be a plain write. +o2.plainField = 99 +console.log("classPlainWrite:", o2.plainField) + +console.log("DONE") +"#, + ); + + assert_lines( + &stdout, + &[ + "setProtoOf: 11 false", + "objectCreate: 21 false", + "classSetter: 15", + "classProtoDesc: 7 false", + "classPlainWrite: 99", + "DONE", + ], + ); +} + +/// A descriptor installed on one object must not leak onto a different object — +/// the point of a per-object gate. Also pins that a freeze on object A does not +/// make writes to object B throw or silently drop. +#[test] +fn descriptors_do_not_leak_across_objects() { + let stdout = compile_and_run( + r#" +const a: any = { locked: 1 } +Object.defineProperty(a, "locked", { value: 1, writable: false, configurable: true }) +Object.freeze(a) + +// A different object with the SAME key name must be freely writable. +const b: any = {} +b.locked = 2 +b.locked = 3 +console.log("b:", b.locked, Object.keys(b).join(",")) + +// And a whole batch of fresh objects keeps taking plain data writes. +let total = 0 +for (let i = 0; i < 2000; i++) { + const o: any = {} + o.locked = i + o.other = i + 1 + total += o.locked + o.other +} +console.log("batch:", total) +console.log("a-still-frozen:", a.locked, Object.isFrozen(a)) +console.log("DONE") +"#, + ); + + assert_lines( + &stdout, + &[ + "b: 3 locked", + "batch: 4000000", + "a-still-frozen: 1 true", + "DONE", + ], + ); +} + +/// The perf half: the identical write loop must not get materially slower just +/// because an unrelated object was frozen. On `main` the freeze flipped the +/// global latch and pushed every write onto the slow `[[Set]]` walk. +/// +/// Pinned as a RATIO of two runs of the same loop in the same process, so it is +/// insensitive to machine speed and CI contention (both halves are perturbed +/// together). The regression this guards against is unbounded — the slow path is +/// O(own-key-count) per write — so a generous ceiling still catches it. +#[test] +fn unrelated_freeze_does_not_slow_the_write_loop() { + let stdout = compile_and_run( + r#" +let sink = 0 +function build(n: number): number { + const t0 = Date.now() + for (let i = 0; i < n; i++) { + const o: any = {} + o.a = i + o.b = i + 1 + o.c = i + 2 + sink += o.c + } + return Date.now() - t0 +} + +const N = 200000 +build(20000) // warm +const before = build(N) +Object.freeze({ unrelated: 1 }) // <- must NOT poison the fast path +const after = build(N) + +// Guard against a zero-length window on a very fast machine. +const ratio = after / Math.max(before, 1) +console.log("before=" + before + " after=" + after + " ratio=" + ratio.toFixed(2)) +console.log("REGRESSED:" + (before >= 4 && ratio > 2.0)) +console.log("sink=" + (sink > 0)) +"#, + ); + + assert_lines(&stdout, &["REGRESSED:false", "sink=true"]); +}