From f992be8c6f149d12124431fccc24273b564b4e00 Mon Sep 17 00:00:00 2001 From: yush-1018 Date: Sun, 15 Mar 2026 02:23:14 +0530 Subject: [PATCH 1/3] perf: add dense array fast-path for Array.prototype.unshift --- core/engine/src/builtins/array/mod.rs | 43 +++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/core/engine/src/builtins/array/mod.rs b/core/engine/src/builtins/array/mod.rs index acd695878ea..5ca69701b86 100644 --- a/core/engine/src/builtins/array/mod.rs +++ b/core/engine/src/builtins/array/mod.rs @@ -1291,6 +1291,49 @@ impl Array { .with_message("length + number of arguments exceeds the max safe integer limit") .into()); } + + // Small optimization for arrays using dense properties. + // Mirrors the fast-path in `shift`. + if o.is_array() { + let mut o_borrow = o.borrow_mut(); + let props = &mut o_borrow.properties_mut().indexed_properties; + + let fast_path_done = match props { + IndexedProperties::DenseI32(dense) if len <= dense.len() as u64 => { + // Only take the fast path if ALL new items are i32. + let all_i32: Option> = args.iter().map(JsValue::as_i32).collect(); + if let Some(items) = all_i32 { + dense.splice(0..0, items); + true + } else { + false + } + } + IndexedProperties::DenseF64(dense) if len <= dense.len() as u64 => { + let all_f64: Option> = + args.iter().map(JsValue::as_number).collect(); + if let Some(items) = all_f64 { + dense.splice(0..0, items); + true + } else { + false + } + } + IndexedProperties::DenseElement(dense) if len <= dense.len() as u64 => { + dense.splice(0..0, args.iter().cloned()); + true + } + _ => false, + }; + + drop(o_borrow); + + if fast_path_done { + Self::set_length(&o, len + arg_count, context)?; + return Ok((len + arg_count).into()); + } + } + // b. Let k be len. let mut k = len; // c. Repeat, while k > 0, From 216d58348dcdc0a8f5784b5fa0c0d6e4fbab7267 Mon Sep 17 00:00:00 2001 From: yush-1018 Date: Wed, 18 Mar 2026 04:03:46 +0530 Subject: [PATCH 2/3] fix: use len > 0 guard for unshift fast path --- core/engine/src/builtins/array/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/engine/src/builtins/array/mod.rs b/core/engine/src/builtins/array/mod.rs index 5ca69701b86..22c46e158e3 100644 --- a/core/engine/src/builtins/array/mod.rs +++ b/core/engine/src/builtins/array/mod.rs @@ -1294,7 +1294,11 @@ impl Array { // Small optimization for arrays using dense properties. // Mirrors the fast-path in `shift`. - if o.is_array() { + // Guard: only take the fast path when len > 0, because for empty + // arrays (len == 0) there are no own indexed properties and `Set` + // must traverse the prototype chain (which may have setters that + // freeze the array or make `length` non-writable mid-operation). + if o.is_array() && len > 0 { let mut o_borrow = o.borrow_mut(); let props = &mut o_borrow.properties_mut().indexed_properties; From a75a11d5b75640676e05d2bc78f69b231d1d091c Mon Sep 17 00:00:00 2001 From: yush-1018 Date: Thu, 19 Mar 2026 14:30:36 +0530 Subject: [PATCH 3/3] fix(engine): remove flawed unshift fast path The unshift fast path introduced in #5076 incorrectly bypassed the prototype chain when setting new elements, leading to unobserved setters. This removes the fast path to ensure spec compliance and adds a regression test. --- core/engine/src/builtins/array/mod.rs | 46 ------------------------- core/engine/src/builtins/array/tests.rs | 18 ++++++++++ 2 files changed, 18 insertions(+), 46 deletions(-) diff --git a/core/engine/src/builtins/array/mod.rs b/core/engine/src/builtins/array/mod.rs index 22c46e158e3..ff79728832f 100644 --- a/core/engine/src/builtins/array/mod.rs +++ b/core/engine/src/builtins/array/mod.rs @@ -1292,52 +1292,6 @@ impl Array { .into()); } - // Small optimization for arrays using dense properties. - // Mirrors the fast-path in `shift`. - // Guard: only take the fast path when len > 0, because for empty - // arrays (len == 0) there are no own indexed properties and `Set` - // must traverse the prototype chain (which may have setters that - // freeze the array or make `length` non-writable mid-operation). - if o.is_array() && len > 0 { - let mut o_borrow = o.borrow_mut(); - let props = &mut o_borrow.properties_mut().indexed_properties; - - let fast_path_done = match props { - IndexedProperties::DenseI32(dense) if len <= dense.len() as u64 => { - // Only take the fast path if ALL new items are i32. - let all_i32: Option> = args.iter().map(JsValue::as_i32).collect(); - if let Some(items) = all_i32 { - dense.splice(0..0, items); - true - } else { - false - } - } - IndexedProperties::DenseF64(dense) if len <= dense.len() as u64 => { - let all_f64: Option> = - args.iter().map(JsValue::as_number).collect(); - if let Some(items) = all_f64 { - dense.splice(0..0, items); - true - } else { - false - } - } - IndexedProperties::DenseElement(dense) if len <= dense.len() as u64 => { - dense.splice(0..0, args.iter().cloned()); - true - } - _ => false, - }; - - drop(o_borrow); - - if fast_path_done { - Self::set_length(&o, len + arg_count, context)?; - return Ok((len + arg_count).into()); - } - } - // b. Let k be len. let mut k = len; // c. Repeat, while k > 0, diff --git a/core/engine/src/builtins/array/tests.rs b/core/engine/src/builtins/array/tests.rs index 7f8d2d98f72..12255e848e0 100644 --- a/core/engine/src/builtins/array/tests.rs +++ b/core/engine/src/builtins/array/tests.rs @@ -249,6 +249,24 @@ fn unshift() { TestAction::assert_eq("arr.unshift(1, 2)", 4), TestAction::assert("arrayEquals(arr, [1, 2, 3, 4])"), ]); + + // Test case from PR 5076 ensuring unshift doesn't bypass setters + run_test_actions([ + TestAction::run_harness(), + TestAction::run(indoc! {r#" + var array = [1]; + Object.defineProperty(Array.prototype, "1", { + set(_val) { + Object.freeze(array); + }, + }); + "#}), + TestAction::assert_native_error( + "array.unshift(0)", + JsNativeErrorKind::Type, + "cannot set non-writable property: 0", + ), + ]); } #[test]