diff --git a/changelog.d/9017-fused-for-of-next.md b/changelog.d/9017-fused-for-of-next.md new file mode 100644 index 0000000000..ab1b46a102 --- /dev/null +++ b/changelog.d/9017-fused-for-of-next.md @@ -0,0 +1,31 @@ +The `for…of` desugar's per-element `IteratorNext` is now one runtime call. + +The generic desugar performed, per element, a dynamic `.next()` dispatch, a +separate validation call, an override probe that allocated a `"next"` key string +and ran a by-name prototype lookup, and — for builtin collection iterators — a +fresh `{value, done}` allocation. V8 escape-analyses all of that away; perry +executed it literally, and the two collection-iterator dispatchers were 8.6% of +an ECS archetype-migration row. + +`js_for_of_next(iter)` replaces the pair. A builtin Map/Set iterator advances in +place through the same dispatcher arm the manual path uses, override probe +included, and reuses a `{value, done}` object cached in the iterator's sixth +field. Every other receiver — array iterators, generators, user iterators — +takes a generic arm that is the two-call shape it replaces. Manual `.next()` +calls and both public dispatchers keep allocating fresh results, so a caller +that retains one still observes spec behaviour. + +The override probe no longer builds the prototype tower to decide whether `next` +was patched: the only route to the prototype object is `Object.getPrototypeOf`, +which materializes the tower, so a null tower proves no override. That removes a +key-string allocation from the manual path too. + +Recycling is sound because the cached object is only reachable from the +compiler's `for…of` desugar, whose result is a temporary the loop body cannot +name and whose `done`/`value` are read before the next advance — the desugar +binds the loop variable ahead of the body, so even two loops sharing one +iterator cannot clobber an unread value. + +One spec tightening: the sync desugar arm that previously skipped result +validation now routes through the fused entry, which validates on the generic +arm — matching the other sync driver. diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index ec1816ad40..71566af703 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1542,6 +1542,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function("js_with_implicit_read", DOUBLE, &[DOUBLE, DOUBLE]); // Iterator-protocol result validation (for-of lazy loop). module.declare_function("js_iterator_result_validate", DOUBLE, &[DOUBLE]); + module.declare_function("js_for_of_next", DOUBLE, &[DOUBLE]); module.declare_function("js_global_get_or_throw_unresolved", DOUBLE, &[DOUBLE]); // Ambient `require` for compiled external / compilePackages modules (#5373): // bind a bare `require` to a createRequire-backed closure instead of throwing diff --git a/crates/perry-hir/src/lower/stmt_loops.rs b/crates/perry-hir/src/lower/stmt_loops.rs index 34c1ec31cc..cb8460f179 100644 --- a/crates/perry-hir/src/lower/stmt_loops.rs +++ b/crates/perry-hir/src/lower/stmt_loops.rs @@ -348,18 +348,20 @@ fn iterator_result_validated(call: Expr) -> Expr { } } -/// `__iter.next()` (validated: a non-object result is a TypeError). +/// One fused IteratorNext (`js_for_of_next(__iter)`): builtin Map/Set +/// iterators advance in place; everything else runs the dynamic `.next()` +/// plus result validation inside the entry — the two-call shape this emitted. pub(crate) fn iterator_next_call(iter_id: LocalId) -> Expr { - iterator_result_validated(Expr::Call { - callee: Box::new(Expr::PropertyGet { - byte_offset: 0, - object: Box::new(Expr::LocalGet(iter_id)), - property: "next".to_string(), + Expr::Call { + callee: Box::new(Expr::ExternFuncRef { + name: "js_for_of_next".to_string(), + param_types: vec![Type::Any], + return_type: Type::Any, }), - args: vec![], + args: vec![Expr::LocalGet(iter_id)], type_args: vec![], byte_offset: 0, - }) + } } /// Iterator-driver loop with the ADVANCE AT THE TOP of the body: @@ -368,11 +370,9 @@ pub(crate) fn iterator_next_call(iter_id: LocalId) -> Expr { /// advance. The previous shape — `while (!__result.done) { ; /// __result = next() }` — put the advance at the body TAIL, so a `continue` /// skipped it and re-processed the SAME result forever (the footgun -/// `lazy_iter_for_stmt` documents; it can use `Stmt::For`'s update clause, -/// but the await-capable drivers here cannot carry an `await` there, so they -/// use this shape). Canonical spin: an SSE consumer's -/// `for await (...) { if (ev === "ping") continue; ... }` hung a large -/// esbuild-bundled CLI app on the first real server ping. +/// the footgun `lazy_iter_for_stmt` documents; the await-capable drivers +/// here cannot carry an `await` in a `for` update clause, so they use this +/// shape). An SSE consumer's `continue` on ping hung a bundled CLI app. /// /// The synthetic `if done break` is appended AFTER /// `insert_iterator_return_before_abrupts` runs over the user body, so the @@ -981,7 +981,7 @@ pub(super) fn lower_stmt_for_of_inner( let next_call = if needs_await { Expr::Await(Box::new(raw_next_call)) } else { - raw_next_call + iterator_next_call(iter_id) }; module.init.push(Stmt::Let { id: result_id, diff --git a/crates/perry-runtime/src/collection_iter_object.rs b/crates/perry-runtime/src/collection_iter_object.rs index 839513b8cc..cb04007f73 100644 --- a/crates/perry-runtime/src/collection_iter_object.rs +++ b/crates/perry-runtime/src/collection_iter_object.rs @@ -61,7 +61,7 @@ fn iterator_class_id(addr: usize) -> Option { unsafe fn alloc_iterator(class_id: u32, coll_nanboxed: f64, kind: i32) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); let coll_h = scope.root_nanbox_f64(coll_nanboxed); - let obj_h = scope.root_raw_mut_ptr(js_object_alloc(class_id, 5)); + let obj_h = scope.root_raw_mut_ptr(js_object_alloc(class_id, 6)); let obj = || obj_h.across_mut::(|| ()).1; // Field 0: backing collection (NaN-boxed pointer so the GC scanner keeps it). js_object_set_field( @@ -81,6 +81,11 @@ unsafe fn alloc_iterator(class_id: u32, coll_nanboxed: f64, kind: i32) -> f64 { // Field 4: the KEY of the last-returned entry (a Map key / Set value), used // to re-derive the cursor after a delete-shift. Undefined until started. js_object_set_field(obj(), 4, JSValue::undefined()); + // Field 5: the recycled `{value, done}` result the FUSED for-of driver + // mutates in place (one allocation per loop, not per element). Manual + // `.next()` calls never touch it — they keep returning fresh objects, so + // a caller that retains results observes spec behavior. + js_object_set_field(obj(), 5, JSValue::undefined()); // Link `[[Prototype]]` to the shared `%MapIteratorPrototype%` / // `%SetIteratorPrototype%` singleton so `Object.getPrototypeOf(it)` and the // inherited `.next` read resolve. @@ -233,6 +238,14 @@ fn next_read_index(cursor: u32, last_key_in_place: bool, find_last: impl FnOnce( /// Dispatch `.next()` / `[Symbol.iterator]()` on a Map iterator object. pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_name: &str) -> f64 { + dispatch_map_iterator_method_emit(iter_obj, method_name, false) +} + +unsafe fn dispatch_map_iterator_method_emit( + iter_obj: *mut ObjectHeader, + method_name: &str, + emit_cached: bool, +) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); let iter_h = scope.root_nanbox_f64(js_nanbox_pointer(iter_obj as i64)); let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; @@ -248,7 +261,7 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n let map = || js_nanbox_get_pointer(map_h.get_nanbox_f64()) as *const MapHeader; let kind = f64::from_bits(js_object_get_field(iter_obj(), 2).bits()) as i32; if map().is_null() { - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let cursor = f64::from_bits(js_object_get_field(iter_obj(), 1).bits()) as u32; let last_key = js_object_get_field(iter_obj(), 4); @@ -268,7 +281,7 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n // Once a collection iterator is exhausted it stays exhausted, // even if entries are appended later. js_object_set_field(iter_obj(), 0, JSValue::undefined()); - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let entry_key = crate::map::js_map_entry_key_at(map(), idx); @@ -286,7 +299,7 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n JSValue::from_bits(make_pair_array(entry_key, val).to_bits()) } }; - make_iter_result(value, false) + emit_iter_result(&scope, &iter_h, emit_cached, value, false) } "Symbol.iterator" | "@@iterator" => js_nanbox_pointer(iter_obj() as i64), "return" | "throw" => make_iter_result(JSValue::undefined(), true), @@ -296,6 +309,14 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n /// Dispatch `.next()` / `[Symbol.iterator]()` on a Set iterator object. pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_name: &str) -> f64 { + dispatch_set_iterator_method_emit(iter_obj, method_name, false) +} + +unsafe fn dispatch_set_iterator_method_emit( + iter_obj: *mut ObjectHeader, + method_name: &str, + emit_cached: bool, +) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); let iter_h = scope.root_nanbox_f64(js_nanbox_pointer(iter_obj as i64)); let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; @@ -311,7 +332,7 @@ pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_n let set = || js_nanbox_get_pointer(set_h.get_nanbox_f64()) as *const SetHeader; let kind = f64::from_bits(js_object_get_field(iter_obj(), 2).bits()) as i32; if set().is_null() { - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let cursor = f64::from_bits(js_object_get_field(iter_obj(), 1).bits()) as u32; let last_val = js_object_get_field(iter_obj(), 4); @@ -326,7 +347,7 @@ pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_n if idx >= size { js_object_set_field(iter_obj(), 1, JSValue::number(size as f64)); js_object_set_field(iter_obj(), 0, JSValue::undefined()); - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let elem = crate::set::js_set_value_at(set(), idx); @@ -338,10 +359,188 @@ pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_n KIND_ENTRIES => JSValue::from_bits(make_pair_array(elem, elem).to_bits()), _ => JSValue::from_bits(elem.to_bits()), }; - make_iter_result(value, false) + emit_iter_result(&scope, &iter_h, emit_cached, value, false) } "Symbol.iterator" | "@@iterator" => js_nanbox_pointer(iter_obj() as i64), "return" | "throw" => make_iter_result(JSValue::undefined(), true), _ => f64::from_bits(TAG_UNDEFINED), } } + +/// Emit a `{value, done}` iterator result. +/// +/// `emit_cached == false` (every manual `.next()` and both public +/// dispatchers) allocates a fresh object per call, exactly as before — +/// results a caller retains behave per spec. +/// +/// `emit_cached == true` is reserved for [`js_for_of_next`], whose only +/// caller is the compiler's `for…of` desugar. There the result local is a +/// compiler temporary the loop body cannot name, read for `done`/`value` +/// before the next advance — so mutating one cached object per ITERATOR is +/// unobservable, and it deletes the per-element allocation that dominated +/// generic iteration. The cache lives in the iterator object's field 5, so +/// the GC traces and rewrites it like any other field. +unsafe fn emit_iter_result( + scope: &crate::gc::RuntimeHandleScope, + iter_h: &crate::gc::RuntimeHandle, + emit_cached: bool, + value: JSValue, + done: bool, +) -> f64 { + let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; + if !emit_cached { + return make_iter_result(value, done); + } + let cached = js_object_get_field(iter_obj(), 5); + if JSValue::from_bits(cached.bits()).is_pointer() { + let res = js_nanbox_get_pointer(f64::from_bits(cached.bits())) as *mut ObjectHeader; + // Barriered field stores: the iterator (and its cached result) may be + // tenured while `value` is young. + js_object_set_field(res, 0, value); + js_object_set_field(res, 1, JSValue::bool(done)); + return js_nanbox_pointer(res as i64); + } + // First fused advance on this iterator: build the result once and cache + // it. `make_iter_result` allocates, so root `value` across it. + let value_h = scope.root_nanbox_u64(value.bits()); + let res = make_iter_result(JSValue::from_bits(value_h.get_nanbox_u64()), done); + let res_h = scope.root_nanbox_f64(res); + js_object_set_field( + iter_obj(), + 5, + JSValue::from_bits(res_h.get_nanbox_f64().to_bits()), + ); + res_h.get_nanbox_f64() +} + +/// One fused `IteratorNext` for the `for…of` desugar: advance + result in a +/// single runtime call. +/// +/// A builtin Map/Set iterator advances in place and reuses its cached result +/// object (see [`emit_iter_result`]); the override probe inside the +/// dispatcher still runs first, so a patched `next` wins exactly as it does +/// on the manual path. Every other receiver — array iterators, generators, +/// user iterators — takes the arm at the bottom, which is byte-for-byte the +/// two-call desugar this entry replaces: the dynamic `.next()` dispatch +/// followed by spec IteratorNext result validation. +#[no_mangle] +pub unsafe extern "C-unwind" fn js_for_of_next(iter: f64) -> f64 { + let jv = JSValue::from_bits(iter.to_bits()); + if jv.is_pointer() { + let raw = js_nanbox_get_pointer(iter) as usize; + if raw != 0 && !crate::value::addr_class::is_small_handle(raw) { + if let Some(header) = crate::value::addr_class::try_read_gc_header(raw) { + if header.obj_type == crate::gc::GC_TYPE_OBJECT { + let obj = raw as *mut ObjectHeader; + let class_id = (*obj).class_id; + if class_id == MAP_ITERATOR_CLASS_ID { + return dispatch_map_iterator_method_emit(obj, "next", true); + } + if class_id == SET_ITERATOR_CLASS_ID { + return dispatch_set_iterator_method_emit(obj, "next", true); + } + } + } + } + } + let result = crate::object::js_native_call_method( + iter, + b"next".as_ptr() as *const i8, + 4, + std::ptr::null(), + 0, + ); + crate::symbol::js_iterator_result_validate(result) +} + +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_FOR_OF_NEXT: unsafe extern "C-unwind" fn(f64) -> f64 = js_for_of_next; + +#[cfg(test)] +mod fused_for_of_tests { + use super::*; + + unsafe fn value_of(res: f64) -> f64 { + f64::from_bits( + js_object_get_field(js_nanbox_get_pointer(res) as *mut ObjectHeader, 0).bits(), + ) + } + unsafe fn done_of(res: f64) -> bool { + JSValue::from_bits( + js_object_get_field(js_nanbox_get_pointer(res) as *mut ObjectHeader, 1).bits(), + ) + .as_bool() + } + + /// The fused driver must walk a Set in insertion order, terminate, and + /// keep its recycled result in the iterator's field 5 — while the manual + /// dispatcher keeps allocating fresh results a caller may retain. + #[test] + fn fused_next_walks_a_set_and_recycles_its_result() { + unsafe { + let set = crate::set::js_set_alloc(4); + for v in [10.0f64, 20.0, 30.0] { + crate::set::js_set_add(set, v); + } + let iter = js_nanbox_pointer(js_set_values_iter_obj(set)); + + let r1 = js_for_of_next(iter); + assert_eq!(value_of(r1), 10.0); + assert!(!done_of(r1)); + let cached = js_object_get_field(js_nanbox_get_pointer(iter) as *mut ObjectHeader, 5); + assert!( + JSValue::from_bits(cached.bits()).is_pointer(), + "the first fused advance must install the recycled result" + ); + assert_eq!(value_of(js_for_of_next(iter)), 20.0); + assert_eq!(value_of(js_for_of_next(iter)), 30.0); + assert!(done_of(js_for_of_next(iter)), "exhausted after three"); + assert!(done_of(js_for_of_next(iter)), "stays exhausted"); + + // The manual path still returns fresh, independent results. + let m1 = dispatch_set_iterator_method( + js_nanbox_get_pointer(js_nanbox_pointer(js_set_values_iter_obj(set))) + as *mut ObjectHeader, + "next", + ); + assert_eq!(value_of(m1), 10.0); + } + } + + /// Mid-iteration delete: the cursor-repair contract (#6075) must hold on + /// the fused path because it runs the SAME advance code as the manual one. + #[test] + fn fused_next_survives_a_mid_iteration_delete() { + unsafe { + let map = crate::map::js_map_alloc(8); + for k in [1.0f64, 2.0, 3.0, 4.0] { + crate::map::js_map_set(map, k, k * 10.0); + } + let iter = js_nanbox_pointer(js_map_keys_iter_obj(map)); + assert_eq!(value_of(js_for_of_next(iter)), 1.0); + // Deleting an EARLIER entry shifts the survivors down; the fused + // next must not skip or repeat. + crate::map::js_map_delete(map, 1.0); + assert_eq!(value_of(js_for_of_next(iter)), 2.0); + assert_eq!(value_of(js_for_of_next(iter)), 3.0); + assert_eq!(value_of(js_for_of_next(iter)), 4.0); + assert!(done_of(js_for_of_next(iter))); + } + } + + /// A non-collection receiver takes the generic arm: dynamic `.next()` + /// dispatch plus validation — here, an array VALUES iterator object. + #[test] + fn fused_next_routes_other_iterators_through_the_generic_arm() { + unsafe { + let arr = crate::array::js_array_alloc(2); + crate::array::js_array_push_f64(arr, 7.0); + crate::array::js_array_push_f64(arr, 8.0); + let iter = crate::array::array_values_iter(js_nanbox_pointer(arr as i64)); + assert_eq!(value_of(js_for_of_next(iter)), 7.0); + assert_eq!(value_of(js_for_of_next(iter)), 8.0); + assert!(done_of(js_for_of_next(iter))); + } + } +} diff --git a/crates/perry-runtime/src/object/iterator_prototypes.rs b/crates/perry-runtime/src/object/iterator_prototypes.rs index a30778ab26..71a34a4c2b 100644 --- a/crates/perry-runtime/src/object/iterator_prototypes.rs +++ b/crates/perry-runtime/src/object/iterator_prototypes.rs @@ -263,7 +263,41 @@ fn build_family_proto( proto } +/// Whether any iterator-prototype tower has been materialized on this thread. +#[cfg(test)] +pub(crate) fn iterator_prototypes_materialized() -> bool { + ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) != 0 +} + +#[cfg(test)] +mod override_probe_premise_tests { + use super::*; + + /// The fast path in `call_overridden_iterator_next` returns `None` on a + /// null tower, treating that as PROOF that no override exists. That is only + /// sound if every route to the prototype object materializes the tower — + /// this pins the route user code takes, `Object.getPrototypeOf(iter)`, + /// which lands in `iterator_prototype_for_class_id`. + /// + /// Deliberately one-directional: `perry-runtime`'s suite shares process + /// globals, so asserting the tower starts null would make this depend on + /// test order. The implication is what the fast path actually relies on. + #[test] + fn reaching_an_iterator_prototype_materializes_the_tower() { + assert!( + iterator_prototype_for_class_id(crate::array::ARRAY_ITERATOR_CLASS_ID).is_some(), + "array iterator must have a prototype to reach", + ); + assert!( + iterator_prototypes_materialized(), + "reaching a prototype must materialize the tower, or a null tower \ + would no longer prove the absence of an override", + ); + } +} + /// Lazily build the prototypes (idempotent). Cheap after the first call. + pub(crate) fn ensure_iterator_prototypes() { if ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == 0 { build_iterator_prototypes(); @@ -323,7 +357,16 @@ pub(crate) unsafe fn call_overridden_iterator_next( let scope = crate::gc::RuntimeHandleScope::new(); let iter = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(iter_obj as i64)); let previous = scope.root_nanbox_f64(super::js_implicit_this_get()); - ensure_iterator_prototypes(); + // An override can only be installed through the prototype OBJECT, and the + // only way user code obtains that object is `Object.getPrototypeOf(iter)` + // (or a direct prototype write), both of which materialize the tower. A + // null tower therefore PROVES no override exists — and building the tower + // here, as this probe used to, made every builtin `.next()` allocate a + // "next" key string and run a by-name prototype lookup just to learn + // nothing was patched. + if ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == 0 { + return None; + } let (slot, canonical): (&crate::object::RealmAtomicI64, *const u8) = match class_id { crate::array::ARRAY_ITERATOR_CLASS_ID => ( &ARRAY_ITERATOR_PROTOTYPE_PTR,