From a3c7eacaec999f6ff8f57027692d2d08c9bae9d0 Mon Sep 17 00:00:00 2001 From: Kamil Jarosz Date: Thu, 11 Sep 2025 21:29:26 +0200 Subject: [PATCH 1/2] avm2: Refactor Array.splice to ArrayStorage This refactor moves splice implementation from AVM2 to ArrayStorage. The goal was to improve performance of splice, which was only partially successful, as it works better, but the bottleneck is still the weird FP behavior with holes and the prototype. Nonetheless, this looks like a step in the right direction. --- core/src/avm2/array.rs | 73 +++++++++++++++++++++++++++++++++- core/src/avm2/globals/array.rs | 60 ++++++++++------------------ 2 files changed, 93 insertions(+), 40 deletions(-) diff --git a/core/src/avm2/array.rs b/core/src/avm2/array.rs index a099b05c3752..537102ad7197 100644 --- a/core/src/avm2/array.rs +++ b/core/src/avm2/array.rs @@ -358,7 +358,7 @@ impl<'gc> ArrayStorage<'gc> { /// Pop a value from the back of the array. /// - /// This method preferrentially pops non-holes from the array first. If a + /// This method preferentially pops non-holes from the array first. If a /// hole is popped, it will become `undefined`. pub fn pop(&mut self) -> Value<'gc> { match self { @@ -527,6 +527,77 @@ impl<'gc> ArrayStorage<'gc> { } } } + + pub fn splice( + &mut self, + start: usize, + delete_count: usize, + items: &[Value<'gc>], + ) -> ArrayStorage<'gc> { + let delete_count = delete_count.min(self.length() - start); + let end = start + delete_count; + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + let mut occupied_removed = 0; + let splice = storage + .splice(start..end, items.iter().map(|i| Some(*i))) + .inspect(|v| { + if v.is_some() { + occupied_removed += 1; + } + }) + .collect::>(); + *occupied_count = occupied_count.saturating_sub(occupied_removed); + ArrayStorage::from_storage(splice) + } + ArrayStorage::Sparse { storage, length } => { + // 1. Split the map into 3 ranges: + // storage, splice, remaining + let remaining = storage.split_off(&end); + let splice = storage.split_off(&start); + + // 2. Remap indices in remaining elements + let rshift = items.len() as isize - delete_count as isize; + let mut remaining = if rshift == 0 { + remaining + } else { + remaining + .into_iter() + .map(|(i, v)| (i.saturating_add_signed(rshift), v)) + .collect::>() + }; + + // 3. Put remaining elements back in original array + storage.append(&mut remaining); + + // 4. Put new items to the original array + for (i, item) in items.iter().enumerate() { + storage.insert(start + i, *item); + } + + // 5. Remap indices in splice + let splice = if start == 0 { + splice + } else { + splice + .into_iter() + .map(|(i, v)| (i.saturating_sub(start), v)) + .collect::>() + }; + + // 6. Update length + *length = length.saturating_add_signed(rshift); + + ArrayStorage::Sparse { + storage: splice, + length: delete_count, + } + } + } + } } impl<'gc, V> FromIterator for ArrayStorage<'gc> diff --git a/core/src/avm2/globals/array.rs b/core/src/avm2/globals/array.rs index 4c0931abc384..cea66b9e55bc 100644 --- a/core/src/avm2/globals/array.rs +++ b/core/src/avm2/globals/array.rs @@ -11,7 +11,7 @@ use crate::avm2::Error; use crate::string::AvmString; use bitflags::bitflags; use ruffle_macros::istr; -use std::cmp::{min, Ordering}; +use std::cmp::Ordering; use std::mem::swap; pub use crate::avm2::object::array_allocator; @@ -643,49 +643,31 @@ pub fn splice<'gc>( ) -> Result, Error<'gc>> { let this = this.as_object().unwrap(); - let array_length = this.as_array_storage().map(|a| a.length()); - - if let Some(array_length) = array_length { - if let Some(start) = args.get_optional(0) { - let actual_start = resolve_index(activation, start, array_length)?; - let delete_count = args - .get_optional(1) - .unwrap_or_else(|| array_length.into()) - .coerce_to_i32(activation)?; - - let actual_end = min(array_length, actual_start + delete_count as usize); - let args_slice = if args.len() > 2 { - args[2..].iter().cloned() - } else { - [].iter().cloned() - }; - - let contents = this - .as_array_storage() - .map(|a| a.iter().collect::>>>()) - .unwrap(); - - let mut resolved = Vec::with_capacity(contents.len()); - for (i, v) in contents.iter().enumerate() { - resolved.push(resolve_array_hole(activation, this, i, *v)?); - } + let Some(mut array_storage) = this.as_array_storage_mut(activation.gc()) else { + return Ok(Value::Undefined); + }; + let array_length = array_storage.length(); + let Some(start) = args.get_optional(0) else { + return Ok(Value::Undefined); + }; - let removed = resolved - .splice(actual_start..actual_end, args_slice) - .collect::>>(); - let removed_array = ArrayStorage::from_args(&removed[..]); + let actual_start = resolve_index(activation, start, array_length)?; + let delete_count = args + .get_optional(1) + .unwrap_or_else(|| array_length.into()) + .as_u32(); - let mut resolved_array = ArrayStorage::from_args(&resolved[..]); + let args_slice = if args.len() > 2 { &args[2..] } else { &[] }; - if let Some(mut array) = this.as_array_storage_mut(activation.gc()) { - swap(&mut *array, &mut resolved_array) - } - - return Ok(build_array(activation, removed_array)); - } + // FIXME Flash does not iterate over those elements like we do, it's too + // inefficient. Flash probably iterates through set properties only. + for i in actual_start..array_length { + let item = array_storage.get(i); + array_storage.set(i, resolve_array_hole(activation, this, i, item)?); } - Ok(Value::Undefined) + let ret = array_storage.splice(actual_start, delete_count as usize, args_slice); + Ok(build_array(activation, ret)) } /// Insert an element into a specific position of an array. From f443f73b8a426ecc61134818d8f2faa0da9e806a Mon Sep 17 00:00:00 2001 From: Kamil Jarosz Date: Thu, 11 Sep 2025 20:52:17 +0200 Subject: [PATCH 2/2] tests: Add avm2/array_splice2 test --- tests/tests/swfs/avm2/array_splice2/Test.as | 116 +++++ .../tests/swfs/avm2/array_splice2/output.txt | 428 ++++++++++++++++++ tests/tests/swfs/avm2/array_splice2/test.swf | Bin 0 -> 1933 bytes tests/tests/swfs/avm2/array_splice2/test.toml | 1 + 4 files changed, 545 insertions(+) create mode 100644 tests/tests/swfs/avm2/array_splice2/Test.as create mode 100644 tests/tests/swfs/avm2/array_splice2/output.txt create mode 100644 tests/tests/swfs/avm2/array_splice2/test.swf create mode 100644 tests/tests/swfs/avm2/array_splice2/test.toml diff --git a/tests/tests/swfs/avm2/array_splice2/Test.as b/tests/tests/swfs/avm2/array_splice2/Test.as new file mode 100644 index 000000000000..dee671e35ba7 --- /dev/null +++ b/tests/tests/swfs/avm2/array_splice2/Test.as @@ -0,0 +1,116 @@ +package { +import flash.display.*; + +public class Test extends MovieClip { + public function Test() { + var array_sparse1:Array = []; + array_sparse1[100000] = 5; + + var array_sparse2:Array = [1]; + array_sparse2[100000] = 5; + + var arrays:Array = [ + [], + [1], + [1, 2, 3, 4, 5], + array_sparse1, + array_sparse2, + [[1], [1, 2], [1, 2, 3]], + [1, 2, [3], 4, [5], 6], + ]; + + var i:int = 0; + while (i < arrays.length) { + testArray(i, arrays[i]); + ++i; + } + } + + private function testArray(i:int, array:Array):void { + trace("Testing array " + i + ":"); + trace(" array: " + arrToString(array)); + + trace(" splice():"); + testSplice(array, function(a:Array):Array { + return a.splice(); + }); + + trace(" splice(0):"); + testSplice(array, function(a:Array):Array { + return a.splice(0); + }); + + trace(" splice(1):"); + testSplice(array, function(a:Array):Array { + return a.splice(1); + }); + + trace(" splice(a.length):"); + testSplice(array, function(a:Array):Array { + return a.splice(a.length); + }); + + trace(" splice(a.length-1):"); + testSplice(array, function(a:Array):Array { + return a.splice(a.length-1); + }); + + trace(" splice(a.length+1):"); + testSplice(array, function(a:Array):Array { + return a.splice(a.length+1); + }); + + trace(" splice(a.length/2):"); + testSplice(array, function(a:Array):Array { + return a.splice(a.length/2); + }); + + trace(" splice(1,2):"); + testSplice(array, function(a:Array):Array { + return a.splice(1,2); + }); + + trace(" splice(1,2,3,4):"); + testSplice(array, function(a:Array):Array { + return a.splice(1,2,3,4); + }); + + trace(" splice(1,2,3,4,5,6):"); + testSplice(array, function(a:Array):Array { + return a.splice(1,2,3,4,5,6); + }); + } + + private function testSplice(array:Array, f:Function):void { + array = array.concat(); + var ret:Array = f(array); + + trace(" returned: " + arrToString(ret)); + trace(" after: " + arrToString(array)); + for (var i in [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 99999, 100000, 100001]) { + if (ret != null && ret[i] != null) { + trace(" returned[" + i + "]: " + ret[i]); + } + if (array != null && array[i] != null) { + trace(" after[" + i + "]: " + array[i]); + } + } + } + + private function arrToString(array:*):String { + if (array == null) { + return "null"; + } else if (!(array is Array)) { + return "" + array; + } else if (array.length > 1000) { + return ""; + } else if (array.length == 0) { + return ""; + } else { + return "[" + array.map(function(el:*, ...rest):String { + return arrToString(el); + }) + "]"; + } + } +} +} diff --git a/tests/tests/swfs/avm2/array_splice2/output.txt b/tests/tests/swfs/avm2/array_splice2/output.txt new file mode 100644 index 000000000000..6ed6750d3f7d --- /dev/null +++ b/tests/tests/swfs/avm2/array_splice2/output.txt @@ -0,0 +1,428 @@ +Testing array 0: + array: + splice(): + returned: null + after: + splice(0): + returned: + after: + splice(1): + returned: + after: + splice(a.length): + returned: + after: + splice(a.length-1): + returned: + after: + splice(a.length+1): + returned: + after: + splice(a.length/2): + returned: + after: + splice(1,2): + returned: + after: + splice(1,2,3,4): + returned: + after: [3,4] + after[0]: 3 + after[1]: 4 + splice(1,2,3,4,5,6): + returned: + after: [3,4,5,6] + after[0]: 3 + after[1]: 4 + after[2]: 5 + after[3]: 6 +Testing array 1: + array: [1] + splice(): + returned: null + after: [1] + after[0]: 1 + splice(0): + returned: [1] + after: + returned[0]: 1 + splice(1): + returned: + after: [1] + after[0]: 1 + splice(a.length): + returned: + after: [1] + after[0]: 1 + splice(a.length-1): + returned: [1] + after: + returned[0]: 1 + splice(a.length+1): + returned: + after: [1] + after[0]: 1 + splice(a.length/2): + returned: [1] + after: + returned[0]: 1 + splice(1,2): + returned: + after: [1] + after[0]: 1 + splice(1,2,3,4): + returned: + after: [1,3,4] + after[0]: 1 + after[1]: 3 + after[2]: 4 + splice(1,2,3,4,5,6): + returned: + after: [1,3,4,5,6] + after[0]: 1 + after[1]: 3 + after[2]: 4 + after[3]: 5 + after[4]: 6 +Testing array 2: + array: [1,2,3,4,5] + splice(): + returned: null + after: [1,2,3,4,5] + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + after[4]: 5 + splice(0): + returned: [1,2,3,4,5] + after: + returned[0]: 1 + returned[1]: 2 + returned[2]: 3 + returned[3]: 4 + returned[4]: 5 + splice(1): + returned: [2,3,4,5] + after: [1] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + returned[2]: 4 + returned[3]: 5 + splice(a.length): + returned: + after: [1,2,3,4,5] + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + after[4]: 5 + splice(a.length-1): + returned: [5] + after: [1,2,3,4] + returned[0]: 5 + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + splice(a.length+1): + returned: + after: [1,2,3,4,5] + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + after[4]: 5 + splice(a.length/2): + returned: [3,4,5] + after: [1,2] + returned[0]: 3 + after[0]: 1 + returned[1]: 4 + after[1]: 2 + returned[2]: 5 + splice(1,2): + returned: [2,3] + after: [1,4,5] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + after[1]: 4 + after[2]: 5 + splice(1,2,3,4): + returned: [2,3] + after: [1,3,4,4,5] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + after[1]: 3 + after[2]: 4 + after[3]: 4 + after[4]: 5 + splice(1,2,3,4,5,6): + returned: [2,3] + after: [1,3,4,5,6,4,5] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + after[1]: 3 + after[2]: 4 + after[3]: 5 + after[4]: 6 + after[5]: 4 + after[6]: 5 +Testing array 3: + array: + splice(): + returned: null + after: + splice(0): + returned: + after: + splice(1): + returned: + after: [null] + splice(a.length): + returned: + after: + splice(a.length-1): + returned: [5] + after: + returned[0]: 5 + splice(a.length+1): + returned: + after: + splice(a.length/2): + returned: + after: + splice(1,2): + returned: [null,null] + after: + splice(1,2,3,4): + returned: [null,null] + after: + after[1]: 3 + after[2]: 4 + splice(1,2,3,4,5,6): + returned: [null,null] + after: + after[1]: 3 + after[2]: 4 + after[3]: 5 + after[4]: 6 +Testing array 4: + array: + splice(): + returned: null + after: + after[0]: 1 + splice(0): + returned: + after: + returned[0]: 1 + splice(1): + returned: + after: [1] + after[0]: 1 + splice(a.length): + returned: + after: + after[0]: 1 + splice(a.length-1): + returned: [5] + after: + returned[0]: 5 + after[0]: 1 + splice(a.length+1): + returned: + after: + after[0]: 1 + splice(a.length/2): + returned: + after: + after[0]: 1 + splice(1,2): + returned: [null,null] + after: + after[0]: 1 + splice(1,2,3,4): + returned: [null,null] + after: + after[0]: 1 + after[1]: 3 + after[2]: 4 + splice(1,2,3,4,5,6): + returned: [null,null] + after: + after[0]: 1 + after[1]: 3 + after[2]: 4 + after[3]: 5 + after[4]: 6 +Testing array 5: + array: [[1],[1,2],[1,2,3]] + splice(): + returned: null + after: [[1],[1,2],[1,2,3]] + after[0]: 1 + after[1]: 1,2 + after[2]: 1,2,3 + splice(0): + returned: [[1],[1,2],[1,2,3]] + after: + returned[0]: 1 + returned[1]: 1,2 + returned[2]: 1,2,3 + splice(1): + returned: [[1,2],[1,2,3]] + after: [[1]] + returned[0]: 1,2 + after[0]: 1 + returned[1]: 1,2,3 + splice(a.length): + returned: + after: [[1],[1,2],[1,2,3]] + after[0]: 1 + after[1]: 1,2 + after[2]: 1,2,3 + splice(a.length-1): + returned: [[1,2,3]] + after: [[1],[1,2]] + returned[0]: 1,2,3 + after[0]: 1 + after[1]: 1,2 + splice(a.length+1): + returned: + after: [[1],[1,2],[1,2,3]] + after[0]: 1 + after[1]: 1,2 + after[2]: 1,2,3 + splice(a.length/2): + returned: [[1,2],[1,2,3]] + after: [[1]] + returned[0]: 1,2 + after[0]: 1 + returned[1]: 1,2,3 + splice(1,2): + returned: [[1,2],[1,2,3]] + after: [[1]] + returned[0]: 1,2 + after[0]: 1 + returned[1]: 1,2,3 + splice(1,2,3,4): + returned: [[1,2],[1,2,3]] + after: [[1],3,4] + returned[0]: 1,2 + after[0]: 1 + returned[1]: 1,2,3 + after[1]: 3 + after[2]: 4 + splice(1,2,3,4,5,6): + returned: [[1,2],[1,2,3]] + after: [[1],3,4,5,6] + returned[0]: 1,2 + after[0]: 1 + returned[1]: 1,2,3 + after[1]: 3 + after[2]: 4 + after[3]: 5 + after[4]: 6 +Testing array 6: + array: [1,2,[3],4,[5],6] + splice(): + returned: null + after: [1,2,[3],4,[5],6] + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + after[4]: 5 + after[5]: 6 + splice(0): + returned: [1,2,[3],4,[5],6] + after: + returned[0]: 1 + returned[1]: 2 + returned[2]: 3 + returned[3]: 4 + returned[4]: 5 + returned[5]: 6 + splice(1): + returned: [2,[3],4,[5],6] + after: [1] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + returned[2]: 4 + returned[3]: 5 + returned[4]: 6 + splice(a.length): + returned: + after: [1,2,[3],4,[5],6] + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + after[4]: 5 + after[5]: 6 + splice(a.length-1): + returned: [6] + after: [1,2,[3],4,[5]] + returned[0]: 6 + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + after[4]: 5 + splice(a.length+1): + returned: + after: [1,2,[3],4,[5],6] + after[0]: 1 + after[1]: 2 + after[2]: 3 + after[3]: 4 + after[4]: 5 + after[5]: 6 + splice(a.length/2): + returned: [4,[5],6] + after: [1,2,[3]] + returned[0]: 4 + after[0]: 1 + returned[1]: 5 + after[1]: 2 + returned[2]: 6 + after[2]: 3 + splice(1,2): + returned: [2,[3]] + after: [1,4,[5],6] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + after[1]: 4 + after[2]: 5 + after[3]: 6 + splice(1,2,3,4): + returned: [2,[3]] + after: [1,3,4,4,[5],6] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + after[1]: 3 + after[2]: 4 + after[3]: 4 + after[4]: 5 + after[5]: 6 + splice(1,2,3,4,5,6): + returned: [2,[3]] + after: [1,3,4,5,6,4,[5],6] + returned[0]: 2 + after[0]: 1 + returned[1]: 3 + after[1]: 3 + after[2]: 4 + after[3]: 5 + after[4]: 6 + after[5]: 4 + after[6]: 5 + after[7]: 6 diff --git a/tests/tests/swfs/avm2/array_splice2/test.swf b/tests/tests/swfs/avm2/array_splice2/test.swf new file mode 100644 index 0000000000000000000000000000000000000000..3dae069b5d554b7a4f856c54dc2d7e43f35aa2ed GIT binary patch literal 1933 zcmV;82XgpBS5ptK3;+Ol0iBjjY#YZF$7g20_$}&-qG-#e?M-Cb{8*Bf98oeYMlvls zjpbSnliFp4CAlkYOf3m=Daoe_Mo&e34Ukh66zHYL+}mD)qCJr|8sB>8$v18fJ*#hK zX-c#U6sUk*?)(4Vdo%Oq&1w%3|4)PhzegyH(fW88A@s}1w-}+7R!z@6S}#Tq_v&Ul z2iC3mUCV0bQmKQ3gXBRr*=X&g(o0KADJ7H2WD?+!Xg@bC^)O+!ugvFJf_1H3Z5d6= zXqZu&P^~mN)~)&ZLDpJzn6%kx)mh3~HKoqb?j z=~9P@_HLv4>_FYu5_(;2@2;dy9cheZSamI5XsXp+En2K=hta}mICFM9sBi6bp83Hl zb%;o+ESk-WMV$4cam`MpZnSr`*1K`G8oG6$wqU?U$E>lLvcOYE8dhy-s@3?PLGRpX zNL@8|I_i#=*UarNwph-g#8#kMT7FAwM$_qdG^1n|ocU69B}GerW-A5?>~0V%!cy5%B zBsuReZJ#!J?hgH-T(zKETW$*q!`3U-HFL+>b<;|v=nbnPtj4Hn-l31I2&_ng1*2;L z>SdTaQKm(4&K-3gb5Soyj~iQ73$QmDb@E(|{c%7KZ~AnU;|(Av#5+t^2OPvcj94dRFY3$d9p9Kw}u@gM)Zt9V1xu zI84m#;kwFQP|1%(a21K2o{>RCQeGZmEF!6c=eLRgW z!&0vFiUQg|h$El|sX7np)t-!s0tTHQ>T%E$JsCT?K=pCzp9VeC>*>4$L=M0d1&aV~ zQXu2+93~CfGO!hg9Xrf5WUIgmjFI0t%uU$@&3Ok{5hfv%qCjAJ9sx**lKVhDV}#bT zDDXhN63_^xn?N59sVvB1Q4qny$MatQ4+Q93SNZbV8>NqD$ef0IdsxG-_6Te#n(N~* zp0h8M#dt)+b$lE@cU%;4%)SW9`7TJLC}Bss1kwctFEg-8iY^j^^dAF=GBED|ZUC+W zxWqu*0sH_c04_6-c7QMd3jnS#u;>7B0CE6i29_Nl4Zv*xA2G1%0M`Jl1NfMMI}UIU zz&!x-4BU5sX8;}mxXQq$1H1$9=q@5|ggwX$9ftgV$>aSuJu8p9%B2@o9%_gl_i-G< zEkR23s64U1=oS6!u#{fadIdNnZ|tuif2#X>SkRw=t#o@s`-zOIf{dKgK+l3K%98Ao z-3gMw39jUSQ5A#F;4oCgif{o2wx7O^{^6VbYx@)MbrWN!gW3}zgzTR~@^pnij%SV; zaQ5&Qy)ywldYpy<+Ypd-0vK=to@7K}{5K~jF3}~}6*yu6!g;;ZeY}XwKE8=fC_|$x z(M3SZjd7gU|7%*B8g02Bq2-ovjkO#tQq9(AlSQWlgyK0{->`S6UB){8{u{c6II(wG z8*bPJ{ndjXy_Ev+@fTmVzjlP<_{#By{me0B#8Ta|>l^XBZBlVHP#lLgrU$Kgyn(Qn ze)a_kyy|Xu6%ct1m8bM6r97=qE9Hj8YEkVWm8)&no3feUjb2LUYdF?gYdU TSUbG$-+e-^!oz<6C`X9_INYb* literal 0 HcmV?d00001 diff --git a/tests/tests/swfs/avm2/array_splice2/test.toml b/tests/tests/swfs/avm2/array_splice2/test.toml new file mode 100644 index 000000000000..cf6123969a1d --- /dev/null +++ b/tests/tests/swfs/avm2/array_splice2/test.toml @@ -0,0 +1 @@ +num_ticks = 1