Skip to content

Commit be5ed2b

Browse files
proggeramlugRalph Küpper
andauthored
perf(promise,object): O(1) promise settle tables + per-object dynamic-write gate (#6084 items 2 and 6) (#6327)
* perf(promise,object): O(1) promise settle tables + per-object write gate (#6084) Closes the two remaining measured defects on #6084. Item 2 — promise settle was O(N^2). js_promise_resolve/reject drain three promise-pointer-keyed side tables (PROMISE_SETTLE_LISTENERS, PROMISE_OVERFLOW_REACTIONS, PROMISE_ALL_STATES); all three were Vec<(usize, T)> scanned END TO END on every settle, so settling N parked promises is quadratic. The per-sweep death hook had the same shape. Not a data-structure swap: these are GC root side tables whose Vec is load-bearing twice — the incremental root scanner resumes by position (PromiseRootScanState { index, slot }), and evacuation rewrites the key IN PLACE (visit_metadata_usize_slot), which would silently break a HashMap's bucket invariant. New promise/keyed_table.rs: PromiseKeyedTable<T>, the 1:N analogue of the PromiseContextStore split (#6267). The dense Vec stays exactly as the GC traversal/rewrite surface (same access pattern, same append + swap-remove discipline); an O(1) key -> positions index is layered on top as derived state. Every GC path that can invalidate it just sets index_dirty — nothing patches the index from inside a GC path — and the next lookup rebuilds it from the entries alone, at most once per cycle that actually moved a promise. A monotonic per-entry seq restores the observable per-key FIFO order that swap_remove destroys, and is what makes the index reconstructible from entries alone. Measured (release, min of 7), N pending promises each with a 2nd .then: 40k settle 1117ms -> 14ms (80x; node 1ms). 4x the promises costs main 16.0x the time (quadratic) and this branch 7x. The Promise.all shape (one reaction per input, inline slot) is unchanged: 8ms vs 8ms. Item 6 — the dynamic-write fast path was disabled process-wide. Both write fast paths gated on the process-global GLOBAL_DESCRIPTORS_IN_USE latch, which flips on ANY descriptor install anywhere and never reverts. The issue reports "+29% after an unrelated Object.freeze"; in fact the latch is already set before user code runs (the runtime installs latching descriptors during startup/first use: an Error's stack accessor, arguments objects, tagged-template raw, typed-array props), so these fast paths were effectively dead in every program. plain_data_write_may_intercept() replaces the latch on both gates with the same per-receiver predicate ordinary_set's #5054 fast path already applies: own descriptors are visible in OBJ_FLAG_HAS_DESCRIPTORS (folded into the existing FROZEN|SEALED|NO_EXTEND mask, so free); only prototype-level installs can intercept a write to an object whose own flag is clear, so those are checked against the actual chain (Object.prototype per key, a recorded setPrototypeOf target, or the class chain); typed arrays and exotic-expando hosts are excluded outright. When no descriptor exists anywhere it short-circuits on the same single relaxed load the old gate did. Measured: 1M objects x 3 new props, with no descriptor call anywhere in the program, 2845ms -> 1465ms (1.9x). After an unrelated Object.freeze, 3026ms -> 1557ms. Correctness: item 6 re-enables a path that was dead on main, so its semantics are pinned rather than assumed. Every interception source still intercepts, byte-identical to node — inherited setter and non-writable on Object.prototype, own accessor and own non-writable, frozen/sealed receivers, setPrototypeOf and Object.create protos with a setter, class accessors, and defineProperty on a class prototype — while plain data writes on the same objects keep the fast path. Reaction FIFO order across the inline slot plus overflow entries is preserved. The GC rekey path is covered deterministically by gc/tests/copying/promise_side_tables.rs (test_live_promise_side_table_entries_rekeyed_by_copied_minor moves the promise and asserts the entries are findable under the new address). cargo test -p perry-runtime: 1271 passed, 0 failed. Gap suite: no new failures. typed_feedback_object_set_fast_hits_learned_dynamic_key_transition asserted the old semantics ("global latch set => fast path must bail"), which is exactly what item 6 removes; it now asserts the per-receiver behaviour. * perf(promise): make displaced-entry relocation O(1) (#6084 review) CodeRabbit on #6327: the O(1) key -> positions index left a residual quadratic in the *relocation* path, and the benchmark that shipped with the PR (many keys, ONE entry each -- the Promise.all shape) cannot see it. take_all(A) swap_removes A's entries; each removal displaces the table's last entry -- always a foreign one -- into the vacated slot, and that entry's key's slot list has to be repointed at its new home. Slots::relocate found the old position by SCANNING the list, which is O(M) for a key with M parked entries. So `for(M) a.then(f); for(M) b.then(f); resolveA()` -- all of A's entries ahead of all of B's -- drains A in Theta(M^2): the exact defect this table exists to remove, just relocated from the table into a slot list. `p.then()` past the inline slot parks in PROMISE_OVERFLOW_REACTIONS, so this is reachable from plain TS. Measured (release), M entries on key A and M on key B, time to drain A: M=20k 41.3ms -> 0.2ms M=80k 638.6ms -> 0.7ms Before: 4x the entries cost 15.5x the time (quadratic is 16x). After: 3.5x. Fix: each Entry records `slot`, its own offset within its key's slot list -- the reverse of the forward index. A displaced entry names the slot to repoint, so relocation is one indexed store. `slot` is derived state on exactly the same footing as `index` itself: maintained by push/ensure_index only, meaningless while index_dirty, rebuilt wholesale by ensure_index, and never touched from a GC path. The dense Vec remains the GC traversal/rewrite surface and the monotonic seq still carries FIFO order. Adds the two-heavily-populated-keys regression test (it fails on the old scan with the 4x-entries/16x-time signature), and assert_invariants now checks the reverse index -- which the randomized differential test, now asserting as it goes, exercises against the naive model. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 81ab8e5 commit be5ed2b

11 files changed

Lines changed: 1315 additions & 158 deletions

File tree

crates/perry-runtime/src/object/descriptor_state.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,75 @@ pub(crate) fn object_has_descriptors(obj: usize) -> bool {
331331
false
332332
}
333333

334+
/// #6084 (item 6): can anything intercept a plain-data write of `key` to the
335+
/// `GC_TYPE_OBJECT` at `addr` (own accessor / non-writable descriptor, or an
336+
/// inherited setter / non-writable data property), so the dynamic-write
337+
/// transition-cache fast path must be skipped for THIS write?
338+
///
339+
/// Replaces the process-global `GLOBAL_DESCRIPTORS_IN_USE` latch that used to
340+
/// gate both dynamic-write fast paths. That latch flips on *any* descriptor
341+
/// install anywhere — so a single `Object.freeze` on a completely unrelated
342+
/// object (or any library that freezes one config object at import time)
343+
/// permanently pushed EVERY dynamic property write in the process onto the
344+
/// O(own-key-count) slow walk. Measured: 1M objects × 3 new props = 5281 ms;
345+
/// the identical loop after one unrelated `Object.freeze` = 6807 ms (+29%,
346+
/// and it never recovers).
347+
///
348+
/// The vetting here is the same predicate `ordinary_set`'s #5054 fast path
349+
/// (`proxy.rs`) already applies per receiver, and the same receiver-level /
350+
/// prototype-level split as the #5654 read-side guard:
351+
/// - own descriptors are visible per-object in `OBJ_FLAG_HAS_DESCRIPTORS`
352+
/// (set by [`note_descriptor_target`], travels with the object on
353+
/// evacuation, and is clear on every fresh allocation);
354+
/// - only *prototype*-level installs can intercept a write to an object whose
355+
/// own flag is clear, and those are checked against the actual prototype
356+
/// chain — `Object.prototype` per-key via [`object_proto_may_intercept_key`]
357+
/// (a blanket check made wide dynamic builds O(n²), see #5054), a recorded
358+
/// `setPrototypeOf` target, or the class chain via
359+
/// [`class_instance_set_may_intercept`].
360+
///
361+
/// Conservative in every uncertain case (returns `true` = take the slow path).
362+
/// `caller` must have already established that `addr` is a `GC_TYPE_OBJECT`
363+
/// whose frozen/sealed/non-extensible flags are clear.
364+
pub(crate) unsafe fn plain_data_write_may_intercept(addr: usize, class_id: u32, key: f64) -> bool {
365+
// Nothing has ever installed a descriptor or accessor: no per-object work at
366+
// all, just the one relaxed load the old gate did.
367+
if !descriptors_in_use() {
368+
return false;
369+
}
370+
371+
// A descriptor exists SOMEWHERE. Vet this receiver and its prototype chain
372+
// instead of latching the whole process onto the slow path.
373+
374+
// Own accessor / non-writable descriptor on this exact object.
375+
if object_has_descriptors(addr) {
376+
return true;
377+
}
378+
379+
// `note_descriptor_target` cannot record the per-object flag for typed
380+
// arrays (small ones are plain-alloc'd without a GcHeader) or for exotic
381+
// expando hosts, so their descriptors are invisible to the flag check
382+
// above — never fast-path them once any descriptor exists.
383+
if crate::typedarray::lookup_typed_array_kind(addr).is_some() {
384+
return true;
385+
}
386+
let value = crate::value::js_nanbox_pointer(addr as i64);
387+
if super::exotic_expando::exotic_expando_kind_of_value(value).is_some() {
388+
return true;
389+
}
390+
391+
if class_id == 0 {
392+
// Plain object. Its prototype is exactly `Object.prototype` unless a
393+
// `setPrototypeOf` target was recorded for it.
394+
super::prototype_chain::object_static_prototype(addr).is_some()
395+
|| object_proto_may_intercept_key(key)
396+
} else {
397+
// Class instance: an inherited accessor / non-writable data property
398+
// anywhere in the chain intercepts the write.
399+
class_instance_set_may_intercept(addr, class_id, key)
400+
}
401+
}
402+
334403
/// Store a property descriptor for (obj, key).
335404
pub(crate) fn set_property_attrs(obj: usize, key: String, attrs: PropertyAttrs) {
336405
note_descriptor_target(obj);

crates/perry-runtime/src/object/field_set_by_name.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast(
5151
if obj.is_null() || (obj as usize) < crate::gc::GC_HEADER_SIZE + 0x1000 {
5252
return 0;
5353
}
54-
if GLOBAL_DESCRIPTORS_IN_USE.load(Ordering::Relaxed) {
55-
return 0;
56-
}
5754

5855
let scope = crate::gc::RuntimeHandleScope::new();
5956
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(
8077
if object_flags
8178
& (crate::gc::OBJ_FLAG_FROZEN
8279
| crate::gc::OBJ_FLAG_SEALED
83-
| crate::gc::OBJ_FLAG_NO_EXTEND)
80+
| crate::gc::OBJ_FLAG_NO_EXTEND
81+
// #6084 item 6: an own descriptor on THIS object (accessor or
82+
// non-writable) must route through the full setter semantics.
83+
| crate::gc::OBJ_FLAG_HAS_DESCRIPTORS)
8484
!= 0
8585
{
8686
return 0;
@@ -89,6 +89,18 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast(
8989
return 0;
9090
}
9191

92+
// #6084 item 6: this used to be a `GLOBAL_DESCRIPTORS_IN_USE` check at
93+
// the top of the function — one `Object.freeze` anywhere in the process
94+
// (even on an unrelated object) permanently disabled this fast path for
95+
// every object. Vet the receiver's own flag (above) and its prototype
96+
// chain (here) instead. `class_id` is 0 at this point, so the only
97+
// inherited interceptor is `Object.prototype` (or a recorded
98+
// `setPrototypeOf` target).
99+
let key_f64 = f64::from_bits(JSValue::string_ptr(key as *mut _).bits());
100+
if super::plain_data_write_may_intercept(obj as usize, 0, key_f64) {
101+
return 0;
102+
}
103+
92104
let key_gc =
93105
(key as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
94106
if (*key_gc).obj_type != crate::gc::GC_TYPE_STRING {
@@ -910,10 +922,24 @@ pub extern "C" fn js_object_set_field_by_name(
910922
}
911923

912924
// FAST PATH: shape-transition cache with interned string pointer identity.
925+
//
926+
// #6084 item 6: the descriptor gate here used to be the process-global
927+
// `GLOBAL_DESCRIPTORS_IN_USE` latch, so ONE `Object.freeze` anywhere
928+
// (even on an object never written to again) permanently forced every
929+
// dynamic write in the process down the O(own-key-count) slow walk
930+
// below. It is now vetted per receiver: an own descriptor is visible in
931+
// this object's `OBJ_FLAG_HAS_DESCRIPTORS`, and only prototype-level
932+
// interceptors need a chain walk.
933+
let has_own_descriptors = obj_flags & crate::gc::OBJ_FLAG_HAS_DESCRIPTORS != 0;
913934
if !key.is_null()
914935
&& !is_frozen
915936
&& !is_sealed_or_no_extend
916-
&& !GLOBAL_DESCRIPTORS_IN_USE.load(Ordering::Relaxed)
937+
&& !has_own_descriptors
938+
&& !super::plain_data_write_may_intercept(
939+
obj as usize,
940+
(*obj).class_id,
941+
f64::from_bits(JSValue::string_ptr(key as *mut _).bits()),
942+
)
917943
{
918944
if let Some((next_keys, slot_idx)) =
919945
transition_cache_lookup(prev_keys_usize, interned_key)

crates/perry-runtime/src/object/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,11 @@ pub(crate) use descriptor_state::{
157157
descriptors_in_use, disable_class_field_inline_guard, get_accessor_descriptor,
158158
get_property_attrs, json_object_getter_value, mark_all_keys, note_descriptor_target,
159159
object_has_descriptors, object_proto_descriptors_in_use, object_proto_may_intercept_key,
160-
prune_dead_descriptor_owner_entries, reflect_getter_closure_bits, set_accessor_descriptor,
161-
set_builtin_accessor_descriptor, set_builtin_property_attrs, set_property_attrs,
162-
AccessorDescriptor, PropertyAttrs, ACCESSORS_IN_USE, ACCESSOR_DESCRIPTORS,
163-
GLOBAL_DESCRIPTORS_IN_USE, PROPERTY_ATTRS_IN_USE, PROPERTY_DESCRIPTORS,
160+
plain_data_write_may_intercept, prune_dead_descriptor_owner_entries,
161+
reflect_getter_closure_bits, set_accessor_descriptor, set_builtin_accessor_descriptor,
162+
set_builtin_property_attrs, set_property_attrs, AccessorDescriptor, PropertyAttrs,
163+
ACCESSORS_IN_USE, ACCESSOR_DESCRIPTORS, GLOBAL_DESCRIPTORS_IN_USE, PROPERTY_ATTRS_IN_USE,
164+
PROPERTY_DESCRIPTORS,
164165
};
165166
pub use this_binding::{
166167
js_implicit_this_get, js_implicit_this_get_sloppy, js_implicit_this_set, js_new_target_get,

crates/perry-runtime/src/promise/combinators.rs

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use super::*;
66
use std::os::raw::c_int;
77

8+
use super::keyed_table::PromiseKeyedTable;
9+
810
use super::assimilate::{
911
assimilate_via_then_property, enqueue_thenable_job, get_then_action,
1012
promise_resolve_assimilating, thenable_job_reject_fn, thenable_job_resolve_fn,
@@ -19,8 +21,12 @@ pub(super) struct PromiseAllState {
1921
}
2022

2123
thread_local! {
22-
pub(super) static PROMISE_ALL_STATES: RefCell<Vec<(usize, PromiseAllState)>> =
23-
const { RefCell::new(Vec::new()) };
24+
/// Keyed by input-promise address. See `keyed_table.rs`: a dense `Vec` is
25+
/// still the GC scanners' traversal/rewrite surface, with an O(1) key index
26+
/// layered on top (#6084 item 2 — this used to be a raw `Vec` that every
27+
/// settlement scanned end to end).
28+
pub(super) static PROMISE_ALL_STATES: RefCell<PromiseKeyedTable<PromiseAllState>> =
29+
const { RefCell::new(PromiseKeyedTable::new()) };
2430
}
2531

2632
/// Drain ALL `PromiseAllState` entries associated with `promise`.
@@ -42,20 +48,7 @@ pub(super) fn promise_all_take_all_handlers(promise: *mut Promise) -> Vec<Promis
4248
if promise.is_null() {
4349
return Vec::new();
4450
}
45-
PROMISE_ALL_STATES.with(|states| {
46-
let mut states = states.borrow_mut();
47-
let key = promise as usize;
48-
let mut drained = Vec::new();
49-
let mut i = 0;
50-
while i < states.len() {
51-
if states[i].0 == key {
52-
drained.push(states.swap_remove(i).1);
53-
} else {
54-
i += 1;
55-
}
56-
}
57-
drained
58-
})
51+
PROMISE_ALL_STATES.with(|states| states.borrow_mut().take_all(promise as usize))
5952
}
6053

6154
#[inline]
@@ -70,11 +63,17 @@ pub(super) fn promise_all_settle(state: PromiseAllState, value: f64, is_fulfille
7063
pub(super) fn scan_promise_all_states_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_>) {
7164
PROMISE_ALL_STATES.with(|states| {
7265
let mut states = states.borrow_mut();
73-
for (key, state) in states.iter_mut() {
74-
visitor.visit_metadata_usize_slot(key);
75-
visitor.visit_raw_mut_ptr_slot(&mut state.result_promise);
76-
visitor.visit_raw_mut_ptr_slot(&mut state.results_arr);
77-
visitor.visit_raw_mut_ptr_slot(&mut state.state_arr);
66+
let mut rekeyed = false;
67+
for entry in states.iter_mut() {
68+
// Evacuation rewrites the key IN PLACE — the position stays valid,
69+
// but the key → position index no longer does. Rebuild it lazily.
70+
rekeyed |= visitor.visit_metadata_usize_slot(&mut entry.key);
71+
visitor.visit_raw_mut_ptr_slot(&mut entry.value.result_promise);
72+
visitor.visit_raw_mut_ptr_slot(&mut entry.value.results_arr);
73+
visitor.visit_raw_mut_ptr_slot(&mut entry.value.state_arr);
74+
}
75+
if rekeyed {
76+
states.note_key_rewritten();
7877
}
7978
});
8079
}
@@ -88,24 +87,19 @@ pub(super) fn remove_all_states_for_dead_promise(promise: *mut Promise) {
8887
return;
8988
}
9089
let key = promise as usize;
91-
PROMISE_ALL_STATES.with(|states| {
92-
let mut states = states.borrow_mut();
93-
if !states.is_empty() {
94-
states.retain(|(k, _)| *k != key);
95-
}
96-
});
90+
PROMISE_ALL_STATES.with(|states| states.borrow_mut().remove_key(key));
9791
}
9892

9993
/// Copied-minor from-space cleanup for `PROMISE_ALL_STATES` — see
10094
/// `cleanup_copied_minor_settle_listeners_for_gc` (`reactions.rs`).
10195
pub(super) fn cleanup_copied_minor_all_states_for_gc() {
10296
use super::CopiedMinorPromiseKeyFate::*;
10397
PROMISE_ALL_STATES.with(|states| {
104-
states.borrow_mut().retain_mut(|(key, _)| {
105-
match super::copied_minor_promise_key_fate(*key) {
98+
states.borrow_mut().retain_mut(|entry| {
99+
match super::copied_minor_promise_key_fate(entry.key) {
106100
Keep => true,
107101
Rekey(new_key) => {
108-
*key = new_key;
102+
entry.key = new_key;
109103
true
110104
}
111105
Drop => false,
@@ -891,7 +885,7 @@ pub extern "C" fn js_promise_all(promises_arr: *const crate::array::ArrayHeader)
891885
}
892886
PromiseState::Pending => {
893887
PROMISE_ALL_STATES.with(|states| {
894-
states.borrow_mut().push((promise_ptr as usize, state));
888+
states.borrow_mut().push(promise_ptr as usize, state);
895889
});
896890
set_promise_callback_context(promise_ptr);
897891
}
@@ -1721,12 +1715,8 @@ mod tests {
17211715
assert_eq!((*all_b).state, PromiseState::Pending);
17221716

17231717
// PROMISE_ALL_STATES must hold TWO entries keyed on `shared`.
1724-
let registered = PROMISE_ALL_STATES.with(|s| {
1725-
s.borrow()
1726-
.iter()
1727-
.filter(|(k, _)| *k == shared as usize)
1728-
.count()
1729-
});
1718+
let registered =
1719+
PROMISE_ALL_STATES.with(|s| s.borrow_mut().count_for_key(shared as usize));
17301720
assert_eq!(
17311721
registered, 2,
17321722
"expected two Promise.all states keyed on the shared pending promise"

0 commit comments

Comments
 (0)