Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/perry-runtime/src/object/delete_rest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ pub extern "C" fn js_object_delete_field(
if obj.is_null() || key.is_null() {
return 1;
}
// A delete can rewrite key→slot mappings in place (same keys_array
// address), so cached (keys_array, key)→index plans must be flushed
// (`object::prop_plan` read-plan cache).
super::prop_plan::prop_plan_epoch_bump();
// A Proxy is a small registered id in the proxy id band, not a heap
// ObjectHeader. Dereferencing it below (GC header / keys_array reads) would
// segfault. Route `delete proxy.k` / `delete proxy[k]` through the proxy
Expand Down
107 changes: 107 additions & 0 deletions crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,113 @@ pub extern "C" fn js_object_get_field_by_name(
}
}
}
// FAST LANE (store-plan-cache follow-up): resolve an OWN data field on a
// provably-plain arena class instance with no rooting scope, no
// exotic-registry probes, and no key hashing. Every gate proves a property
// the skipped slow-path checks would have tested:
// - band/tag checks: not a proxy (above) / handle / stream encoding;
// - `classify_heap_generation != Unknown`: the address is inside a
// registered arena page, so its GcHeader is real — and no malloc-backed
// exotic (BufferHeader / TypedArrayHeader / DateCell / RegExpHeader /
// Temporal cell, all mi- or gc-malloc'd) can classify as arena;
// - `GC_TYPE_OBJECT`: not a closure / array / error / Map / Set;
// - `class_id != 0` (and not the native-module id): not an arguments
// object (allocated with class 0), URL-shape object, builtin prototype
// host, or plain literal — those keep their existing paths;
// - `OBJ_FLAG_HAS_DESCRIPTORS` clear: no own accessor can shadow the
// slot (an own data property shadows inherited accessors per [[Get]]);
// `OBJ_FLAG_TYPED_ARRAY_PROTO` clear: not the per-kind TypedArray
// prototype host (its reflection accessors have empty backing fields).
// The (keys_array, interned key) → index mapping comes from the
// epoch-guarded read-plan cache (flushed on GC, descriptor / prototype /
// vtable mutations, and property deletes); a lane-local bounded scan
// populates it. An absent own key falls through — prototype and getter
// resolution stay on the existing path.
unsafe {
let bits = obj as u64;
let top16 = bits >> 48;
let raw = if top16 == 0x7FFD {
(bits & 0x0000_FFFF_FFFF_FFFF) as usize
} else if top16 == 0 {
bits as usize
} else {
0
};
if raw >= crate::gc::GC_HEADER_SIZE + 0x1000
&& !crate::value::addr_class::is_small_handle(raw)
&& !crate::value::addr_class::is_stream_id_band(raw)
&& crate::value::addr_class::is_above_handle_band(key as usize)
{
let key_gc =
(key as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
if (*key_gc).gc_flags & crate::gc::GC_FLAG_INTERNED != 0
&& crate::arena::classify_heap_generation(raw)
!= crate::arena::HeapGeneration::Unknown
{
let gc_hdr =
(raw as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
const LANE_BLOCKING: u16 =
crate::gc::OBJ_FLAG_HAS_DESCRIPTORS | crate::gc::OBJ_FLAG_TYPED_ARRAY_PROTO;
if (*gc_hdr).obj_type == crate::gc::GC_TYPE_OBJECT
&& (*gc_hdr)._reserved & LANE_BLOCKING == 0
{
let o = raw as *const ObjectHeader;
let class_id = (*o).class_id;
if class_id != 0
&& class_id != super::super::native_module::NATIVE_MODULE_CLASS_ID
{
let keys = (*o).keys_array;
if !keys.is_null()
&& ((keys as u64) >> 48) == 0
&& crate::value::addr_class::is_above_handle_band(keys as usize)
{
let alloc_limit = std::cmp::max((*o).field_count, 8) as usize;
if let Some(idx) = super::super::prop_plan::read_plan_lookup(
keys as usize,
key as usize,
) {
return if (idx as usize) < alloc_limit {
super::accessors::js_object_get_field(o, idx)
} else {
match super::super::overflow_get(raw, idx as usize) {
Some(b) => JSValue::from_bits(b),
None => JSValue::undefined(),
}
};
}
let keys_gc = (keys as *const u8).sub(crate::gc::GC_HEADER_SIZE)
as *const crate::gc::GcHeader;
if (*keys_gc).obj_type == crate::gc::GC_TYPE_ARRAY {
let key_count =
crate::array::keys_array_len_capped_to_capacity(keys);
if key_count <= 4096 {
for i in 0..key_count {
let kv = crate::array::js_array_get(keys, i as u32);
if crate::string::js_string_key_matches(kv, key) {
super::super::prop_plan::read_plan_record(
keys as usize,
key as usize,
i as u32,
);
return if i < alloc_limit {
super::accessors::js_object_get_field(o, i as u32)
} else {
match super::super::overflow_get(raw, i) {
Some(b) => JSValue::from_bits(b),
None => JSValue::undefined(),
}
};
}
}
}
}
}
}
}
}
}
}

// A receiver that LOOKS like a bare heap pointer (top 16 bits clear) but does
// not land in the platform heap range is a MIS-decoded primitive, not an
// object. The common case is a `number` whose raw f64 bits alias a sub-heap
Expand Down
161 changes: 161 additions & 0 deletions crates/perry-runtime/src/object/field_set_by_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,167 @@ pub extern "C" fn js_object_set_field_by_name(
}
}
}
// FAST LANE (mirror of the read lane in `js_object_get_field_by_name`,
// same gate rationale — see that comment): a provably-plain arena class
// instance whose store plan says "no interceptor for this (class, key)"
// takes the shape-transition cache directly, with no rooting scope and no
// exotic-registry probes. Additional store-only gates: the frozen family
// and the chain-divergence flags must be clear (same set the in-body fast
// path vets), and the plan hit itself certifies no vtable setter /
// prototype interceptor / URL / native-module route. Nothing on this path
// allocates from the arena, so raw pointers stay valid without handles.
unsafe {
let bits = obj as u64;
let top16 = bits >> 48;
let raw = if top16 == 0x7FFD {
(bits & 0x0000_FFFF_FFFF_FFFF) as usize
} else if top16 == 0 {
bits as usize
} else {
0
};
if raw >= crate::gc::GC_HEADER_SIZE + 0x1000
&& !crate::value::addr_class::is_small_handle(raw)
&& !crate::value::addr_class::is_stream_id_band(raw)
&& crate::value::addr_class::is_above_handle_band(key as usize)
{
let key_gc =
(key as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
if (*key_gc).gc_flags & crate::gc::GC_FLAG_INTERNED != 0
&& crate::arena::classify_heap_generation(raw)
!= crate::arena::HeapGeneration::Unknown
{
let gc_hdr =
(raw as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
const LANE_BLOCKING: u16 = crate::gc::OBJ_FLAG_FROZEN
| crate::gc::OBJ_FLAG_SEALED
| crate::gc::OBJ_FLAG_NO_EXTEND
| crate::gc::OBJ_FLAG_HAS_DESCRIPTORS
| crate::gc::OBJ_FLAG_PROTO_OVERRIDE
| crate::gc::OBJ_FLAG_NULL_PROTO
| crate::gc::OBJ_FLAG_TYPED_ARRAY_PROTO;
if (*gc_hdr).obj_type == crate::gc::GC_TYPE_OBJECT
&& (*gc_hdr)._reserved & LANE_BLOCKING == 0
{
let o = raw as *mut ObjectHeader;
let class_id = (*o).class_id;
if class_id != 0
&& class_id != NATIVE_MODULE_CLASS_ID
&& super::prop_plan::store_plan_check(class_id, key as usize)
{
let keys = (*o).keys_array;
let keys_ok = keys.is_null()
|| (((keys as u64) >> 48) == 0
&& crate::value::addr_class::is_above_handle_band(keys as usize));
if keys_ok {
// Overwrite of an EXISTING own key: the keys array
// doesn't change, so the shape-transition cache
// (which stores append EDGES) can never serve it —
// the (keys, key) → index read-plan cache can.
// Miss → one bounded scan populates it; absent own
// key falls to the append-edge lookup below.
if !keys.is_null() {
let mut own_idx =
super::prop_plan::read_plan_lookup(keys as usize, key as usize);
if own_idx.is_none() {
let keys_gc = (keys as *const u8).sub(crate::gc::GC_HEADER_SIZE)
as *const crate::gc::GcHeader;
if (*keys_gc).obj_type == crate::gc::GC_TYPE_ARRAY {
let key_count =
crate::array::keys_array_len_capped_to_capacity(keys);
if key_count <= 4096 {
for i in 0..key_count {
let kv = crate::array::js_array_get(keys, i as u32);
if crate::string::js_string_key_matches(kv, key) {
super::prop_plan::read_plan_record(
keys as usize,
key as usize,
i as u32,
);
own_idx = Some(i as u32);
break;
}
}
}
}
}
if let Some(idx) = own_idx {
let vbits = value.to_bits();
let vbits = if (vbits >> 48) == 0x7FFD
&& (vbits & 0x0000_FFFF_FFFF_FFFF) == 0
{
crate::value::TAG_UNDEFINED
} else {
vbits
};
// Layout safety (#6495 family): the slot's
// pointer-ness may change — degrade the
// layout to full-visit before the store.
super::mark_object_dynamic_shape_unknown(o);
let alloc_limit = std::cmp::max((*o).field_count, 8) as usize;
if (idx as usize) < alloc_limit {
let fields_ptr = (o as *mut u8)
.add(std::mem::size_of::<ObjectHeader>())
as *mut JSValue;
let slot = fields_ptr.add(idx as usize);
crate::gc::runtime_store_jsvalue_slot(
o as usize,
slot as usize,
idx as usize,
vbits,
);
if idx >= (*o).field_count {
(*o).field_count = idx + 1;
}
} else {
overflow_set(o as usize, idx as usize, vbits);
}
return;
}
}
if let Some((next_keys, slot_idx)) =
transition_cache_lookup(keys as usize, key)
{
// Same store semantics as the in-body fast
// path: strip a raw-null POINTER_TAG value,
// transition the keys array, note the dynamic
// shape, then write inline or overflow.
let vbits = value.to_bits();
let vbits = if (vbits >> 48) == 0x7FFD
&& (vbits & 0x0000_FFFF_FFFF_FFFF) == 0
{
crate::value::TAG_UNDEFINED
} else {
vbits
};
set_object_keys_array(o, next_keys as *mut ArrayHeader);
super::mark_object_dynamic_shape_unknown(o);
let alloc_limit = std::cmp::max((*o).field_count, 8) as usize;
if (slot_idx as usize) < alloc_limit {
let fields_ptr = (o as *mut u8)
.add(std::mem::size_of::<ObjectHeader>())
as *mut JSValue;
let slot = fields_ptr.add(slot_idx as usize);
crate::gc::runtime_store_jsvalue_slot(
o as usize,
slot as usize,
slot_idx as usize,
vbits,
);
if slot_idx >= (*o).field_count {
(*o).field_count = slot_idx + 1;
}
} else {
overflow_set(o as usize, slot_idx as usize, vbits);
}
return;
}
}
}
}
}
}
}
// A Buffer is an ordinary object in Node (a Uint8Array), so `buf.foo = v`
// stores an own property — and an own key SHADOWS the same-named prototype
// method. Perry keeps buffers outside the object model (raw BufferHeader,
Expand Down
19 changes: 18 additions & 1 deletion crates/perry-runtime/src/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,10 +762,27 @@ pub extern "C" fn perry_key_content_hash(key: *const crate::StringHeader) -> u64
}

#[inline(always)]
fn key_content_hash(key: *const crate::StringHeader) -> u64 {
pub(crate) fn key_content_hash(key: *const crate::StringHeader) -> u64 {
key_content_hash_impl(key)
}

/// Resolve `key` to its canonical interned `StringHeader` pointer (as a
/// `usize`), the identity the `prop_plan` store/read caches key on. Returns 0
/// for a null / handle-band key. Mirrors the inline interning both field
/// stores do, so a plan recorded on one store path is found by another.
#[inline]
pub(crate) unsafe fn interned_key_ptr(key: *const crate::StringHeader) -> usize {
if key.is_null() || !crate::value::addr_class::is_above_handle_band(key as usize) {
return 0;
}
let gc_hdr = (key as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
if (*gc_hdr).gc_flags & crate::gc::GC_FLAG_INTERNED != 0 {
key as usize
} else {
crate::string::js_string_intern(key, key_content_hash(key)) as usize
}
Comment on lines +774 to +783

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not cache keys the interner declines to intern.

js_string_intern returns the original, unmarked pointer for over-limit strings. Treating that pointer as canonical lets a store-plan verdict follow mutable non-interned contents and potentially skip a setter or non-writable-property check. Return 0 unless the returned pointer actually has GC_FLAG_INTERNED.

Proposed fix
-        crate::string::js_string_intern(key, key_content_hash(key)) as usize
+        let interned = crate::string::js_string_intern(key, key_content_hash(key));
+        if interned.is_null() {
+            return 0;
+        }
+        let gc_hdr = (interned as *const u8).sub(crate::gc::GC_HEADER_SIZE)
+            as *const crate::gc::GcHeader;
+        if (*gc_hdr).gc_flags & crate::gc::GC_FLAG_INTERNED != 0 {
+            interned as usize
+        } else {
+            0
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub(crate) unsafe fn interned_key_ptr(key: *const crate::StringHeader) -> usize {
if key.is_null() || !crate::value::addr_class::is_above_handle_band(key as usize) {
return 0;
}
let gc_hdr = (key as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
if (*gc_hdr).gc_flags & crate::gc::GC_FLAG_INTERNED != 0 {
key as usize
} else {
crate::string::js_string_intern(key, key_content_hash(key)) as usize
}
let interned = crate::string::js_string_intern(key, key_content_hash(key));
if interned.is_null() {
return 0;
}
let gc_hdr = (interned as *const u8).sub(crate::gc::GC_HEADER_SIZE)
as *const crate::gc::GcHeader;
if (*gc_hdr).gc_flags & crate::gc::GC_FLAG_INTERNED != 0 {
interned as usize
} else {
0
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/object/mod.rs` around lines 774 - 783, Update
interned_key_ptr so the result of js_string_intern is validated before returning
it: inspect the returned pointer’s GcHeader and return its address only when
GC_FLAG_INTERNED is set; otherwise return 0. Preserve the existing fast path for
keys already marked interned and the current null/address-band checks.

}

#[inline(always)]
fn key_content_hash_impl(key: *const crate::StringHeader) -> u64 {
unsafe {
Expand Down
Loading
Loading