Dense/Packed JavaScript arrays (#2167)

This PR implements an optimization done by V8 and spidermonkey. Which stores indexed properties in two class storage methods dense and sparse. The dense method stores the property in a contiguous array ( `Vec<T>` ) where the index is the property key. This storage method is the default. While on the other hand we have sparse storage method which stores them in a hash map with key `u32` (like we currently do). This storage method is a backup and is slower to access because we have to do a hash map lookup, instead of an array access.

In the dense array we can store only data descriptors that have a value field and are `writable`, `configurable` and `enumerable` (which by default array elements are). Since all the fields of the property descriptor are the same except value field, we can omit them an just store the `JsValue`s in `Vec<JsValue>` this decreases the memory consumption and because it is smaller we are less likely to have cache misses.

There are cases where we have to convert from dense to sparse (the slow case):
- If we insert index elements in a non-incremental way, like `array[1000] = 1000` (inserting an element to an index that is already occupied only replaces it does not make it sparse)
- If we delete from the middle of the array (making a hole), like `delete array[10]`  (only converts if there is actualy an element there, so calling delete on a non-existent index property will do nothing)

Once it becomes sparse is *stays* sparse there is no way to convert it again. (the computation needed to check whether it can be converted outweighs the benefits of this optimization)

I did some local benchmarks and on array creation/pop and push there is ~45% speed up and the impact _should_ be bigger the more elements we have. For example the code below on `main` takes `~21.5s` while with this optimization is `~3.5s` (both on release build).

```js
let array = [];
for (let i = 0; i < 50000; ++i) {
   array[i] = i;
}
```

In addition I also made `Array::create_array_from_list` do a direct setting of the properties (small deviation from spec but it should have the same behaviour), with this #2058 should be fixed, conversion from `Vec` to `JsArray`, not `JsTypedArray` for that I will work on next :)

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.
-        }
+        //
+        // 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
     }

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)
+            .filter(|idx| new_len <= *idx && *idx < u32::MAX)
-            .copied()
             .collect();
         keys.sort_unstable_by(|x, y| y.cmp(x));
         keys

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;

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
+        let mut indexes: Vec<_> = obj.borrow().properties.index_property_keys().collect();
-            .borrow()
-            .properties
-            .index_property_keys()
-            .copied()
-            .collect();
         indexes.sort_unstable();
         indexes
     };

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();

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

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<K: Trace> Trace for OrderedHashMap<K> {
     });
 }
 
+/// 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<JsValue>),
+
+    /// 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<u32, PropertyDescriptor>),
+}
+
+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<PropertyDescriptor> {
+        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<JsValue>) -> FxHashMap<u32, PropertyDescriptor> {
+        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<PropertyDescriptor> {
+        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<PropertyDescriptor> {
+        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<u32, PropertyDescriptor>,
+    indexed_properties: IndexedProperties,
     /// Properties
     string_properties: OrderedHashMap<JsString>,
     /// 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<PropertyDescriptor> {
         match key {
-            PropertyKey::Index(index) => self.indexed_properties.get(index),
+            PropertyKey::Index(index) => self.indexed_properties.get(*index),
-            PropertyKey::String(string) => self.string_properties.0.get(string),
+            PropertyKey::String(string) => self.string_properties.0.get(string).cloned(),
-            PropertyKey::Symbol(symbol) => self.symbol_properties.0.get(symbol),
+            PropertyKey::Symbol(symbol) => self.symbol_properties.0.get(symbol).cloned(),
         }
     }
 
@@ -67,12 +261,17 @@ impl PropertyMap {
 
     pub fn remove(&mut self, key: &PropertyKey) -> Option<PropertyDescriptor> {
         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<JsValue>) {
+        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<Self::Item> {
         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<Self::Item> {
         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<std::slice::Iter<'a, JsValue>>),
+    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::Item> {
-        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<usize>) {
-        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<u32>),
+    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::Item> {
-        self.0.next()
+        match self {
+            Self::Dense(vec) => vec.next(),
+            Self::Sparse(map) => map.next().copied(),
+        }
     }
 
     #[inline]
     fn size_hint(&self) -> (usize, Option<usize>) {
-        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::Item> {
-        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<usize>) {
-        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(),
+        }
     }
 }
 

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("<empty>")
                                 }

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;
                 }