diff --git a/changelog.d/11373-timer-churn-constant-factor.md b/changelog.d/11373-timer-churn-constant-factor.md new file mode 100644 index 0000000000..c2ed3f95d6 --- /dev/null +++ b/changelog.d/11373-timer-churn-constant-factor.md @@ -0,0 +1,7 @@ +### Performance + +- `setTimeout` + `unref()` + `clearTimeout` churn (rate-limiter-flexible's `MemoryStorage` shape, #10522) drops from ~13.7 k to ~7.8 k instructions per round. The id-count and live-count cliffs the issue measured were already gone (per-agent timer heap with an id index; FIFO eviction of retired ids in the ref-state registry); what remained was constant factor: + - `t.ref()` / `t.unref()` / `t.hasRef()` / `t.refresh()` on a timer handle no longer walk the whole dynamic dispatch tower (~6.3 k instructions: handle/primitive probes, a by-name prototype read that allocated the key string, a closure rebind and a call through the thunk). `timer::try_timer_method_fast_dispatch` answers the call directly when it provably resolves to the family's own native method — the receiver still links to its family prototype with no own keys, not dictionary-mode and no accessor for the name, and the prototype's own data slot for the name still holds a closure over that method's thunk. There is no cache to invalidate: every user override (`t.unref = f`, `Timeout.prototype.unref = f`, a getter via `defineProperty`, `Object.setPrototypeOf(t, …)`, `delete`) fails a check and takes the tower as before. + - The timer ref-state registry and the `async_hooks` resource table hash their internal monotonic ids with aHash instead of SipHash. + - `async_hooks`' init emission returns before allocating the resource type-name string when no hook is enabled (it was allocated once per `setTimeout` and then discarded). +- Measured on Linux x64 (callgrind, `PERRY_NO_AUTO_OPTIMIZE=1`): 120 k-id churn costs the same per op as 60 k-id churn, and the 5,000-live variant is within ~1.2× of the 50-live one. Unit coverage: `timer::tests_inline::method_fast_path_tests` asserts the fast path's answers and that each override above sends the call back to the tower. diff --git a/crates/perry-runtime/src/async_hooks.rs b/crates/perry-runtime/src/async_hooks.rs index 1781ba54d7..f944f94775 100644 --- a/crates/perry-runtime/src/async_hooks.rs +++ b/crates/perry-runtime/src/async_hooks.rs @@ -160,8 +160,12 @@ struct HookRecord { // thread resolves to, not the scanner's reach. per_test_global! { static HOOKS: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); - static RESOURCES: LazyLock>> = - LazyLock::new(|| Mutex::new(HashMap::new())); + // #10522: aHash, not SipHash — every `setTimeout` inserts and every clear + // or fire removes an entry, keyed by our own monotonic async id, so there + // is no untrusted key to defend against and SipHash's rounds were a + // measurable share of timer churn. + static RESOURCES: LazyLock>> = + LazyLock::new(|| Mutex::new(HashMap::default())); static GC_DESTROY_QUEUE: Mutex> = Mutex::new(VecDeque::new()); static NEXT_CONTEXT_SNAPSHOT_ID: AtomicUsize = AtomicUsize::new(1); static CONTEXT_SNAPSHOTS: LazyLock< @@ -976,6 +980,12 @@ pub fn init_resource_with_trigger( } fn emit_init(async_id: u64, type_name: &str, trigger_async_id: u64, resource: f64) { + // `with_hook_callbacks` returns at once without hooks; check first so the + // common no-hooks case does not allocate the type-name string per resource + // (one per `setTimeout`, #10522). + if !hooks_active() { + return; + } let scope = crate::gc::RuntimeHandleScope::new(); let resource_handle = scope.root_nanbox_f64(resource); let type_ptr = js_string_from_bytes(type_name.as_ptr(), type_name.len() as u32); diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index b6fb9fc753..4cb4403c9c 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -1295,6 +1295,14 @@ pub unsafe extern "C-unwind" fn js_native_call_method( { return result; } + // #10522: `t.unref()` & co. on a pristine timer handle; the guard proves the + // tower would resolve the family's own native method (`timer::handle_object`). + if !method_name_ptr.is_null() { + let name = std::slice::from_raw_parts(method_name_ptr as *const u8, method_name_len); + if let Some(result) = crate::timer::try_timer_method_fast_dispatch(object, name) { + return result; + } + } // Get the method name (parsed early for depth guard logging). // diff --git a/crates/perry-runtime/src/timer.rs b/crates/perry-runtime/src/timer.rs index acc972d111..aa1c918b49 100644 --- a/crates/perry-runtime/src/timer.rs +++ b/crates/perry-runtime/src/timer.rs @@ -389,7 +389,7 @@ pub(crate) use gc_scan::{new_timer_root_scan_state, scan_timer_roots_mut_step}; pub use ref_states::is_known_timer_id; use ref_states::register_scheduled_timer; -pub(crate) use handle_object::scan_timer_prototype_roots_mut; +pub(crate) use handle_object::{scan_timer_prototype_roots_mut, try_timer_method_fast_dispatch}; // `crate::timer::`-qualified only from unit tests (`timer/tests_inline.rs`, // `gc/tests/handle_bound_method_name.rs`, `timer/ref_states.rs`'s test module); // an unconditional `pub(crate) use` would be an unused import in a lib build diff --git a/crates/perry-runtime/src/timer/handle_object.rs b/crates/perry-runtime/src/timer/handle_object.rs index b6960475cb..68e09ffe0b 100644 --- a/crates/perry-runtime/src/timer/handle_object.rs +++ b/crates/perry-runtime/src/timer/handle_object.rs @@ -184,6 +184,109 @@ extern "C" fn timer_proto_refresh_thunk(_c: *const crate::closure::ClosureHeader this } +/// #10522: `t.ref()` / `t.unref()` / `t.hasRef()` / `t.refresh()` answered +/// without the generic dispatch tower, when the call provably resolves to the +/// native method this family installed. +/// +/// A timer handle carries a `meta` record (its state word) and a recorded +/// prototype, so the class-vtable fast path (#7769) refuses it, and every call +/// walked the whole tower instead: primitive and handle probes, a by-name +/// prototype read that allocates the key string, then a closure rebind and a +/// call through the thunk. That walk was ~6.3 k instructions of a ~13.7 k +/// `setTimeout` + `unref` + `clearTimeout` round (rate-limiter-flexible's +/// `MemoryStorage` shape), more than the timer bookkeeping itself. +/// +/// The fast path resolves the property itself, with no cache to go stale: +/// +/// * the receiver is a timer handle whose `[[Prototype]]` is still its +/// family's prototype, with no own string keys, not in dictionary mode, and +/// no accessor recorded for the name — so the lookup reaches the prototype; +/// * the prototype holds the name as an own DATA property (no accessor; a +/// dictionary-mode prototype is not scanned) whose value is a closure over +/// exactly this method's thunk. +/// +/// Any user change on either side — `t.unref = f`, `Object.setPrototypeOf`, +/// `Timeout.prototype.unref = f`, a getter via `defineProperty`, `delete` — +/// fails one of those checks and the call takes the tower as before. The +/// answer is the thunk's own, so the two paths cannot disagree. +pub(crate) unsafe fn try_timer_method_fast_dispatch(object: f64, name: &[u8]) -> Option { + let thunk = match name { + b"unref" => timer_proto_unref_thunk as *const u8, + b"ref" => timer_proto_ref_thunk as *const u8, + b"hasRef" => timer_proto_has_ref_thunk as *const u8, + b"refresh" => timer_proto_refresh_thunk as *const u8, + _ => return None, + }; + let (id, is_immediate) = timer_handle_parts(object)?; + let obj = (object.to_bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; + let slot = if is_immediate { + &IMMEDIATE_PROTOTYPE_PTR + } else { + &TIMEOUT_PROTOTYPE_PTR + }; + let proto = slot.load(Ordering::Acquire) as *const crate::object::ObjectHeader; + if proto.is_null() { + return None; + } + // `timer_handle_parts` proved `meta` non-null. + if (*(*obj).meta).prototype != crate::value::js_nanbox_pointer(proto as i64).to_bits() { + return None; + } + let name_str = std::str::from_utf8_unchecked(name); + if crate::object::dictionary::is_dictionary(obj) + || crate::object::shapes::object_shape_descriptor(obj)?.logical_key_count != 0 + || crate::object::descriptor_state::may_have_descriptor_entry(obj as usize, name_str, true) + { + return None; + } + if crate::object::dictionary::is_dictionary(proto) + || crate::object::descriptor_state::may_have_descriptor_entry( + proto as usize, + name_str, + true, + ) + { + return None; + } + let descriptor = crate::object::shapes::object_shape_descriptor(proto)?; + let keys = descriptor.keys as usize as *const crate::array::ArrayHeader; + if keys.is_null() || !crate::value::addr_class::is_above_handle_band(keys as usize) { + return None; + } + let (slots, slot_len) = crate::object::keys_array_dense_slots_resolved(keys); + let key_count = (descriptor.logical_key_count as usize).min(slot_len); + let index = (0..key_count).find(|&i| { + let key = crate::JSValue::from_bits((*slots.add(i)).to_bits()); + crate::string::js_string_key_matches_bytes(key, name) + })?; + let value = crate::object::js_object_get_field(proto, index as u32); + if !value.is_pointer() { + return None; + } + let closure = value.as_pointer::(); + if !crate::closure::is_closure_ptr(closure as usize) + || crate::closure::get_valid_func_ptr(closure) != thunk + { + return None; + } + // The thunks' bodies, with the receiver already in hand. + Some(match name { + b"unref" => { + js_timer_unref(id); + object + } + b"ref" => { + js_timer_ref(id); + object + } + b"hasRef" => f64::from_bits(crate::value::JSValue::bool(js_timer_has_ref(id) != 0).bits()), + _ => { + js_timer_refresh(id); + object + } + }) +} + fn clear_every_kind(id: i64) { clearTimeout(id); clearInterval(id); diff --git a/crates/perry-runtime/src/timer/ref_states.rs b/crates/perry-runtime/src/timer/ref_states.rs index fc885b34f9..5dd17a1ee3 100644 --- a/crates/perry-runtime/src/timer/ref_states.rs +++ b/crates/perry-runtime/src/timer/ref_states.rs @@ -31,7 +31,10 @@ struct TimerHandleState { /// [`ScheduledTimerId`]. The map is bounded by live timers + `cap`. #[derive(Default)] pub(super) struct TimerRefStates { - states: HashMap, + /// #10522: aHash — the key is our own monotonic timer id (no untrusted + /// input), and this map is touched on every schedule, `ref`/`unref` and + /// retire, where SipHash's rounds were a measurable share of timer churn. + states: HashMap, /// Retired ids, oldest first — the only eviction candidates. retired: VecDeque, } diff --git a/crates/perry-runtime/src/timer/tests_inline.rs b/crates/perry-runtime/src/timer/tests_inline.rs index 89f5d02894..1c2326d553 100644 --- a/crates/perry-runtime/src/timer/tests_inline.rs +++ b/crates/perry-runtime/src/timer/tests_inline.rs @@ -524,3 +524,153 @@ mod first_timer_cost_tests { .expect("the probe thread must not panic"); } } + +/// #10522: `t.unref()` & co. skip the dispatch tower only while the call +/// provably resolves to the family's own native method. Each override a +/// program can make must send the call back to the tower, or the fast path +/// would run the native method where JS says something else runs. +#[cfg(test)] +mod method_fast_path_tests { + use super::*; + use crate::timer::try_timer_method_fast_dispatch as fast; + + fn key(name: &str) -> *const crate::StringHeader { + crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32) + } + + /// Timer prototypes are per-thread singletons, and a test that mutates one + /// must not leak that into later tests on a reused thread. + fn on_fresh_thread(body: impl FnOnce() + Send + 'static) { + let _serial = crate::gc::global_side_table_test_lock(); + std::thread::spawn(move || { + crate::gc::ensure_gc_initialized(); + body(); + }) + .join() + .expect("the probe thread must not panic"); + } + + fn new_timeout() -> (f64, i64) { + let value = crate::value::js_nanbox_pointer(js_set_timeout_callback(0, 50_000.0)); + let (id, _) = crate::timer::timer_handle_parts(value).expect("branded handle"); + (value, id) + } + + fn timeout_proto() -> *mut crate::object::ObjectHeader { + handle_object::TIMEOUT_PROTOTYPE_PTR.load(Ordering::Acquire) as *mut _ + } + + #[test] + fn a_pristine_handle_takes_the_fast_path_with_the_thunks_answers() { + on_fresh_thread(|| unsafe { + let (t, id) = new_timeout(); + assert_eq!(fast(t, b"unref").map(f64::to_bits), Some(t.to_bits())); + assert_eq!(js_timer_has_ref(id), 0, "unref() did not take"); + let has_ref = fast(t, b"hasRef").expect("hasRef on the fast path"); + assert_eq!(has_ref.to_bits(), crate::value::JSValue::bool(false).bits()); + assert_eq!(fast(t, b"ref").map(f64::to_bits), Some(t.to_bits())); + assert_eq!(js_timer_has_ref(id), 1, "ref() did not take"); + assert_eq!(fast(t, b"refresh").map(f64::to_bits), Some(t.to_bits())); + // Not a fast-path name: the tower answers it. + assert!(fast(t, b"close").is_none()); + // An Immediate has no `refresh` in node; the tower throws for it. + let imm = crate::value::js_nanbox_pointer(js_set_immediate_callback(0)); + assert!(fast(imm, b"refresh").is_none()); + assert!(fast(imm, b"unref").is_some()); + // And the generic entry point reaches it: `t.unref()` as emitted. + let result = crate::object::js_native_call_method( + t, + b"unref".as_ptr() as *const i8, + 5, + std::ptr::null(), + 0, + ); + assert_eq!(result.to_bits(), t.to_bits()); + assert_eq!(js_timer_has_ref(id), 0); + clearTimeout(id); + }); + } + + #[test] + fn an_own_property_on_the_handle_is_not_bypassed() { + on_fresh_thread(|| unsafe { + let (t, id) = new_timeout(); + let obj = + (t.to_bits() & crate::value::POINTER_MASK) as *mut crate::object::ObjectHeader; + crate::object::js_object_set_field_by_name(obj, key("unref"), 1.0); + assert!( + fast(t, b"unref").is_none(), + "an own `unref` shadows the prototype" + ); + // Any own key at all leaves the pristine shape; stay conservative. + let (t2, id2) = new_timeout(); + let obj2 = + (t2.to_bits() & crate::value::POINTER_MASK) as *mut crate::object::ObjectHeader; + crate::object::js_object_set_field_by_name(obj2, key("tag"), 1.0); + assert!(fast(t2, b"unref").is_none()); + clearTimeout(id); + clearTimeout(id2); + }); + } + + #[test] + fn a_replaced_prototype_method_is_not_bypassed() { + on_fresh_thread(|| unsafe { + let (t, id) = new_timeout(); + let proto = timeout_proto(); + let original = crate::object::js_object_get_field_by_name(proto, key("unref")); + let ref_method = crate::object::js_object_get_field_by_name(proto, key("ref")); + // `Timeout.prototype.unref = Timeout.prototype.ref`: a closure, and + // a native one, but not THIS method's thunk. + crate::object::js_object_set_field_by_name( + proto, + key("unref"), + f64::from_bits(ref_method.bits()), + ); + assert!(fast(t, b"unref").is_none()); + crate::object::js_object_set_field_by_name( + proto, + key("unref"), + f64::from_bits(original.bits()), + ); + assert!( + fast(t, b"unref").is_some(), + "restoring the method restores the path" + ); + clearTimeout(id); + }); + } + + #[test] + fn a_prototype_getter_is_not_bypassed() { + on_fresh_thread(|| unsafe { + let (t, id) = new_timeout(); + let proto = timeout_proto(); + let getter = crate::object::js_object_get_field_by_name(proto, key("ref")); + crate::object::js_object_define_getter( + crate::value::js_nanbox_pointer(proto as i64), + crate::value::js_nanbox_string(key("unref") as i64), + f64::from_bits(getter.bits()), + ); + assert!( + fast(t, b"unref").is_none(), + "an accessor must run, not the thunk" + ); + clearTimeout(id); + }); + } + + #[test] + fn a_replaced_handle_prototype_is_not_bypassed() { + on_fresh_thread(|| unsafe { + let (t, id) = new_timeout(); + let other = crate::object::js_object_alloc(0, 0); + crate::object::prototype_chain::object_set_user_prototype( + (t.to_bits() & crate::value::POINTER_MASK) as usize, + crate::value::js_nanbox_pointer(other as i64).to_bits(), + ); + assert!(fast(t, b"unref").is_none()); + clearTimeout(id); + }); + } +}