diff --git a/core/src/avm2/array.rs b/core/src/avm2/array.rs index 1b7d8bbf4..fa69388c7 100644 --- a/core/src/avm2/array.rs +++ b/core/src/avm2/array.rs @@ -2,7 +2,10 @@ use crate::avm2::value::Value; use gc_arena::Collect; -use std::{cmp::max, ops::RangeBounds}; +use std::collections::BTreeMap; + +const MIN_SPARSE_LENGTH: usize = 32; +const MAX_DENSE_LENGTH: usize = 1 << 28; /// The array storage portion of an array object. /// @@ -11,8 +14,62 @@ use std::{cmp::max, ops::RangeBounds}; /// the prototype. #[derive(Clone, Collect)] #[collect(no_drop)] -pub struct ArrayStorage<'gc> { - storage: Vec>>, +pub enum ArrayStorage<'gc> { + /// Dense arrays store a vector of values and a count of non-holes in the vector (m_denseUsed in avmplus). + Dense { + storage: Vec>>, + occupied_count: usize, + }, + /// Sparse arrays store a BTreeMap of values and an explicit ECMAScript "Array.length". + Sparse { + storage: BTreeMap>, + length: usize, + }, +} + +/// An iterator over array storage. This iterator will yield `Some(None)` for holes. +struct ArrayStorageIterator<'a, 'gc> { + storage: &'a ArrayStorage<'gc>, + index: usize, + index_back: usize, +} + +impl<'a, 'gc> Iterator for ArrayStorageIterator<'a, 'gc> { + type Item = Option>; + + fn next(&mut self) -> Option { + if self.index >= self.index_back { + None + } else { + let value = match &self.storage { + ArrayStorage::Dense { storage, .. } => storage[self.index], + ArrayStorage::Sparse { storage, .. } => storage.get(&self.index).copied(), + }; + self.index += 1; + Some(value) + } + } +} + +impl<'a, 'gc> DoubleEndedIterator for ArrayStorageIterator<'a, 'gc> { + fn next_back(&mut self) -> Option { + if self.index >= self.index_back || self.index_back == 0 { + None + } else { + self.index_back -= 1; + let value = match &self.storage { + ArrayStorage::Dense { storage, .. } => storage[self.index_back], + ArrayStorage::Sparse { storage, .. } => storage.get(&self.index_back).copied(), + }; + Some(value) + } + } +} + +impl<'a, 'gc> ExactSizeIterator for ArrayStorageIterator<'a, 'gc> { + fn len(&self) -> usize { + self.index_back - self.index + } } impl<'gc> ArrayStorage<'gc> { @@ -21,8 +78,16 @@ impl<'gc> ArrayStorage<'gc> { /// The length parameter indicates how big the array storage should start /// out as. All array storage consists of holes. pub fn new(length: usize) -> Self { - Self { - storage: vec![None; length], + if length > MAX_DENSE_LENGTH { + ArrayStorage::Sparse { + storage: BTreeMap::new(), + length, + } + } else { + ArrayStorage::Dense { + storage: Vec::with_capacity(length), + occupied_count: 0, + } } } @@ -33,12 +98,20 @@ impl<'gc> ArrayStorage<'gc> { .map(|v| Some(*v)) .collect::>>>(); - Self { storage } + ArrayStorage::Dense { + storage, + occupied_count: values.len(), + } } /// Wrap an existing storage Vec in an `ArrayStorage`. pub fn from_storage(storage: Vec>>) -> Self { - Self { storage } + let occupied_count = storage.iter().filter(|v| v.is_some()).count(); + + ArrayStorage::Dense { + storage, + occupied_count, + } } /// Retrieve a value from array storage by index. @@ -46,7 +119,34 @@ impl<'gc> ArrayStorage<'gc> { /// Array holes and out of bounds values will be treated the same way, by /// yielding `None`. pub fn get(&self, item: usize) -> Option> { - self.storage.get(item).cloned().unwrap_or(None) + match &self { + ArrayStorage::Dense { storage, .. } => { + return storage.get(item).copied().unwrap_or(None); + } + ArrayStorage::Sparse { storage, .. } => storage.get(&item).copied(), + } + } + + /// Get the next index after the given index that contains a value. + pub fn get_next_enumerant(&self, last_index: usize) -> Option { + match &self { + ArrayStorage::Dense { storage, .. } => { + let mut last_index = last_index; + while last_index < storage.len() { + if storage[last_index].is_some() { + return Some(last_index + 1); + } + last_index += 1; + } + None + } + ArrayStorage::Sparse { storage, .. } => { + if let Some((&key, _)) = storage.range(last_index..).next() { + return Some(key + 1); + } + None + } + } } /// Set an array storage slot to a particular value. @@ -54,28 +154,95 @@ impl<'gc> ArrayStorage<'gc> { /// If the item index extends beyond the length of the array, then the /// array will be extended with holes. pub fn set(&mut self, item: usize, value: Value<'gc>) { - if self.storage.len() < (item + 1) { - self.storage.resize(item + 1, None) + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + if item >= storage.len() { + if Self::should_convert_to_sparse(item + 1, *occupied_count) { + self.convert_to_sparse(); + if let ArrayStorage::Sparse { storage, length } = self { + *length = item + 1; + storage.insert(item, value); + } + } else { + storage.resize(item + 1, None); + storage[item] = Some(value); + *occupied_count += 1; + } + } else { + if storage[item].is_none() { + *occupied_count += 1; + } + storage[item] = Some(value); + } + } + ArrayStorage::Sparse { storage, length } => { + storage.insert(item, value); + if item >= *length { + *length = item + 1; + } + } } - - *self.storage.get_mut(item).unwrap() = Some(value) } /// Delete an array storage slot, leaving a hole. pub fn delete(&mut self, item: usize) { - if let Some(i) = self.storage.get_mut(item) { - *i = None; + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + if let Some(i) = storage.get_mut(item) { + *i = None; + if *occupied_count > 0 { + *occupied_count -= 1; + self.maybe_convert_to_sparse(); + } + } + } + ArrayStorage::Sparse { storage, .. } => { + storage.remove(&item); + self.maybe_convert_to_dense(); + } } } /// Get the length of the array. pub fn length(&self) -> usize { - self.storage.len() + match &self { + ArrayStorage::Dense { storage, .. } => storage.len(), + ArrayStorage::Sparse { length, .. } => *length, + } } /// Set the length of the array. pub fn set_length(&mut self, size: usize) { - self.storage.resize(size, None) + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + if Self::should_convert_to_sparse(size, *occupied_count) { + self.convert_to_sparse(); + if let ArrayStorage::Sparse { .. } = self { + self.set_length(size); + } + } else { + storage.resize(size, None); + } + } + ArrayStorage::Sparse { storage, length } => { + if size > *length { + *length = size; + } else { + storage.retain(|&k, _| k < size); + *length = size; + self.maybe_convert_to_dense(); + } + } + } } /// Append the contents of another array into this one. @@ -85,8 +252,12 @@ impl<'gc> ArrayStorage<'gc> { /// /// Holes are copied as holes and not resolved at append time. pub fn append(&mut self, other_array: &Self) { - for other_item in other_array.storage.iter() { - self.storage.push(*other_item) + for value in other_array.iter() { + if let Some(value) = value { + self.push(value); + } else { + self.push_hole(); + } } } @@ -94,12 +265,78 @@ impl<'gc> ArrayStorage<'gc> { /// /// It is not possible to push a hole onto the array. pub fn push(&mut self, item: Value<'gc>) { - self.storage.push(Some(item)) + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + storage.push(Some(item)); + *occupied_count += 1; + } + ArrayStorage::Sparse { storage, length } => { + storage.insert(*length, item); + *length += 1; + } + } + } + + /// Determine if the array should be converted to a sparse representation based on its size and the number of occupied slots. + fn should_convert_to_sparse(size: usize, occupied_count: usize) -> bool { + (occupied_count < (size / 4) && size > MIN_SPARSE_LENGTH) || size > MAX_DENSE_LENGTH + } + + /// Convert the array storage to a sparse representation. + fn convert_to_sparse(&mut self) { + if let ArrayStorage::Dense { storage, .. } = self { + let mut new_storage = BTreeMap::new(); + for (i, v) in storage.iter().enumerate() { + if let Some(v) = v { + new_storage.insert(i, *v); + } + } + *self = ArrayStorage::Sparse { + storage: new_storage, + length: storage.len(), + }; + } + } + + /// Convert the array to a sparse representation if it meets the criteria. + fn maybe_convert_to_sparse(&mut self) { + if let ArrayStorage::Dense { + storage, + occupied_count, + } = self + { + if Self::should_convert_to_sparse(storage.len(), *occupied_count) { + self.convert_to_sparse(); + } + } + } + + /// Convert the array to a dense representation if it meets the criteria. + fn maybe_convert_to_dense(&mut self) { + if let ArrayStorage::Sparse { storage, length } = self { + if storage.is_empty() && *length == 0 { + *self = ArrayStorage::Dense { + storage: Vec::new(), + occupied_count: 0, + }; + } + } } /// Push an array hole onto the end of this array. pub fn push_hole(&mut self) { - self.storage.push(None) + match self { + ArrayStorage::Dense { storage, .. } => { + storage.push(None); + self.maybe_convert_to_sparse(); + } + ArrayStorage::Sparse { length, .. } => { + *length += 1; + } + } } /// Pop a value from the back of the array. @@ -107,22 +344,38 @@ impl<'gc> ArrayStorage<'gc> { /// This method preferrentially pops non-holes from the array first. If a /// hole is popped, it will become `undefined`. pub fn pop(&mut self) -> Value<'gc> { - let mut non_hole = None; + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + let non_hole = storage + .iter() + .enumerate() + .rposition(|(_, item)| item.is_some()); - for (i, item) in self.storage.iter().enumerate().rev() { - if item.is_some() { - non_hole = Some(i); - break; + if let Some(non_hole) = non_hole { + *occupied_count -= 1; + let value = storage.remove(non_hole).unwrap(); + self.maybe_convert_to_sparse(); + value + } else { + storage.pop().unwrap_or(None).unwrap_or(Value::Undefined) + } + } + ArrayStorage::Sparse { storage, length } => { + let non_hole = storage.pop_last(); + if let Some((_index, value)) = non_hole { + *length -= 1; + value + } else { + if *length > 0 { + *length -= 1; + } + self.maybe_convert_to_dense(); + Value::Undefined + } } - } - - if let Some(non_hole) = non_hole { - self.storage.remove(non_hole).unwrap() - } else { - self.storage - .pop() - .unwrap_or(None) - .unwrap_or(Value::Undefined) } } @@ -131,10 +384,38 @@ impl<'gc> ArrayStorage<'gc> { /// This method preferrentially pops non-holes from the array first. If a /// hole is popped, it will become `undefined`. pub fn shift(&mut self) -> Value<'gc> { - if !self.storage.is_empty() { - self.storage.remove(0).unwrap_or(Value::Undefined) - } else { - Value::Undefined + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + if !storage.is_empty() { + let value = storage.remove(0); + if value.is_some() { + *occupied_count -= 1; + } + self.maybe_convert_to_sparse(); + return value.unwrap_or(Value::Undefined); + } + Value::Undefined + } + ArrayStorage::Sparse { storage, length } => { + let value = storage.get(&0).copied().unwrap_or(Value::Undefined); + storage.remove(&0); + + let mut new_storage = BTreeMap::new(); + for (&key, &value) in storage.iter() { + new_storage.insert(key - 1, value); + } + + *storage = new_storage; + + if *length > 0 { + *length -= 1; + } + self.maybe_convert_to_dense(); + value + } } } @@ -142,7 +423,24 @@ impl<'gc> ArrayStorage<'gc> { /// /// It is not possible to push a hole onto the array. pub fn unshift(&mut self, item: Value<'gc>) { - self.storage.insert(0, Some(item)) + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + storage.insert(0, Some(item)); + *occupied_count += 1; + } + ArrayStorage::Sparse { storage, length } => { + let mut new_storage = BTreeMap::new(); + new_storage.insert(0, item); + for (key, value) in storage.iter() { + new_storage.insert(key + 1, *value); + } + *storage = new_storage; + *length += 1; + } + } } /// Iterate over array values. @@ -151,7 +449,12 @@ impl<'gc> ArrayStorage<'gc> { ) -> impl DoubleEndedIterator>> + ExactSizeIterator>> + 'a { - self.storage.iter().cloned() + let index_back = self.length(); + ArrayStorageIterator { + storage: self, + index: 0, + index_back, + } } /// Remove a value from a specific position in the array. @@ -162,32 +465,51 @@ impl<'gc> ArrayStorage<'gc> { /// Negative bounds are supported and treated as indexing from the end of /// the array, backwards. pub fn remove(&mut self, position: i32) -> Option> { - let position = if position < 0 { - max(position + self.storage.len() as i32, 0) as usize - } else { - position as usize - }; + match self { + ArrayStorage::Dense { + storage, + occupied_count, + } => { + let position = if position < 0 { + std::cmp::max(position + storage.len() as i32, 0) as usize + } else { + position as usize + }; - if position >= self.storage.len() { - None - } else { - self.storage.remove(position) + if position >= storage.len() { + None + } else { + let value = storage.remove(position); + if value.is_some() { + *occupied_count -= 1; + } + self.maybe_convert_to_sparse(); + value + } + } + ArrayStorage::Sparse { storage, length } => { + let position = if position < 0 { + std::cmp::max(position + *length as i32, 0) as usize + } else { + position as usize + }; + + if position >= *length { + None + } else { + let value = storage.get(&position).copied(); + storage.remove(&position); + *length -= 1; + let new_storage = storage.split_off(&position); + for (&key, &value) in new_storage.iter() { + storage.insert(key - 1, value); + } + self.maybe_convert_to_dense(); + value + } + } } } - - pub fn splice<'a, R, I>( - &'a mut self, - range: R, - replace_with: I, - ) -> impl Iterator>> + 'a - where - R: RangeBounds, - I: IntoIterator>, - ::IntoIter: 'a, - { - self.storage - .splice(range, replace_with.into_iter().map(Some)) - } } impl<'gc, V> FromIterator for ArrayStorage<'gc> @@ -198,8 +520,13 @@ where where I: IntoIterator, { - let storage = values.into_iter().map(|v| Some(v.into())).collect(); + let storage: Vec<_> = values.into_iter().map(|v| Some(v.into())).collect(); - Self { storage } + let occupied_count = storage.iter().filter(|v| v.is_some()).count(); + + ArrayStorage::Dense { + storage, + occupied_count, + } } } diff --git a/core/src/avm2/globals/array.rs b/core/src/avm2/globals/array.rs index 0a887c6a3..e89323ae1 100644 --- a/core/src/avm2/globals/array.rs +++ b/core/src/avm2/globals/array.rs @@ -89,12 +89,6 @@ pub fn instance_init<'gc>( )?)); } - // [NA] temporarily limit this. It may not be correct but it's better than 100GB arrays. - // TODO: sparse array support - if expected_len > (1 << 28) as f64 { - return Err("Ruffle does not support sparse arrays yet.".into()); - } - array.set_length(expected_len as usize); return Ok(Value::Undefined); diff --git a/core/src/avm2/object/array_object.rs b/core/src/avm2/object/array_object.rs index 1dba95f92..af178a93a 100644 --- a/core/src/avm2/object/array_object.rs +++ b/core/src/avm2/object/array_object.rs @@ -133,11 +133,6 @@ impl<'gc> TObject<'gc> for ArrayObject<'gc> { if name.contains_public_namespace() { if let Some(name) = name.local_name() { if let Ok(index) = name.parse::() { - // [NA] temporarily limit this. It may not be correct but it's better than 100GB arrays. - // TODO: sparse array support - if index > 1 << 28 { - return Err("Ruffle does not support sparse arrays yet.".into()); - } write.array.set(index, value); return Ok(()); } @@ -158,11 +153,6 @@ impl<'gc> TObject<'gc> for ArrayObject<'gc> { if name.contains_public_namespace() { if let Some(name) = name.local_name() { if let Ok(index) = name.parse::() { - // [NA] temporarily limit this. It may not be correct but it's better than 100GB arrays. - // TODO: sparse array support - if index > 1 << 28 { - return Err("Ruffle does not support sparse arrays yet.".into()); - } write.array.set(index, value); return Ok(()); } @@ -217,13 +207,12 @@ impl<'gc> TObject<'gc> for ArrayObject<'gc> { let array_length = read.array.length() as u32; // Array enumeration skips over holes. - while last_index < array_length { - if read.array.get(last_index as usize).is_some() { - return Ok(Some(last_index + 1)); - } - last_index += 1; + if let Some(index) = read.array.get_next_enumerant(last_index as usize) { + return Ok(Some(index as u32)); } + last_index = std::cmp::max(last_index, array_length); + drop(read); // After enumerating all of the 'normal' array entries, diff --git a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_1_2/test.toml b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_1_2/test.toml index 29f3cef79..cf6123969 100644 --- a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_1_2/test.toml +++ b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_1_2/test.toml @@ -1,2 +1 @@ num_ticks = 1 -known_failure = true diff --git a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2/test.toml b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2/test.toml index 29f3cef79..cf6123969 100644 --- a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2/test.toml +++ b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2/test.toml @@ -1,2 +1 @@ num_ticks = 1 -known_failure = true diff --git a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_1/test.toml b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_1/test.toml index 29f3cef79..cf6123969 100644 --- a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_1/test.toml +++ b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_1/test.toml @@ -1,2 +1 @@ num_ticks = 1 -known_failure = true diff --git a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_2/test.toml b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_2/test.toml index 29f3cef79..cf6123969 100644 --- a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_2/test.toml +++ b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_2_2_2/test.toml @@ -1,2 +1 @@ num_ticks = 1 -known_failure = true diff --git a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_5_2_1/test.toml b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_5_2_1/test.toml index 29f3cef79..cf6123969 100644 --- a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_5_2_1/test.toml +++ b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4_5_2_1/test.toml @@ -1,2 +1 @@ num_ticks = 1 -known_failure = true diff --git a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4__1/test.toml b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4__1/test.toml index 29f3cef79..cf6123969 100644 --- a/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4__1/test.toml +++ b/tests/tests/swfs/from_avmplus/ecma3/Array/e15_4__1/test.toml @@ -1,2 +1 @@ num_ticks = 1 -known_failure = true