Skip to content

Commit b3e88f1

Browse files
proggeramlugRalph Küpper
andauthored
land #8794: reuse exact ShapeId transitions (#8806)
* perf(runtime): reuse exact ShapeId transitions * docs(changelog): note ShapeId transition reuse * fix(runtime): guard cached shape transition invariants * chore: landing fixes for #8794 (object/mod.rs split, raw-handle ceilings) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 066f9d5 commit b3e88f1

14 files changed

Lines changed: 1010 additions & 430 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Dynamic property writes now reuse exact structural transitions by stable predecessor ShapeId and interned key, then stamp the learned successor ShapeId directly after fail-closed content and identity guards. Anonymous-object construction avoids repeated descriptor hashing and publication, shared shape-key arrays retain an all-pointer layout, reverse indices update incrementally, and full GC drops transition roots after their weak key or predecessor descriptor dies.

crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,28 +1499,28 @@ fn test_live_boxed_primitive_payload_owner_survives_full_gc() {
14991499
js_shadow_slot_set(0, 0);
15001500
}
15011501

1502-
/// #8192. A transition-cache entry carries TWO metadata-only keys — the
1503-
/// pre-transition `keys_array` and the interned property-name string — while
1504-
/// only `next_keys` is a strong root. Either weak half dying is enough to make
1505-
/// the entry's rekey read recycled bytes, so either is enough to drop it.
1502+
/// #8192. A transition-cache entry carries one metadata-only key — the
1503+
/// interned property-name string — while `next_keys` is a strong root and both
1504+
/// ShapeIds are stable non-pointer metadata. A dead weak key drops the entry.
15061505
#[test]
1507-
fn test_dead_transition_cache_prev_keys_pruned_on_full_gc() {
1506+
fn test_dead_transition_cache_key_pruned_on_full_gc() {
15081507
let _guard = CopyingNurseryTestGuard::new(1);
15091508
gc_register_mutable_root_scanner(crate::object::scan_transition_cache_roots_mut);
15101509

15111510
// `next_keys` is a strong root of the entry and must stay live, or the test
15121511
// would be measuring the wrong pointer. It goes in OLD-GEN on purpose:
15131512
// arena block reset is all-or-nothing, so a reachable neighbour in
1514-
// `dead_prev`'s own nursery block would force-MARK it (#7975,
1513+
// `dead_key`'s own nursery block would force-MARK it (#7975,
15151514
// `mark_block_persisting_arena_objects`) and the prune would correctly
15161515
// decline — the test would then blame the prune for something it got right.
15171516
let next_keys = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_ARRAY) as usize;
15181517
js_shadow_slot_set(0, ptr_bits(next_keys));
15191518
crate::arena::arena_reset_all_blocks_to_zero();
1520-
let dead_prev = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_ARRAY) as usize;
1519+
let dead_key = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_STRING) as usize;
1520+
let prev_shape_id = crate::object::shapes::shape_id_for_keys_ensure(std::ptr::null(), 0);
15211521

