diff --git a/boa_engine/src/builtins/array/mod.rs b/boa_engine/src/builtins/array/mod.rs index 9ef6b162f7..829f601e1b 100644 --- a/boa_engine/src/builtins/array/mod.rs +++ b/boa_engine/src/builtins/array/mod.rs @@ -266,15 +266,23 @@ impl Array { // 2. Let array be ! ArrayCreate(0). let array = Self::array_create(0, None, context) .expect("creating an empty array with the default prototype must not fail"); + // 3. Let n be 0. // 4. For each element e of elements, do - for (i, elem) in elements.into_iter().enumerate() { - // a. Perform ! CreateDataPropertyOrThrow(array, ! ToString(𝔽(n)), e). - array - .create_data_property_or_throw(i, elem, context) - .expect("new array must be extensible"); - // b. Set n to n + 1. - } + // a. Perform ! CreateDataPropertyOrThrow(array, ! ToString(𝔽(n)), e). + // b. Set n to n + 1. + // + // NOTE: This deviates from the spec, but it should have the same behaviour. + let elements: Vec<_> = elements.into_iter().collect(); + let length = elements.len(); + array + .borrow_mut() + .properties_mut() + .override_indexed_properties(elements); + array + .set("length", length, true, context) + .expect("Should not fail"); + // 5. Return array. array } diff --git a/boa_engine/src/object/internal_methods/array.rs b/boa_engine/src/object/internal_methods/array.rs index 007af7a0a7..2abc0b2de6 100644 --- a/boa_engine/src/object/internal_methods/array.rs +++ b/boa_engine/src/object/internal_methods/array.rs @@ -201,8 +201,7 @@ fn array_set_length( .borrow() .properties .index_property_keys() - .filter(|idx| new_len <= **idx && **idx < u32::MAX) - .copied() + .filter(|idx| new_len <= *idx && *idx < u32::MAX) .collect(); keys.sort_unstable_by(|x, y| y.cmp(x)); keys diff --git a/boa_engine/src/object/internal_methods/global.rs b/boa_engine/src/object/internal_methods/global.rs index 69d07b44e5..89308f7a71 100644 --- a/boa_engine/src/object/internal_methods/global.rs +++ b/boa_engine/src/object/internal_methods/global.rs @@ -52,7 +52,7 @@ pub(crate) fn global_get_own_property( // 7. Set D.[[Enumerable]] to the value of X's [[Enumerable]] attribute. // 8. Set D.[[Configurable]] to the value of X's [[Configurable]] attribute. // 9. Return D. - Ok(context.realm.global_property_map.get(key).cloned()) + Ok(context.realm.global_property_map.get(key)) } /// Abstract operation `OrdinaryIsExtensible`. @@ -202,7 +202,7 @@ pub(crate) fn global_set_no_receiver( // https://tc39.es/ecma262/multipage/ordinary-and-exotic-objects-behaviours.html#sec-ordinarysetwithowndescriptor // 1. Assert: IsPropertyKey(P) is true. - let own_desc = if let Some(desc) = context.realm.global_property_map.get(key).cloned() { + let own_desc = if let Some(desc) = context.realm.global_property_map.get(key) { desc } // c. Else, @@ -250,7 +250,7 @@ pub(crate) fn global_set_no_receiver( }; // 1. Let current be ? O.[[GetOwnProperty]](P). - let current = context.realm.global_property_map.get(key).cloned(); + let current = context.realm.global_property_map.get(key); // 2. Let extensible be ? IsExtensible(O). let extensible = context.realm.global_extensible; diff --git a/boa_engine/src/object/internal_methods/mod.rs b/boa_engine/src/object/internal_methods/mod.rs index 7591378778..a8fc18514b 100644 --- a/boa_engine/src/object/internal_methods/mod.rs +++ b/boa_engine/src/object/internal_methods/mod.rs @@ -465,7 +465,7 @@ pub(crate) fn ordinary_get_own_property( // 7. Set D.[[Enumerable]] to the value of X's [[Enumerable]] attribute. // 8. Set D.[[Configurable]] to the value of X's [[Configurable]] attribute. // 9. Return D. - Ok(obj.borrow().properties.get(key).cloned()) + Ok(obj.borrow().properties.get(key)) } /// Abstract operation `OrdinaryDefineOwnProperty`. @@ -725,12 +725,7 @@ pub(crate) fn ordinary_own_property_keys( let mut keys = Vec::new(); let ordered_indexes = { - let mut indexes: Vec<_> = obj - .borrow() - .properties - .index_property_keys() - .copied() - .collect(); + let mut indexes: Vec<_> = obj.borrow().properties.index_property_keys().collect(); indexes.sort_unstable(); indexes }; diff --git a/boa_engine/src/object/internal_methods/string.rs b/boa_engine/src/object/internal_methods/string.rs index 453b90d586..2cf9af5ba8 100644 --- a/boa_engine/src/object/internal_methods/string.rs +++ b/boa_engine/src/object/internal_methods/string.rs @@ -112,7 +112,6 @@ pub(crate) fn string_exotic_own_property_keys( let mut remaining_indices: Vec<_> = obj .properties .index_property_keys() - .copied() .filter(|idx| (*idx as usize) >= len) .collect(); remaining_indices.sort_unstable(); diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 204644b74b..4c3dd10b98 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -1317,6 +1317,11 @@ impl Object { &self.properties } + #[inline] + pub(crate) fn properties_mut(&mut self) -> &mut PropertyMap { + &mut self.properties + } + /// Inserts a field in the object `properties` without checking if it's writable. /// /// If a field was already in the object with the same name, then a `Some` is returned diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index f661820ca9..c83eeb2146 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -1,5 +1,5 @@ use super::{PropertyDescriptor, PropertyKey}; -use crate::{JsString, JsSymbol}; +use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; use boa_gc::{custom_trace, Finalize, Trace}; use indexmap::IndexMap; use rustc_hash::{FxHashMap, FxHasher}; @@ -28,9 +28,202 @@ unsafe impl Trace for OrderedHashMap { }); } +/// This represents all the indexed properties. +/// +/// The index properties can be stored in two storage methods: +/// - `Dense` Storage +/// - `Sparse` Storage +/// +/// By default it is dense storage. +#[derive(Debug, Trace, Finalize)] +enum IndexedProperties { + /// Dense storage holds a contiguous array of properties where the index in the array is the key of the property. + /// These are known to be data descriptors with a value field, writable field set to `true`, configurable field set to `true`, enumerable field set to `true`. + /// + /// Since we know the properties of the property descriptors (and they are all the same) we can omit it and just store only + /// the value field and construct the data property descriptor on demand. + /// + /// This storage method is used by default. + Dense(Vec), + + /// Sparse storage this storage is used as a backup if the element keys are not continuous or the property descriptors + /// are not data descriptors with with a value field, writable field set to `true`, configurable field set to `true`, enumerable field set to `true`. + /// + /// This method uses more space, since we also have to store the property descriptors, not just the value. + /// It is also slower because we need to to a hash lookup. + Sparse(FxHashMap), +} + +impl Default for IndexedProperties { + #[inline] + fn default() -> Self { + Self::Dense(Vec::new()) + } +} + +impl IndexedProperties { + /// Get a property descriptor if it exists. + #[inline] + fn get(&self, key: u32) -> Option { + match self { + Self::Sparse(ref map) => map.get(&key).cloned(), + Self::Dense(ref vec) => vec.get(key as usize).map(|value| { + PropertyDescriptorBuilder::new() + .writable(true) + .enumerable(true) + .configurable(true) + .value(value.clone()) + .build() + }), + } + } + + /// Helper function for converting from a dense storage type to sparse storage type. + #[inline] + fn convert_dense_to_sparse(vec: &mut Vec) -> FxHashMap { + let data = std::mem::take(vec); + + data.into_iter() + .enumerate() + .map(|(index, value)| { + ( + index as u32, + PropertyDescriptorBuilder::new() + .writable(true) + .enumerable(true) + .configurable(true) + .value(value) + .build(), + ) + }) + .collect() + } + + /// Inserts a property descriptor with the specified key. + #[inline] + fn insert(&mut self, key: u32, property: PropertyDescriptor) -> Option { + let vec = match self { + Self::Sparse(map) => return map.insert(key, property), + Self::Dense(vec) => { + let len = vec.len() as u32; + if key <= len + && property.value().is_some() + && property.writable().unwrap_or(false) + && property.enumerable().unwrap_or(false) + && property.configurable().unwrap_or(false) + { + // Fast Path: continues array access. + + let mut value = property + .value() + .cloned() + .expect("already checked that the property descriptor has a value field"); + + // If the key is pointing one past the last element, we push it! + // + // Since the previous key is the current key - 1. Meaning that the elements are continuos. + if key == len { + vec.push(value); + return None; + } + + // If it the key points in at a already taken index, swap and return it. + std::mem::swap(&mut vec[key as usize], &mut value); + return Some( + PropertyDescriptorBuilder::new() + .writable(true) + .enumerable(true) + .configurable(true) + .value(value) + .build(), + ); + } + + vec + } + }; + + // Slow path: converting to sparse storage. + let mut map = Self::convert_dense_to_sparse(vec); + let old_property = map.insert(key, property); + *self = Self::Sparse(map); + + old_property + } + + /// Inserts a property descriptor with the specified key. + #[inline] + fn remove(&mut self, key: u32) -> Option { + let vec = match self { + Self::Sparse(map) => return map.remove(&key), + Self::Dense(vec) => { + // Fast Path: contiguous storage. + + // Has no elements or out of range, nothing to delete! + if vec.is_empty() || key as usize >= vec.len() { + return None; + } + + // If the key is pointing at the last element, then we pop it and return it. + // + // It does not make the storage sparse + if key as usize == vec.len().wrapping_sub(1) { + let value = vec.pop().expect("Already checked if it is out of bounds"); + return Some( + PropertyDescriptorBuilder::new() + .writable(true) + .enumerable(true) + .configurable(true) + .value(value) + .build(), + ); + } + + vec + } + }; + + // Slow Path: conversion to sparse storage. + let mut map = Self::convert_dense_to_sparse(vec); + let old_property = map.remove(&key); + *self = Self::Sparse(map); + + old_property + } + + /// Check if we contain the key to a property descriptor. + fn contains_key(&self, key: u32) -> bool { + match self { + Self::Sparse(map) => map.contains_key(&key), + Self::Dense(vec) => (0..vec.len() as u32).contains(&key), + } + } + + fn iter(&self) -> IndexProperties<'_> { + match self { + Self::Dense(vec) => IndexProperties::Dense(vec.iter().enumerate()), + Self::Sparse(map) => IndexProperties::Sparse(map.iter()), + } + } + + fn keys(&self) -> IndexPropertyKeys<'_> { + match self { + Self::Dense(vec) => IndexPropertyKeys::Dense(0..vec.len() as u32), + Self::Sparse(map) => IndexPropertyKeys::Sparse(map.keys()), + } + } + + fn values(&self) -> IndexPropertyValues<'_> { + match self { + Self::Dense(vec) => IndexPropertyValues::Dense(vec.iter()), + Self::Sparse(map) => IndexPropertyValues::Sparse(map.values()), + } + } +} + #[derive(Default, Debug, Trace, Finalize)] pub struct PropertyMap { - indexed_properties: FxHashMap, + indexed_properties: IndexedProperties, /// Properties string_properties: OrderedHashMap, /// Symbol Properties @@ -41,11 +234,12 @@ impl PropertyMap { pub fn new() -> Self { Self::default() } - pub fn get(&self, key: &PropertyKey) -> Option<&PropertyDescriptor> { + + pub fn get(&self, key: &PropertyKey) -> Option { match key { - PropertyKey::Index(index) => self.indexed_properties.get(index), - PropertyKey::String(string) => self.string_properties.0.get(string), - PropertyKey::Symbol(symbol) => self.symbol_properties.0.get(symbol), + PropertyKey::Index(index) => self.indexed_properties.get(*index), + PropertyKey::String(string) => self.string_properties.0.get(string).cloned(), + PropertyKey::Symbol(symbol) => self.symbol_properties.0.get(symbol).cloned(), } } @@ -67,12 +261,17 @@ impl PropertyMap { pub fn remove(&mut self, key: &PropertyKey) -> Option { match key { - PropertyKey::Index(index) => self.indexed_properties.remove(index), + PropertyKey::Index(index) => self.indexed_properties.remove(*index), PropertyKey::String(string) => self.string_properties.0.shift_remove(string), PropertyKey::Symbol(symbol) => self.symbol_properties.0.shift_remove(symbol), } } + /// Overrides all the indexed properties, setting it to dense storage. + pub(crate) fn override_indexed_properties(&mut self, properties: Vec) { + self.indexed_properties = IndexedProperties::Dense(properties); + } + /// An iterator visiting all key-value pairs in arbitrary order. The iterator element type is `(PropertyKey, &'a Property)`. /// /// This iterator does not recurse down the prototype chain. @@ -131,7 +330,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn index_properties(&self) -> IndexProperties<'_> { - IndexProperties(self.indexed_properties.iter()) + self.indexed_properties.iter() } /// An iterator visiting all index keys in arbitrary order. The iterator element type is `&'a u32`. @@ -139,7 +338,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn index_property_keys(&self) -> IndexPropertyKeys<'_> { - IndexPropertyKeys(self.indexed_properties.keys()) + self.indexed_properties.keys() } /// An iterator visiting all index values in arbitrary order. The iterator element type is `&'a Property`. @@ -147,7 +346,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn index_property_values(&self) -> IndexPropertyValues<'_> { - IndexPropertyValues(self.indexed_properties.values()) + self.indexed_properties.values() } /// An iterator visiting all string key-value pairs in arbitrary order. The iterator element type is `(&'a RcString, &'a Property)`. @@ -177,7 +376,7 @@ impl PropertyMap { #[inline] pub fn contains_key(&self, key: &PropertyKey) -> bool { match key { - PropertyKey::Index(index) => self.indexed_properties.contains_key(index), + PropertyKey::Index(index) => self.indexed_properties.contains_key(*index), PropertyKey::String(string) => self.string_properties.0.contains_key(string), PropertyKey::Symbol(symbol) => self.symbol_properties.0.contains_key(symbol), } @@ -197,21 +396,21 @@ impl PropertyMap { /// An iterator over the property entries of an `Object` #[derive(Debug, Clone)] pub struct Iter<'a> { - indexed_properties: hash_map::Iter<'a, u32, PropertyDescriptor>, + indexed_properties: IndexProperties<'a>, string_properties: indexmap::map::Iter<'a, JsString, PropertyDescriptor>, symbol_properties: indexmap::map::Iter<'a, JsSymbol, PropertyDescriptor>, } impl<'a> Iterator for Iter<'a> { - type Item = (PropertyKey, &'a PropertyDescriptor); + type Item = (PropertyKey, PropertyDescriptor); fn next(&mut self) -> Option { if let Some((key, value)) = self.indexed_properties.next() { - Some(((*key).into(), value)) + Some((key.into(), value)) } else if let Some((key, value)) = self.string_properties.next() { - Some((key.clone().into(), value)) + Some((key.clone().into(), value.clone())) } else { let (key, value) = self.symbol_properties.next()?; - Some((key.clone().into(), value)) + Some((key.clone().into(), value.clone())) } } } @@ -251,7 +450,7 @@ impl FusedIterator for Keys<'_> {} pub struct Values<'a>(Iter<'a>); impl<'a> Iterator for Values<'a> { - type Item = &'a PropertyDescriptor; + type Item = PropertyDescriptor; fn next(&mut self) -> Option { let (_, value) = self.0.next()?; Some(value) @@ -350,26 +549,48 @@ impl FusedIterator for SymbolPropertyValues<'_> {} /// An iterator over the indexed property entries of an `Object` #[derive(Debug, Clone)] -pub struct IndexProperties<'a>(hash_map::Iter<'a, u32, PropertyDescriptor>); +pub enum IndexProperties<'a> { + Dense(std::iter::Enumerate>), + Sparse(hash_map::Iter<'a, u32, PropertyDescriptor>), +} impl<'a> Iterator for IndexProperties<'a> { - type Item = (&'a u32, &'a PropertyDescriptor); + type Item = (u32, PropertyDescriptor); #[inline] fn next(&mut self) -> Option { - self.0.next() + match self { + Self::Dense(vec) => vec.next().map(|(index, value)| { + ( + index as u32, + PropertyDescriptorBuilder::new() + .writable(true) + .configurable(true) + .enumerable(true) + .value(value.clone()) + .build(), + ) + }), + Self::Sparse(map) => map.next().map(|(index, value)| (*index, value.clone())), + } } #[inline] fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() + match self { + Self::Dense(vec) => vec.size_hint(), + Self::Sparse(map) => map.size_hint(), + } } } impl ExactSizeIterator for IndexProperties<'_> { #[inline] fn len(&self) -> usize { - self.0.len() + match self { + Self::Dense(vec) => vec.len(), + Self::Sparse(map) => map.len(), + } } } @@ -377,26 +598,38 @@ impl FusedIterator for IndexProperties<'_> {} /// An iterator over the index keys (`u32`) of an `Object`. #[derive(Debug, Clone)] -pub struct IndexPropertyKeys<'a>(hash_map::Keys<'a, u32, PropertyDescriptor>); +pub enum IndexPropertyKeys<'a> { + Dense(std::ops::Range), + Sparse(hash_map::Keys<'a, u32, PropertyDescriptor>), +} impl<'a> Iterator for IndexPropertyKeys<'a> { - type Item = &'a u32; + type Item = u32; #[inline] fn next(&mut self) -> Option { - self.0.next() + match self { + Self::Dense(vec) => vec.next(), + Self::Sparse(map) => map.next().copied(), + } } #[inline] fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() + match self { + Self::Dense(vec) => vec.size_hint(), + Self::Sparse(map) => map.size_hint(), + } } } impl ExactSizeIterator for IndexPropertyKeys<'_> { #[inline] fn len(&self) -> usize { - self.0.len() + match self { + Self::Dense(vec) => vec.len(), + Self::Sparse(map) => map.len(), + } } } @@ -404,26 +637,45 @@ impl FusedIterator for IndexPropertyKeys<'_> {} /// An iterator over the index values (`Property`) of an `Object`. #[derive(Debug, Clone)] -pub struct IndexPropertyValues<'a>(hash_map::Values<'a, u32, PropertyDescriptor>); +pub enum IndexPropertyValues<'a> { + Dense(std::slice::Iter<'a, JsValue>), + Sparse(hash_map::Values<'a, u32, PropertyDescriptor>), +} impl<'a> Iterator for IndexPropertyValues<'a> { - type Item = &'a PropertyDescriptor; + type Item = PropertyDescriptor; #[inline] fn next(&mut self) -> Option { - self.0.next() + match self { + Self::Dense(vec) => vec.next().map(|value| { + PropertyDescriptorBuilder::new() + .writable(true) + .configurable(true) + .enumerable(true) + .value(value.clone()) + .build() + }), + Self::Sparse(map) => map.next().cloned(), + } } #[inline] fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() + match self { + Self::Dense(vec) => vec.size_hint(), + Self::Sparse(map) => map.size_hint(), + } } } impl ExactSizeIterator for IndexPropertyValues<'_> { #[inline] fn len(&self) -> usize { - self.0.len() + match self { + Self::Dense(vec) => vec.len(), + Self::Sparse(map) => map.len(), + } } } diff --git a/boa_engine/src/value/display.rs b/boa_engine/src/value/display.rs index 1cee752262..ccc4d6049d 100644 --- a/boa_engine/src/value/display.rs +++ b/boa_engine/src/value/display.rs @@ -139,9 +139,9 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children .borrow() .properties() .get(&i.into()) - .and_then(PropertyDescriptor::value) + .and_then(|x| x.value().cloned()) { - log_string_from(value, print_internals, false) + log_string_from(&value, print_internals, false) } else { String::from("") } diff --git a/boa_engine/src/value/mod.rs b/boa_engine/src/value/mod.rs index 9b2c900ecb..c69f4f528c 100644 --- a/boa_engine/src/value/mod.rs +++ b/boa_engine/src/value/mod.rs @@ -311,7 +311,7 @@ impl JsValue { match self { Self::Object(ref object) => { // TODO: had to skip `__get_own_properties__` since we don't have context here - let property = object.borrow().properties().get(&key).cloned(); + let property = object.borrow().properties().get(&key); if property.is_some() { return property; }