diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index fafaa9ae23a..cff54430ef1 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -1,5 +1,5 @@ use super::{ - shape::{Shape, Slot, TransitionKey}, + shape::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey}, PropertyDescriptor, PropertyKey, }; use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; @@ -229,11 +229,10 @@ pub struct PropertyMap { indexed_properties: IndexedProperties, pub(crate) shape: Shape, - storage: Vec, - - // TODO: remove these, since they will be replaced by the shape - string_properties: OrderedHashMap, - symbol_properties: OrderedHashMap, + // FIXME: currently every property takes 2 spaces in the array + // to account for the accessor (get, set), add different sized + // storage based on attribute flags. + storage: Vec, } impl PropertyMap { @@ -243,8 +242,6 @@ impl PropertyMap { pub fn new(shape: Shape) -> Self { Self { indexed_properties: IndexedProperties::default(), - string_properties: OrderedHashMap::default(), - symbol_properties: OrderedHashMap::default(), shape, storage: Vec::default(), } @@ -257,7 +254,7 @@ impl PropertyMap { return self.indexed_properties.get(*index); } if let Some(slot) = self.shape.lookup(key) { - return Some(self.storage[slot.index as usize].clone()); + return Some(self.get_storage(slot)); } None @@ -265,11 +262,30 @@ impl PropertyMap { /// Get the property with the given key from the [`PropertyMap`]. #[must_use] - pub fn get_storage(&self, slot: Slot) -> PropertyDescriptor { - self.storage[slot.index as usize].clone() + pub fn get_storage(&self, Slot { index, attributes }: Slot) -> PropertyDescriptor { + let index = index as usize; + let mut builder = PropertyDescriptor::builder() + .configurable(attributes.contains(PropertyDescriptorAttribute::CONFIGURABLE)) + .enumerable(attributes.contains(PropertyDescriptorAttribute::ENUMERABLE)); + if attributes.contains(PropertyDescriptorAttribute::HAS_GET) + || attributes.contains(PropertyDescriptorAttribute::HAS_SET) + { + if attributes.contains(PropertyDescriptorAttribute::HAS_GET) { + builder = builder.get(self.storage[index].clone()); + } + if attributes.contains(PropertyDescriptorAttribute::HAS_SET) { + builder = builder.set(self.storage[index + 1].clone()); + } + } else { + builder = builder.writable(attributes.contains(PropertyDescriptorAttribute::WRITABLE)); + builder = builder.value(self.storage[index].clone()); + } + builder.build() } /// Insert the given property descriptor with the given key [`PropertyMap`]. + // FIXME: Temporary lint allow, remove after prototyping + #[allow(clippy::missing_panics_doc)] pub fn insert( &mut self, key: &PropertyKey, @@ -278,19 +294,103 @@ impl PropertyMap { if let PropertyKey::Index(index) = key { return self.indexed_properties.insert(*index, property); } + + // TODO: maybe extract into a PropertyDescriptor method? + let mut attributes = PropertyDescriptorAttribute::empty(); + attributes.set( + PropertyDescriptorAttribute::CONFIGURABLE, + property.expect_configurable(), + ); + attributes.set( + PropertyDescriptorAttribute::ENUMERABLE, + property.expect_enumerable(), + ); + if property.is_data_descriptor() { + attributes.set( + PropertyDescriptorAttribute::WRITABLE, + property.expect_writable(), + ); + } else if property.is_accessor_descriptor() { + attributes.set( + PropertyDescriptorAttribute::HAS_GET, + property.get().is_some(), + ); + attributes.set( + PropertyDescriptorAttribute::HAS_SET, + property.set().is_some(), + ); + } else { + unreachable!() + } + + // println!("Attributes: {key} IN {attributes:?}"); + if let Some(slot) = self.shape.lookup(key) { - return Some(std::mem::replace( - &mut self.storage[slot.index as usize], - property, - )); + let index = slot.index as usize; + + // TODO: figure out if the return value of self.insert is ever used, + // if not remove it, seems unneded. + let old = self.get_storage(slot); + if slot.attributes != attributes { + let key = TransitionKey { + property_key: key.clone(), + attributes, + }; + self.shape = self.shape.change_attributes_transition(key); + } + + // TODO: maybe implement is_accessor/data_descriptor on attributes? + if attributes.contains(PropertyDescriptorAttribute::HAS_GET) + || attributes.contains(PropertyDescriptorAttribute::HAS_SET) + { + if attributes.contains(PropertyDescriptorAttribute::HAS_GET) { + self.storage[index] = property + .get() + .cloned() + .map(JsValue::new) + .unwrap_or_default(); + } + if attributes.contains(PropertyDescriptorAttribute::HAS_SET) { + self.storage[index + 1] = property + .set() + .cloned() + .map(JsValue::new) + .unwrap_or_default(); + } + } else { + // TODO: maybe take the value instead of cloning, we own it. + self.storage[index] = property.expect_value().clone(); + } + return Some(old); } let transition_key = TransitionKey { property_key: key.clone(), - // attributes: Attribute::all(), + attributes, }; self.shape = self.shape.insert_property_transition(transition_key); - self.storage.push(property); + if attributes.contains(PropertyDescriptorAttribute::HAS_GET) + || attributes.contains(PropertyDescriptorAttribute::HAS_SET) + { + self.storage.push( + property + .get() + .cloned() + .map(JsValue::new) + .unwrap_or_default(), + ); + self.storage.push( + property + .set() + .cloned() + .map(JsValue::new) + .unwrap_or_default(), + ); + } else { + self.storage + .push(property.value().cloned().unwrap_or_default()); + self.storage.push(JsValue::undefined()); + } None } @@ -301,15 +401,14 @@ impl PropertyMap { return self.indexed_properties.remove(*index); } if let Some(slot) = self.shape.lookup(key) { - let result = self.storage.remove(slot.index as usize); - - let transition_key = TransitionKey { - property_key: key.clone(), - // attributes: Attribute::all(), - }; - self.shape = self.shape.remove_property_transition(&transition_key); - - return Some(result); + // TOOD: figure out if the removed value is ever used otherwise make it return nothing. + let old = self.get_storage(slot); + // shift all elements + self.storage.remove(slot.index as usize + 1); + self.storage.remove(slot.index as usize); + + self.shape = self.shape.remove_property_transition(key); + return Some(old); } None diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs index c70860a51c7..a2c5fba5741 100644 --- a/boa_engine/src/object/shape/mod.rs +++ b/boa_engine/src/object/shape/mod.rs @@ -19,6 +19,7 @@ use std::{ rc::Rc, }; +use bitflags::bitflags; use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc}; use rustc_hash::FxHashMap; @@ -31,10 +32,21 @@ use self::{shared_shape::SharedShape, unique_shape::UniqueShape}; use super::JsPrototype; +bitflags! { + #[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Hash)] + pub struct PropertyDescriptorAttribute: u8 { + const WRITABLE = 0b0000_0001; + const ENUMERABLE = 0b0000_0010; + const CONFIGURABLE = 0b0000_0100; + const HAS_GET = 0b0000_1000; + const HAS_SET = 0b0001_0000; + } +} + #[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] pub(crate) struct TransitionKey { pub(crate) property_key: PropertyKey, - // attributes: Attribute, + pub(crate) attributes: PropertyDescriptorAttribute, } // SAFETY: non of the member of this struct are garbage collected, @@ -47,10 +59,8 @@ unsafe impl Trace for TransitionKey { enum TransitionType { /// Inserts a new property. Insert, - // Change existing property attributes - // TODO: - // Configure, - + // Change existing property attributes. + Configure, // TODO: // Prototype, } @@ -68,7 +78,7 @@ enum Inner { #[derive(Debug, Clone, Copy)] pub struct Slot { pub index: u32, - // pub attribute: Attribute, + pub attributes: PropertyDescriptorAttribute, } #[derive(Debug, Trace, Finalize, Clone)] @@ -108,7 +118,14 @@ impl Shape { } } - pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self { + pub(crate) fn change_attributes_transition(&self, key: TransitionKey) -> Self { + match &self.inner { + Inner::Shared(shape) => Self::shared(shape.change_attributes_transition(key)), + Inner::Unique(shape) => Self::unique(shape.change_attributes_transition(key)), + } + } + + pub(crate) fn remove_property_transition(&self, key: &PropertyKey) -> Self { match &self.inner { Inner::Shared(shape) => Self::shared(shape.remove_property_transition(key)), Inner::Unique(shape) => Self::unique(shape.remove_property_transition(key)), diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index ed4064e5e00..5e1a5abf32c 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -3,16 +3,16 @@ use std::cell::{Cell, RefCell}; use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; use rustc_hash::FxHashMap; -use crate::property::PropertyKey; +use crate::property::{Attribute, PropertyDescriptor, PropertyKey}; -use super::{Shape, Slot, TransitionKey, TransitionType}; +use super::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey, TransitionType}; /// TODO: doc #[derive(Debug, Trace, Finalize)] struct Inner { /// Cached property map, only constucted when a property is accesed through it. #[unsafe_ignore_trace] - property_table: RefCell>>, + property_table: RefCell>>, #[unsafe_ignore_trace] property_access: Cell, @@ -24,6 +24,9 @@ struct Inner { property_key: PropertyKey, property_index: u32, + #[unsafe_ignore_trace] + property_attributes: PropertyDescriptorAttribute, + // TODO: add prototype to shape transition_type: TransitionType, } @@ -75,6 +78,7 @@ impl SharedShape { forward_property_transitions: GcRefCell::new(FxHashMap::default()), property_key: PropertyKey::Index(0), property_index: 0, + property_attributes: PropertyDescriptorAttribute::default(), previous: None, transition_type: TransitionType::Insert, }) @@ -91,7 +95,7 @@ impl SharedShape { let next_property_index = if self.previous().is_none() { self.property_index() } else { - self.property_index() + 1 + self.property_index() + 2 }; let new_inner_shape = Inner { property_table: RefCell::new(None), @@ -99,6 +103,7 @@ impl SharedShape { forward_property_transitions: GcRefCell::new(FxHashMap::default()), property_index: next_property_index, property_key: key.property_key.clone(), + property_attributes: key.attributes, previous: Some(self.clone()), transition_type: TransitionType::Insert, }; @@ -135,7 +140,40 @@ impl SharedShape { self.create_property_transition(key) } - // Rollback to the transiotion before the propery was inserted. + pub(crate) fn change_attributes_transition(&self, key: TransitionKey) -> Self { + if Self::DEBUG { + println!( + "Shape: Change attributes transition: {} -------> {:?}", + self.property_key(), + key.attributes, + ); + } + let mut next_property_index = if self.previous().is_none() { + self.property_index() + } else { + self.property_index() + 2 + }; + let new_inner_shape = Inner { + property_table: RefCell::new(None), + property_access: Cell::new(0), + forward_property_transitions: GcRefCell::new(FxHashMap::default()), + property_index: self.property_index(), + property_key: key.property_key.clone(), + property_attributes: key.attributes, + previous: Some(self.clone()), + transition_type: TransitionType::Configure, + }; + let new_shape = Self::new(new_inner_shape); + + { + let mut map = self.inner.forward_property_transitions.borrow_mut(); + map.insert(key, WeakGc::new(&new_shape.inner)); + } + + new_shape + } + + // Rollback to the transition before the propery was inserted. // // For example with the following chain: // @@ -158,9 +196,9 @@ impl SharedShape { // | INSERT(z) // \----------------> { x, z } <----- The shape we return :) // - pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self { + pub(crate) fn remove_property_transition(&self, key: &PropertyKey) -> Self { if Self::DEBUG { - println!("Shape: deleting {}", key.property_key); + println!("Shape: deleting {key}"); } let mut transitions = Vec::default(); let mut p = Some(self); @@ -170,9 +208,7 @@ impl SharedShape { unreachable!("The chain should have insert transition type!") }; - if shape.transition_type() == TransitionType::Insert - && shape.property_key() == &key.property_key - { + if shape.transition_type() == TransitionType::Insert && shape.property_key() == key { let mut base = if let Some(base) = shape.previous() { base.clone() } else { @@ -184,7 +220,7 @@ impl SharedShape { transitions.push(TransitionKey { property_key: shape.property_key().clone(), - // attributes: shape.property_attribute(), + attributes: shape.inner.property_attributes, }); p = shape.previous(); @@ -209,8 +245,8 @@ impl SharedShape { if Self::DEBUG { println!(" Shape: Using cached property table"); } - if let Some(index) = property_table.get(key).copied() { - return Some(Slot { index }); + if let Some((index, attributes)) = property_table.get(key).copied() { + return Some(Slot { index, attributes }); } return None; @@ -238,6 +274,7 @@ impl SharedShape { } return Some(Slot { index: shape.property_index(), + attributes: shape.inner.property_attributes, }); } p = shape.previous(); @@ -260,7 +297,7 @@ impl SharedShape { if shape.is_root() { break; } - if shape.property_key() == key { + if result.is_none() && shape.property_key() == key { if Self::DEBUG { println!( " Shape: found {} with {} property index", @@ -270,9 +307,16 @@ impl SharedShape { } result = Some(Slot { index: shape.property_index(), + attributes: shape.inner.property_attributes, }); } - property_table.insert(shape.property_key().clone(), shape.property_index()); + + // If we already added a property we don't add the previous versions + // which can happens if a configure property transition occurs. + property_table + .entry(shape.property_key().clone()) + .or_insert((shape.property_index(), shape.inner.property_attributes)); + p = shape.previous(); } diff --git a/boa_engine/src/object/shape/unique_shape.rs b/boa_engine/src/object/shape/unique_shape.rs index 243e987d92a..adce2ebaa73 100644 --- a/boa_engine/src/object/shape/unique_shape.rs +++ b/boa_engine/src/object/shape/unique_shape.rs @@ -17,12 +17,12 @@ use crate::{ JsString, }; -use super::{JsPrototype, Slot, TransitionKey}; +use super::{JsPrototype, PropertyDescriptorAttribute, Slot, TransitionKey}; /// TODO: doc #[derive(Default, Debug)] struct Inner { - property_table: RefCell>, + property_table: RefCell>, } /// TODO: doc @@ -33,7 +33,7 @@ pub(crate) struct UniqueShape { impl UniqueShape { /// Create shape from a pre initialized property table. - fn new(property_table: IndexSet) -> Self { + fn new(property_table: IndexMap) -> Self { Self { inner: Rc::new(Inner { property_table: property_table.into(), @@ -45,9 +45,9 @@ impl UniqueShape { pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { { let mut property_table = self.inner.property_table.borrow_mut(); - let (index, inserted) = property_table.insert_full(key.property_key); + let (index, inserted) = property_table.insert_full(key.property_key, key.attributes); - debug_assert!(inserted); + debug_assert!(inserted.is_none()); debug_assert!(index + 1 == property_table.len()); } @@ -55,9 +55,9 @@ impl UniqueShape { } /// TODO: doc - pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self { + pub(crate) fn remove_property_transition(&self, key: &PropertyKey) -> Self { let mut property_table = self.inner.property_table.borrow_mut(); - let Some((index, _property_key)) = property_table.shift_remove_full(&key.property_key) else { + let Some((index, _property_key, _attributes)) = property_table.shift_remove_full(key) else { return self.clone(); }; @@ -69,7 +69,7 @@ impl UniqueShape { // The property that was deleted was not the last property added. // Therefore we need to create a new unique shape, - // to invalidateany pointers to this shape i.e inline caches. + // to invalidate any pointers to this shape i.e inline caches. let property_table = std::mem::take(&mut *property_table); Self::new(property_table) } @@ -78,28 +78,41 @@ impl UniqueShape { pub(crate) fn lookup(&self, key: &PropertyKey) -> Option { // println!("Unique: lookup {key}"); let mut property_table = self.inner.property_table.borrow(); - if let Some(index) = property_table.get_index_of(key) { + if let Some((index, _property_key, attributes)) = property_table.get_full(key) { return Some(Slot { - index: index as u32, + index: index as u32 * 2, + attributes: *attributes, }); } None } + pub(crate) fn change_attributes_transition(&self, key: TransitionKey) -> Self { + { + let mut property_table = self.inner.property_table.borrow_mut(); + let Some(attributes) = property_table.get_mut(&key.property_key) else { + unreachable!("Attribute change can only happen on existing property") + }; + + *attributes = key.attributes; + } + self.clone() + } + /// Gets all keys first strings then symbols. pub(crate) fn keys(&self) -> Vec { let mut property_table = self.inner.property_table.borrow(); let mut keys = Vec::with_capacity(property_table.len()); for key in property_table - .iter() + .keys() .filter(|x| matches!(x, PropertyKey::String(_))) { keys.push(key.clone()); } for key in property_table - .iter() + .keys() .filter(|x| matches!(x, PropertyKey::Symbol(_))) { keys.push(key.clone());