15221522
let occupancy_before = crate::object::test_transition_cache_occupancy();
1523-
crate::object::test_seed_transition_cache_entry(dead_prev, 0, next_keys);
1523+
crate::object::test_seed_transition_cache_entry(prev_shape_id, dead_key, next_keys);
15241524
assert_eq!(
15251525
crate::object::test_transition_cache_occupancy(),
15261526
occupancy_before + 1,
@@ -1532,32 +1532,65 @@ fn test_dead_transition_cache_prev_keys_pruned_on_full_gc() {
15321532
assert_eq!(
15331533
crate::object::test_transition_cache_occupancy(),
15341534
occupancy_before,
1535-
"a transition entry whose prev_keys array died must be dropped, not \
1535+
"a transition entry whose interned key died must be dropped, not \
15361536
rekeyed out of the bytes that replaced it"
15371537
);
15381538
js_shadow_slot_set(0, 0);
15391539
}
15401540

1541+
#[test]
1542+
fn test_dead_transition_cache_predecessor_shape_pruned_on_full_gc() {
1543+
let _guard = CopyingNurseryTestGuard::new(1);
1544+
gc_register_mutable_root_scanner(crate::object::scan_transition_cache_roots_mut);
1545+
1546+
let next_keys = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_ARRAY) as usize;
1547+
js_shadow_slot_set(0, ptr_bits(next_keys));
1548+
crate::arena::arena_reset_all_blocks_to_zero();
1549+
let dead_predecessor_keys = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_ARRAY) as usize;
1550+
let prev_shape_id = crate::object::shapes::shape_id_for_keys_ensure(
1551+
dead_predecessor_keys as *const crate::ArrayHeader,
1552+
0,
1553+
);
1554+
1555+
let occupancy_before = crate::object::test_transition_cache_occupancy();
1556+
crate::object::test_seed_transition_cache_entry(prev_shape_id, 0, next_keys);
1557+
assert_eq!(
1558+
crate::object::test_transition_cache_occupancy(),
1559+
occupancy_before + 1,
1560+
"test premise: the transition entry must be installed"
1561+
);
1562+
1563+
full_gc();
1564+
1565+
assert_eq!(
1566+
crate::object::test_transition_cache_occupancy(),
1567+
occupancy_before,
1568+
"a transition whose predecessor descriptor died must release its rooted target"
1569+
);
1570+
js_shadow_slot_set(0, 0);
1571+
}
1572+
15411573
#[test]
15421574
fn test_live_transition_cache_entry_survives_full_gc() {
1543-
// Two shadow slots: both halves of the entry have to be rooted.
1575+
// Two shadow slots: the strong target and weak lookup key both stay live.
15441576
let _guard = CopyingNurseryTestGuard::new(2);
15451577
gc_register_mutable_root_scanner(crate::object::scan_transition_cache_roots_mut);
15461578

15471579
let next_keys = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_ARRAY) as usize;
1548-
let live_prev = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_ARRAY) as usize;
1580+
let live_key = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_STRING) as usize;
1581+
let prev_shape_id = crate::object::shapes::shape_id_for_keys_ensure(std::ptr::null(), 0);
15491582
js_shadow_slot_set(0, ptr_bits(next_keys));
1550-
js_shadow_slot_set(1, ptr_bits(live_prev));
1583+
js_shadow_slot_set(1, ptr_bits(live_key));
15511584

15521585
let occupancy_before = crate::object::test_transition_cache_occupancy();
1553-
crate::object::test_seed_transition_cache_entry(live_prev, 0, next_keys);
1586+
crate::object::test_seed_transition_cache_entry(prev_shape_id, live_key, next_keys);
15541587

15551588
full_gc();
15561589

