diff --git a/changelog.d/8983-feedback-gate-and-shape-field.md b/changelog.d/8983-feedback-gate-and-shape-field.md new file mode 100644 index 0000000000..66a4acaecb --- /dev/null +++ b/changelog.d/8983-feedback-gate-and-shape-field.md @@ -0,0 +1,38 @@ +Two dead-work removals on the property read path. The computed-key read loop +now **matches node** (23 ms vs 23 ms on the same host), and the pure property +read drops 21 → 17 ms. + +**1. Typed-feedback observation is skipped when recording is off.** Recording +is off by default, and `guard_observe` and `record_fallback_call` both +early-return in that mode — but the property wrappers built the whole +`Observation` first, hashing the key and resolving the receiver's shape, purely +to hand it to functions that discard it. `js_typed_feedback_object_get_field_by_name_f64` +was 10% of an isolated property-read loop, nearly all of it that. The array +index wrappers have carried #5094's gate for exactly this reason, and #8951 +gave it to the fast store path; the property get/set wrappers never got it. + +Behaviour is unchanged in both modes: with recording off `guard_observe` +returns `contract_valid` unmodified and the fallback recorder is a no-op, so +the wrapper already reduced to precisely the underlying call it now makes +directly. + +**2. The slot bound stops copying a descriptor to read four bytes.** +`shape_descriptor_by_id` returns `ShapeDescriptor` **by value**, so +`object_live_slot_count` — consulted on essentially every property read and +write — lifted the whole ~48-byte record and kept only +`live_inline_slot_count`. It now reads that field through the table's record +using the same way-cache probe and the same epoch validation. +`shape_descriptor_by_id` was 10.1% of the same loop. + +Interleaved A/B, min-of-21, node on the same host in brackets: + +| loop | base | this PR | | +|---|---|---|---| +| pure property read | 21 ms (4) | **17 ms** | −19% | +| computed-key read | 27 ms (23) | **23 ms** | −15% — now at parity with node | +| combined overwrite | 46 ms (31) | **41 ms** | −11% | +| write only | 21 ms (23) | 21 ms | unchanged; already faster than node | + +Suite 2779 passed (including the 55 typed-feedback tests). Private-member +output is byte-identical to base; computed-key differential is byte-identical +to node. diff --git a/crates/perry-runtime/src/object/live_slots.rs b/crates/perry-runtime/src/object/live_slots.rs index 01186812cd..c37d688a1e 100644 --- a/crates/perry-runtime/src/object/live_slots.rs +++ b/crates/perry-runtime/src/object/live_slots.rs @@ -50,9 +50,11 @@ pub extern "C" fn perry_object_header_abi_revision() -> u32 { /// configuration. #[inline] pub unsafe fn object_live_slot_count(obj: *const ObjectHeader) -> u32 { - shapes::object_shape_descriptor(obj) - .map(|descriptor| descriptor.live_inline_slot_count) - .unwrap_or(0) + // Reads the one field through the table's record instead of lifting the + // whole ~48-byte descriptor to discard all but four bytes of it. This is + // the bound consulted on essentially every property read and write, and + // `shape_descriptor_by_id` was 10.1% of an isolated property-read loop. + shapes::shape_live_inline_slot_count_by_id(shapes::object_shape_stamp(obj)).unwrap_or(0) } /// C-ABI accessor for [`object_live_slot_count`], for out-of-runtime consumers diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 2d85bd7ecb..087ef78f16 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -569,6 +569,44 @@ pub(crate) fn shape_id_for_keys_ensure(keys: *const ArrayHeader, key_count: u32) publish_shape_result(shape_descriptor_ensure(keys, key_count, key_count)) } +/// One FIELD of a shape's descriptor, without lifting the whole record. +/// +/// [`shape_descriptor_by_id`] returns `ShapeDescriptor` **by value**, so every +/// caller that wants a single `u32` still copies the entire ~48-byte record +/// out of the table. That is most of them: `object_live_slot_count` — the slot +/// bound consulted on essentially every property read and write — throws away +/// all of it but `live_inline_slot_count`. +/// +/// This shares the way-cache probe with `shape_descriptor_by_id` and reads the +/// field through the record pointer instead. Same lookup, same validation, +/// four bytes instead of forty-eight. +#[inline] +fn shape_descriptor_field_by_id( + shape_id: u32, + read: impl Fn(&ShapeDescriptor) -> T, +) -> Option { + if !is_shape_id(shape_id) { + return None; + } + let table = &crate::state::state().shapes; + let epoch = table.lookup_epoch.get(); + let way = &table.lookup_ways[(shape_id as usize) & (SHAPE_LOOKUP_WAYS - 1)]; + let (cached_id, record, cached_epoch) = way.get(); + if cached_id == shape_id && cached_epoch == epoch && record != 0 { + // SAFETY: identical to `shape_descriptor_by_id`'s hit arm — the way is + // only filled from a live `Box` and the epoch is + // bumped whenever a record's address can change under an id still in + // use, so a matching epoch means this address is the table's record. + return Some(read(unsafe { &*(record as *const ShapeDescriptor) })); + } + shape_descriptor_by_id(shape_id).map(|d| read(&d)) +} + +/// The live inline-slot bound for `shape_id`, without copying its descriptor. +pub(crate) fn shape_live_inline_slot_count_by_id(shape_id: u32) -> Option { + shape_descriptor_field_by_id(shape_id, |d| d.live_inline_slot_count) +} + pub(crate) fn shape_descriptor_by_id(shape_id: u32) -> Option { if !is_shape_id(shape_id) { return None; diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index 735cd3a33b..0a7c123787 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -954,6 +954,17 @@ pub extern "C" fn js_typed_feedback_object_get_field_by_name_f64( obj: *const ObjectHeader, key: *const crate::StringHeader, ) -> f64 { + // #5094's gate, which the property wrappers never got. Typed-feedback + // recording is OFF by default, and `guard_observe` / `record_fallback_call` + // both early-return in that mode — but the caller has already built the + // whole `Observation` to hand them, hashing the key and resolving the + // receiver's shape. On an isolated property-read loop that dead work was + // 10% of self time. Take the underlying op directly, exactly as the array + // index wrappers already do. + if !typed_feedback_enabled() { + return crate::object::js_object_get_field_by_name_f64(obj, key); + } + let object_addr = normalize_raw_object_addr(obj as u64); let (shape_addr, class_id, gc_type) = object_shape(object_addr); let observation = Observation { @@ -985,6 +996,17 @@ pub extern "C" fn js_typed_feedback_object_set_field_by_name( key: *const crate::StringHeader, value: f64, ) { + // #5094's gate, which the property wrappers never got. Typed-feedback + // recording is OFF by default, and `guard_observe` / `record_fallback_call` + // both early-return in that mode — but the caller has already built the + // whole `Observation` to hand them, hashing the key and resolving the + // receiver's shape. On an isolated property-read loop that dead work was + // 10% of self time. Take the underlying op directly, exactly as the array + // index wrappers already do. + if !typed_feedback_enabled() { + crate::object::js_object_set_field_by_name(obj, key, value); + return; + } let object_addr = normalize_raw_object_addr(obj as u64); let (shape_addr, class_id, gc_type) = object_shape(object_addr); let observation = Observation { @@ -2581,6 +2603,17 @@ pub extern "C" fn js_typed_feedback_object_set_index_polymorphic( idx: f64, value: f64, ) { + // #5094's gate, which the property wrappers never got. Typed-feedback + // recording is OFF by default, and `guard_observe` / `record_fallback_call` + // both early-return in that mode — but the caller has already built the + // whole `Observation` to hand them, hashing the key and resolving the + // receiver's shape. On an isolated property-read loop that dead work was + // 10% of self time. Take the underlying op directly, exactly as the array + // index wrappers already do. + if !typed_feedback_enabled() { + crate::object::js_object_set_index_polymorphic(obj_handle, idx, value); + return; + } let index = finite_nonnegative_u32_index(idx).unwrap_or(u32::MAX); observe_array(site_id, obj_handle as *const ArrayHeader, index); record_guard_fail(site_id);