15571590
assert_eq!(
15581591
crate::object::test_transition_cache_occupancy(),
15591592
occupancy_before + 1,
1560-
"an entry whose prev_keys array is still ROOTED must survive"
1593+
"an entry whose interned key is still ROOTED must survive"
15611594
);
15621595
js_shadow_slot_set(0, 0);
15631596
js_shadow_slot_set(1, 0);

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ pub use attr_variants::{
2626
js_object_set_field_by_name_nonconfigurable, js_object_set_field_by_name_nonenum,
2727
};
2828
pub use fast_paths::js_object_set_field_by_name_transition_fast;
29-
pub(crate) use fast_paths::try_existing_own_data_overwrite;
29+
pub(crate) use fast_paths::{
30+
object_set_field_by_name_transition_only_fast, try_existing_own_data_overwrite,
31+
};
32+
#[cfg(test)]
33+
pub(crate) use fast_paths::{test_reset_transition_fast_hits, test_transition_fast_hits};
3034
pub(crate) use tail::set_field_by_name_object_tail;
3135
pub(crate) use write_helpers::nm_field_set_override;
3236
use write_helpers::string_key_eq;
@@ -162,13 +166,10 @@ pub extern "C" fn js_object_set_field_by_name(
162166
&& !super::prototype_chain::object_has_prototype_override(raw)
163167
&& super::prop_plan::store_plan_check(class_id, key as usize)
164168
{
165-
let keys = crate::object::object_keys_array(o);
166-
let keys_ok = keys.is_null()
167-
|| (((keys as u64) >> 48) == 0
168-
&& crate::value::addr_class::is_above_handle_band(keys as usize));
169-
if keys_ok {
170-
if let Some((next_keys, slot_idx)) =
171-
transition_cache_lookup(keys as usize, key)
169+
let prev_shape_id = super::shapes::object_shape_stamp(o);
170+
if prev_shape_id != 0 {
171+
if let Some((next_keys, slot_idx, target_shape_id)) =
172+
transition_cache_lookup(prev_shape_id, key)
172173
{
173174
// Same store semantics as the in-body fast
174175
// path: strip a raw-null POINTER_TAG value,
@@ -182,8 +183,14 @@ pub extern "C" fn js_object_set_field_by_name(
182183
} else {
183184
vbits
184185
};
185-
set_object_keys_array(o, next_keys as *mut ArrayHeader);
186-
super::mark_object_dynamic_shape_unknown(o);
186+
if !super::shapes::install_cached_object_shape_transition(
187+
o,
188+
prev_shape_id,
189+
target_shape_id,
190+
next_keys as *mut ArrayHeader,
191+
) {
192+
set_object_keys_array(o, next_keys as *mut ArrayHeader);
193+
}
187194
// #8113: one bound probe, reused.
188195
let live_slots = crate::object::object_live_slot_count(o);
189196
let alloc_limit = std::cmp::max(

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

Lines changed: 141 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,21 @@
55
66
use super::*;
77

8+
#[cfg(test)]
9+
thread_local! {
10+
static TEST_TRANSITION_FAST_HITS: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
11+
}
12+
13+
#[cfg(test)]
14+
pub(crate) fn test_reset_transition_fast_hits() {
15+
TEST_TRANSITION_FAST_HITS.with(|hits| hits.set(0));
16+
}
17+
18+
#[cfg(test)]
19+
pub(crate) fn test_transition_fast_hits() -> u64 {
20+
TEST_TRANSITION_FAST_HITS.with(std::cell::Cell::get)
21+
}
22+
823
/// Non-allocating-in-the-GC-heap overwrite for an existing own data field.
924
///
1025
/// This is the common assignment case for ordinary objects. It is deliberately
@@ -132,15 +147,40 @@ pub(crate) unsafe fn try_existing_own_data_overwrite(
132147
///
133148
/// This is intentionally narrower than `js_object_set_field_by_name`: it only
134149
/// handles plain object-shape transitions that have already been learned by
135-
/// the runtime transition cache. Accessors/descriptors, frozen/sealed objects,
136-
/// class/prototype receivers, closures, native handles, arrays, strings, and
137-
/// cache misses return 0 so callers preserve the full setter semantics by
150+
/// the runtime transition cache. In addition to class-id-zero objects, HIR's
151+
/// registered anonymous-shape classes qualify: they are the runtime backing
152+
/// for source-level object literals and have ordinary `Object.prototype`
153+
/// semantics. User class instances, accessors/descriptors, frozen/sealed
154+
/// objects, prototype overrides, closures, native handles, arrays, strings,
155+
/// and cache misses return 0 so callers preserve the full setter semantics by
138156
/// falling back to `js_object_set_field_by_name`.
139157
#[no_mangle]
140158
pub extern "C" fn js_object_set_field_by_name_transition_fast(
141159
obj: *mut ObjectHeader,
142160
key: *const crate::StringHeader,
143161
value: f64,
162+
) -> i32 {
163+
object_set_field_by_name_transition_fast_impl(obj, key, value, true)
164+
}
165+
166+
/// Transition-only form for callers that already own a complete semantic
167+
/// fallback. If `key` is an existing property, no append edge can exist for
168+
/// `(current_keys, key)`, so the lookup returns 0 and the caller performs the
169+
/// ordinary write. Skipping the up-front linear overwrite scan is important
170+
/// for repeated computed-key object construction.
171+
pub(crate) fn object_set_field_by_name_transition_only_fast(
172+
obj: *mut ObjectHeader,
173+
key: *const crate::StringHeader,
174+
value: f64,
175+
) -> i32 {
176+
object_set_field_by_name_transition_fast_impl(obj, key, value, false)
177+
}
178+
179+
fn object_set_field_by_name_transition_fast_impl(
180+
obj: *mut ObjectHeader,
181+
key: *const crate::StringHeader,
182+
value: f64,
183+
try_overwrite: bool,
144184
) -> i32 {
145185
if key.is_null() || (key as usize) < 0x10000 {
146186
return 0;
@@ -174,7 +214,7 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast(
174214
return 0;
175215
}
176216

177-
if unsafe { try_existing_own_data_overwrite(obj, key, value) } {
217+
if try_overwrite && unsafe { try_existing_own_data_overwrite(obj, key, value) } {
178218
return 1;
179219
}
180220

@@ -219,21 +259,26 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast(
219259
let key_gc =
220260
(key as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
221261

222-
// The append-transition half below is intentionally restricted to
223-
// class-id-zero plain objects. Existing own-data overwrites were
224-
// already handled by `try_existing_own_data_overwrite` before the
225-
// rooting scope.
226-
if (*obj).class_id != 0 {
262+
// Closed source-level object literals are represented as synthetic
263+
// `__AnonShape_*` classes so their static fields can use the same
264+
// ShapeId machinery as class instances. Semantically they are still
265+
// plain objects: codegen registers their class ids at module init and
266+
// prototype/constructor dispatch already treats them as having
267+
// ordinary Object semantics. Admit exactly that registered population
268+
// alongside genuinely class-id-zero objects; a real user class must
269+
// retain the full inherited-setter/prototype walk.
270+
let class_id = (*obj).class_id;
271+
if class_id != 0 && !crate::object::is_anon_shape_class_id(class_id) {
227272
return 0;
228273
}
229274

230275
// #6084 item 6: this used to be a `GLOBAL_DESCRIPTORS_IN_USE` check at
231276
// the top of the function — one `Object.freeze` anywhere in the process
232277
// (even on an unrelated object) permanently disabled this fast path for
233278
// every object. Vet the receiver's own flag (above) and its prototype
234-
// chain (here) instead. `class_id` is 0 at this point, so the only
235-
// inherited interceptor is `Object.prototype` (or a recorded
236-
// `setPrototypeOf` target).
279+
// chain (here) instead. Pass semantic class id zero for an anon shape:
280+
// its nonzero runtime id is an implementation detail, not a JS class
281+
// whose vtable/prototype chain can carry instance accessors.
237282
let key_f64 = f64::from_bits(JSValue::string_ptr(key as *mut _).bits());
238283
if super::plain_data_write_may_intercept(obj as usize, 0, key_f64) {
239284
return 0;
@@ -255,24 +300,36 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast(
255300
obj = obj_handle.get_raw_mut_ptr::<ObjectHeader>();
256301
let value = value_handle.get_nanbox_f64();
257302

258-
let keys = crate::object::object_keys_array(obj);
259-
let prev_keys = keys as usize;
260-
if !keys.is_null() {
261-
let keys_ptr = keys as usize;
262-
if (keys_ptr as u64) >> 48 != 0 || keys_ptr < 0x10000 {
263-
return 0;
264-
}
265-
}
266-
267-
let Some((next_keys, slot_idx)) = transition_cache_lookup(prev_keys, interned_key) else {
303+
let prev_shape_id = super::shapes::object_shape_stamp(obj);
304+
let Some((next_keys, slot_idx, target_shape_id)) =
305+
transition_cache_lookup(prev_shape_id, interned_key)
306+
else {
268307
return 0;
269308
};
270309
if next_keys == 0 {
271310
return 0;
272311
}
273312

274-
set_object_keys_array(obj, next_keys as *mut ArrayHeader);
275-
super::mark_object_dynamic_shape_unknown(obj);
313+
// `Object.prototype[<index>]` must reach the ordinary setter so it can
314+
// invalidate array hole/OOB guards through
315+
// `note_object_prototype_index_write`. A canonical index must start
316+
// with an ASCII digit, so named transitions avoid the prototype TLS
317+
// lookup entirely. Probe only after a cache hit; ordinary misses
318+
// already take the semantic fallback.
319+
let key_starts_with_digit =
320+
(*key).byte_len != 0 && (*crate::string::string_data(key)).is_ascii_digit();
321+
if key_starts_with_digit && crate::array::object_prototype_addr_matches(obj as usize) {
322+
return 0;
323+
}
324+
325+
if !super::shapes::install_cached_object_shape_transition(
326+
obj,
327+
prev_shape_id,
328+
target_shape_id,
329+
next_keys as *mut ArrayHeader,
330+
) {
331+
set_object_keys_array(obj, next_keys as *mut ArrayHeader);
332+
}
276333

277334
// #8113: one bound probe, reused.
278335
let live_slots = crate::object::object_live_slot_count(obj);
@@ -294,7 +351,67 @@ pub extern "C" fn js_object_set_field_by_name_transition_fast(
294351
} else {
295352
overflow_set(obj as usize, slot_usize, vbits);
296353
}
354+
355+
#[cfg(test)]
356+
TEST_TRANSITION_FAST_HITS.with(|hits| hits.set(hits.get() + 1));
297357
}
298358

299359
1
300360
}
361+
362+
#[cfg(test)]
363+
mod tests {
364+
use super::*;
365+
366+
#[test]
367+
fn transition_fast_rejects_object_prototype_even_with_a_cached_edge() {
368+
let _lock = crate::gc::global_side_table_test_lock();
369+
let scope = crate::gc::RuntimeHandleScope::new();
370+
let prototype = crate::array::object_prototype_addr() as *mut ObjectHeader;
371+
assert!(!prototype.is_null(), "test premise: Object.prototype");
372+
let prototype_handle = scope.root_raw_mut_ptr(prototype);
373+
374+
let raw_key = crate::string::js_string_from_bytes(b"879400001".as_ptr(), 9);
375+
let raw_key_handle = scope.root_string_ptr(raw_key);
376+
let raw_key = raw_key_handle.get_raw_const_ptr::<crate::StringHeader>();
377+
let key = crate::string::js_string_intern(raw_key, key_content_hash(raw_key));
378+
let key_handle = scope.root_string_ptr(key);
379+
380+
let prototype = prototype_handle.get_raw_mut_ptr::<ObjectHeader>();
381+
let predecessor = unsafe { super::super::shapes::object_shape_stamp(prototype) };
382+
assert!(
383+
super::super::shapes::is_shape_id(predecessor),
384+
"test premise: Object.prototype has a resolvable ShapeId"
385+
);
386+
let old_keys = unsafe { super::super::object_keys_array(prototype) };
387+
let next_keys = crate::array::js_array_clone(old_keys);
388+
let slot = crate::array::js_array_length(next_keys);
389+
let next_keys = crate::array::js_array_push(
390+
next_keys,
391+
crate::JSValue::string_ptr(key_handle.get_raw_mut_ptr()),
392+
);
393+
let next_keys_handle = scope.root_raw_mut_ptr(next_keys);
394+
let next_keys = next_keys_handle.get_raw_mut_ptr::<ArrayHeader>();
395+
let target = super::super::shapes::shape_descriptor_ensure(
396+
next_keys,
397+
slot + 1,
398+
unsafe { super::super::object_live_slot_count(prototype) }.max(slot + 1),
399+
)
400+
.expect("shape range unexpectedly exhausted");
401+
let key = key_handle.get_raw_const_ptr::<crate::StringHeader>();
402+
super::super::transition_cache_insert(predecessor, key, next_keys as usize, slot, target);
403+
assert!(
404+
super::super::transition_cache_lookup(predecessor, key).is_some(),
405+
"test premise: the synthetic transition must be cache-resident"
406+
);
407+
408+
test_reset_transition_fast_hits();
409+
assert_eq!(
410+
object_set_field_by_name_transition_only_fast(prototype, key, 42.0),
411+
0,
412+
"Object.prototype must use the setter that records indexed writes"
413+
);
414+
assert_eq!(test_transition_fast_hits(), 0);
415+
super::super::test_clear_transition_cache_root();
416+
}
417+
}

0 commit comments

Comments
 (0)