From 0bbee619473c95f7d6fa7b3ab382aa60c87c9ec9 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Wed, 22 Mar 2023 01:11:58 +0100 Subject: [PATCH 01/19] Basic Implementation of object shapes --- boa_engine/src/context/mod.rs | 5 +- boa_engine/src/object/jsobject.rs | 9 +- boa_engine/src/object/mod.rs | 1 + boa_engine/src/object/property_map.rs | 54 ++++- boa_engine/src/object/shape.rs | 266 +++++++++++++++++++++++ boa_engine/src/property/attribute/mod.rs | 2 +- boa_engine/src/vm/opcode/push/object.rs | 1 + 7 files changed, 332 insertions(+), 6 deletions(-) create mode 100644 boa_engine/src/object/shape.rs diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index bdaff918fee..8b9308cb825 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -22,7 +22,7 @@ use crate::{ class::{Class, ClassBuilder}, job::{IdleJobQueue, JobQueue, NativeJob}, native_function::NativeFunction, - object::{FunctionObjectBuilder, GlobalPropertyMap, JsObject}, + object::{FunctionObjectBuilder, GlobalPropertyMap, JsObject, shape::Shape}, property::{Attribute, PropertyDescriptor, PropertyKey}, realm::Realm, vm::{CallFrame, CodeBlock, Vm}, @@ -101,6 +101,8 @@ pub struct Context<'host> { host_hooks: &'host dyn HostHooks, job_queue: &'host dyn JobQueue, + + pub(crate) root_shape: Shape, } impl std::fmt::Debug for Context<'_> { @@ -643,6 +645,7 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> { kept_alive: Vec::new(), host_hooks, job_queue: self.job_queue.unwrap_or(&IdleJobQueue), + root_shape: Shape::root(), }; builtins::set_default_global_bindings(&mut context)?; diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index f1dd101062a..8b21b7f6636 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -2,7 +2,7 @@ //! //! The `JsObject` is a garbage collected Object. -use super::{JsPrototype, NativeObject, Object, PropertyMap}; +use super::{JsPrototype, NativeObject, Object, PropertyMap, shape::Shape}; use crate::{ error::JsNativeError, object::{ObjectData, ObjectKind}, @@ -33,6 +33,13 @@ pub struct JsObject { } impl JsObject { + + /// TODO: doc + pub(crate) fn set_shape(&self, shape: Shape) { + let mut object = self.inner.borrow_mut(); + object.properties_mut().shape = Some(shape); + } + /// Creates a new ordinary object with its prototype set to the `Object` prototype. /// /// This is equivalent to calling the specification's abstract operation diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 86bfef4b7c3..e4f26a4b49d 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -71,6 +71,7 @@ pub mod builtins; mod jsobject; mod operations; mod property_map; +pub(crate) mod shape; pub(crate) use builtins::*; diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index 67b2ec08de1..83a880f70f0 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -1,4 +1,4 @@ -use super::{PropertyDescriptor, PropertyKey}; +use super::{PropertyDescriptor, PropertyKey, shape::{Shape, TransitionKey}}; use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; use boa_gc::{custom_trace, Finalize, Trace}; use indexmap::IndexMap; @@ -230,19 +230,36 @@ pub struct PropertyMap { /// Properties stored with `Symbol`s a keys. symbol_properties: OrderedHashMap, + + pub(crate) shape: Option, + storage: Vec, } impl PropertyMap { /// Create a new [`PropertyMap`]. #[must_use] #[inline] - pub fn new() -> Self { - Self::default() + pub fn new(shape: Shape) -> Self { + Self { + indexed_properties: IndexedProperties::default(), + string_properties: OrderedHashMap::default(), + symbol_properties: OrderedHashMap::default(), + shape: Some(shape), + storage: Vec::default(), + } } /// Get the property with the given key from the [`PropertyMap`]. #[must_use] pub fn get(&self, key: &PropertyKey) -> Option { + if let Some(shape) = &self.shape { + // println!("Get!!!"); + if let Some(slot) = shape.lookup(key) { + return Some(self.storage[slot.index as usize].clone()); + } + + return None; + } match key { PropertyKey::Index(index) => self.indexed_properties.get(*index), PropertyKey::String(string) => self.string_properties.0.get(string).cloned(), @@ -256,6 +273,21 @@ impl PropertyMap { key: &PropertyKey, property: PropertyDescriptor, ) -> Option { + if let Some(shape) = &mut self.shape { + // println!("Insert!!!"); + if let Some(slot) = shape.lookup(key) { + return Some(std::mem::replace(&mut self.storage[slot.index as usize], property)); + } + + let transition_key = TransitionKey { + property_key: key.clone(), + // attributes: Attribute::all(), + }; + *shape = shape.insert_property_transition(transition_key); + self.storage.push(property); + + return None; + } match &key { PropertyKey::Index(index) => self.indexed_properties.insert(*index, property), PropertyKey::String(string) => { @@ -269,6 +301,22 @@ impl PropertyMap { /// Remove the property with the given key from the [`PropertyMap`]. pub fn remove(&mut self, key: &PropertyKey) -> Option { + if let Some(shape) = &mut self.shape { + // println!("Remove!!!"); + if let Some(slot) = shape.lookup(key) { + let result = self.storage.remove(slot.index as usize); + + let transition_key = TransitionKey { + property_key: key.clone(), + // attributes: Attribute::all(), + }; + *shape = shape.delete_property_transition(&transition_key); + + return Some(result); + } + + return None; + } match key { PropertyKey::Index(index) => self.indexed_properties.remove(*index), PropertyKey::String(string) => self.string_properties.0.shift_remove(string), diff --git a/boa_engine/src/object/shape.rs b/boa_engine/src/object/shape.rs new file mode 100644 index 00000000000..e6fb9ebd69e --- /dev/null +++ b/boa_engine/src/object/shape.rs @@ -0,0 +1,266 @@ +// TODO: remove, after prototyping +#![allow(dead_code)] +#![allow(unreachable_code)] +#![allow(unused_imports)] +#![allow(clippy::let_and_return)] +#![allow(clippy::needless_pass_by_value)] +#![allow(unused)] + + +//! TODO: doc + +use std::collections::VecDeque; + +use boa_gc::{Trace, Finalize, Gc, empty_trace, GcRefCell}; +use rustc_hash::FxHashMap; + +use crate::{JsString, property::{Attribute, PropertyKey}}; + +use super::JsPrototype; + + + +#[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] +pub(crate) struct TransitionKey { + pub(crate) property_key: PropertyKey, + // attributes: Attribute, +} + +// SAFETY: non of the member of this struct are garbage collected, +// so this should be fine. +unsafe impl Trace for TransitionKey { + empty_trace!(); +} + +#[derive(Debug, Finalize, Clone, Copy, PartialEq, Eq)] +enum TransitionType { + /// Inserts a new property. + Insert, + + /// Change existing property attributes + // TODO: + Configure, + + // TODO: + // Prototype, +} + +unsafe impl Trace for TransitionType { + empty_trace!(); +} + +/// TODO: doc +#[derive(Debug, Trace, Finalize, Clone)] +struct Inner { + // TODO: probably should be weak ref gc pointer. + forward_property_transitions: GcRefCell>, + // forward_prototype_transitions: FxHashMap, + previous: Option, + + #[unsafe_ignore_trace] + property_key: PropertyKey, + // #[unsafe_ignore_trace] + // property_attribute: Attribute, + + property_index: u32, + + // TODO: add prototype to shape + // prototype: JsPrototype, + + transition_type: TransitionType, +} + +#[derive(Debug, Clone, Copy)] +pub struct Slot { + pub index: u32, + // pub attribute: Attribute, +} + +#[derive(Debug, Trace, Finalize, Clone)] +pub struct Shape { + inner: Gc, +} + +impl Default for Shape { + fn default() -> Self { + Shape::root() + } +} + +// helper +impl Shape { + #[inline] + fn new(inner: Inner) -> Self { + Self { + inner: Gc::new(inner), + } + } + + #[inline] + pub(crate) fn previous(&self) -> Option<&Shape> { + self.inner.previous.as_ref() + } + #[inline] + pub(crate) fn property_key(&self) -> &PropertyKey { + &self.inner.property_key + } + #[inline] + pub(crate) fn property_index(&self) -> u32 { + self.inner.property_index + } + // #[inline] + // pub(crate) fn property_attribute(&self) -> Attribute { + // self.inner.property_attribute + // } + fn transition_type(&self) -> TransitionType { + self.inner.transition_type + } + // #[inline] + // pub(crate) fn prototype(&self) -> &JsPrototype { + // &self.inner.prototype + // } + #[inline] + pub(crate) fn is_root(&self) -> bool { + self.inner.previous.is_none() + } + #[inline] + pub(crate) fn get_forward_transition(&self, key: &TransitionKey) -> Option { + let map = self.inner.forward_property_transitions.borrow(); + map.get(key).cloned() + } +} + +impl Shape { + #[inline] + pub(crate) fn root() -> Self { + Self::new(Inner { + forward_property_transitions: GcRefCell::new(FxHashMap::default()), + // forward_prototype_transitions: FxHashMap::default(), + property_key: PropertyKey::Index(0), + // property_attribute: Attribute::default(), + property_index: 0, + // prototype: None, + // The root object has previous set to None. + previous: None, + transition_type: TransitionType::Configure, + }) + } + + fn create_property_transition(&self, key: TransitionKey) -> Self { + println!("Shape: Create Transition: {} -------> {}", self.property_key(), key.property_key); + let next_property_index = if self.previous().is_none() {self.property_index()} else { self.property_index() + 1 }; + let new_inner_shape = Inner { + forward_property_transitions: GcRefCell::new(FxHashMap::default()), + // forward_prototype_transitions: FxHashMap::default(), + property_index: next_property_index, + property_key: key.property_key.clone(), + // property_attribute: key.attributes, + // prototype: self.prototype().clone(), + previous: Some(self.clone()), + transition_type: TransitionType::Insert, + }; + // println!("Shape: {} -----> {}", self.property_key(), new_inner_shape.previous); + let new_shape = Self::new(new_inner_shape); + + { + let mut map = self.inner.forward_property_transitions.borrow_mut(); + map.insert(key, new_shape.clone()); + } + + new_shape + } + + pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { + // Check if we have already creaded such a transition, if so use it! + if let Some(shape) = self.get_forward_transition(&key) { + println!("Shape: Resuse previous shape"); + return shape; + } + + println!("Shape: Creating new shape"); + self.create_property_transition(key) + } + + + // Rollback to the transiotion before the propery was inserted. + // + // For example with the following chain: + // + // INSERT(x) INSERT(y) INSERT(z) + // { } ------------> { x } ------------> { x, y } ------------> { x, y, z } + // + // Then we call delete on `y`. We rollback to before the property was added and we put + // the transitions in a array for reconstruction of the new branch: + // + // INSERT(x) INSERT(y) INSERT(z) + // { } ------------> { x } ------------> { x, y } ------------> { x, y, z } + // ^ + // \--- base ( with array of transitions to be performed: INSERT(z) ) + // + // Then we apply transitions (z): + // + // INSERT(x) INSERT(y) INSERT(z) + // { } ------------> { x } ------------> { x, y } ------------> { x, y, z } + // | + // | INSERT(z) + // \----------------> { x, z } <<<----- The shape we return :) + // + pub(crate) fn delete_property_transition(&self, key: &TransitionKey) -> Self { + println!("Shape: deleting {}", key.property_key); + let mut transitions = Vec::default(); + let mut p = Some(self); + + let mut base = loop { + let Some(shape) = p else { + unreachable!("The chain should have insert transition type!") + }; + + if shape.transition_type() == TransitionType::Insert + && shape.property_key() == &key.property_key + { + let mut base = if let Some(base) = shape.previous() { + base.clone() + } else { + // It's the root, because it doesn't have previous. + shape.clone() + }; + break base; + } + + transitions.push(TransitionKey { + property_key: shape.property_key().clone(), + // attributes: shape.property_attribute(), + }); + + p = shape.previous(); + }; + + for transition in transitions.into_iter().rev() { + base = base.insert_property_transition(transition); + } + + base + } + + #[inline] + pub fn lookup(&self, key: &PropertyKey) -> Option { + println!("Shape: lookup {key}"); + let mut p = Some(self); + while let Some(shape) = p { + println!(" Shape: sub-lookup transition key {}", shape.property_key()); + // Root has no properties + if shape.is_root() { + return None; + } + if shape.property_key() == key { + println!(" Shape: found {} with {} property index", shape.property_key(), shape.property_index()); + return Some(Slot { + index: shape.property_index(), + // attribute: shape.property_attribute(), + }); + } + p = shape.previous(); + } + None + } +} diff --git a/boa_engine/src/property/attribute/mod.rs b/boa_engine/src/property/attribute/mod.rs index ac816d172a6..fe67effba1f 100644 --- a/boa_engine/src/property/attribute/mod.rs +++ b/boa_engine/src/property/attribute/mod.rs @@ -15,7 +15,7 @@ bitflags! { /// - `[[Configurable]]` (`CONFIGURABLE`) - If `false`, attempts to delete the property, /// change the property to be an `accessor property`, or change its attributes (other than `[[Value]]`, /// or changing `[[Writable]]` to `false`) will fail. - #[derive(Debug, Clone, Copy)] + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Attribute: u8 { /// The `Writable` attribute decides whether the value associated with the property can be changed or not, from its initial value. const WRITABLE = 0b0000_0001; diff --git a/boa_engine/src/vm/opcode/push/object.rs b/boa_engine/src/vm/opcode/push/object.rs index 9ec4b2bda03..7a81c96e986 100644 --- a/boa_engine/src/vm/opcode/push/object.rs +++ b/boa_engine/src/vm/opcode/push/object.rs @@ -17,6 +17,7 @@ impl Operation for PushEmptyObject { fn execute(context: &mut Context<'_>) -> JsResult { let o = JsObject::with_object_proto(context); + o.set_shape(context.root_shape.clone()); context.vm.push(o); Ok(CompletionType::Normal) } From b5c7d044464d7548173eedc1b363db5e7917297c Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Wed, 22 Mar 2023 01:48:34 +0100 Subject: [PATCH 02/19] Make forward reference weak gc --- boa_engine/src/object/shape.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/boa_engine/src/object/shape.rs b/boa_engine/src/object/shape.rs index e6fb9ebd69e..f5be6bb4eb0 100644 --- a/boa_engine/src/object/shape.rs +++ b/boa_engine/src/object/shape.rs @@ -11,7 +11,7 @@ use std::collections::VecDeque; -use boa_gc::{Trace, Finalize, Gc, empty_trace, GcRefCell}; +use boa_gc::{Trace, Finalize, Gc, empty_trace, GcRefCell, WeakGc}; use rustc_hash::FxHashMap; use crate::{JsString, property::{Attribute, PropertyKey}}; @@ -52,8 +52,7 @@ unsafe impl Trace for TransitionType { /// TODO: doc #[derive(Debug, Trace, Finalize, Clone)] struct Inner { - // TODO: probably should be weak ref gc pointer. - forward_property_transitions: GcRefCell>, + forward_property_transitions: GcRefCell>>, // forward_prototype_transitions: FxHashMap, previous: Option, @@ -124,7 +123,7 @@ impl Shape { self.inner.previous.is_none() } #[inline] - pub(crate) fn get_forward_transition(&self, key: &TransitionKey) -> Option { + fn get_forward_transition(&self, key: &TransitionKey) -> Option> { let map = self.inner.forward_property_transitions.borrow(); map.get(key).cloned() } @@ -164,7 +163,7 @@ impl Shape { { let mut map = self.inner.forward_property_transitions.borrow_mut(); - map.insert(key, new_shape.clone()); + map.insert(key, WeakGc::new(&new_shape.inner)); } new_shape @@ -173,8 +172,14 @@ impl Shape { pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { // Check if we have already creaded such a transition, if so use it! if let Some(shape) = self.get_forward_transition(&key) { - println!("Shape: Resuse previous shape"); - return shape; + println!("Shape: Trying to resuse previous shape"); + if let Some(inner) = shape.upgrade() { + // println!(" Shape: Resusing previous shape"); + return Self { inner }; + } + + println!(" Shape: Recreating shape because it got GC collected"); + return self.create_property_transition(key); } println!("Shape: Creating new shape"); @@ -202,8 +207,8 @@ impl Shape { // INSERT(x) INSERT(y) INSERT(z) // { } ------------> { x } ------------> { x, y } ------------> { x, y, z } // | - // | INSERT(z) - // \----------------> { x, z } <<<----- The shape we return :) + // | INSERT(z) + // \----------------> { x, z } <----- The shape we return :) // pub(crate) fn delete_property_transition(&self, key: &TransitionKey) -> Self { println!("Shape: deleting {}", key.property_key); @@ -247,7 +252,6 @@ impl Shape { println!("Shape: lookup {key}"); let mut p = Some(self); while let Some(shape) = p { - println!(" Shape: sub-lookup transition key {}", shape.property_key()); // Root has no properties if shape.is_root() { return None; From 35ff74fda05c8a5fac4d182ea0896174cfe81781 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Wed, 22 Mar 2023 02:42:45 +0100 Subject: [PATCH 03/19] Add shape property table cache --- boa_engine/src/context/mod.rs | 2 +- boa_engine/src/object/jsobject.rs | 3 +- boa_engine/src/object/property_map.rs | 18 ++++-- boa_engine/src/object/shape.rs | 81 ++++++++++++++++++++------- boa_engine/src/property/mod.rs | 2 +- 5 files changed, 77 insertions(+), 29 deletions(-) diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index 8b9308cb825..e98fe9b7063 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -22,7 +22,7 @@ use crate::{ class::{Class, ClassBuilder}, job::{IdleJobQueue, JobQueue, NativeJob}, native_function::NativeFunction, - object::{FunctionObjectBuilder, GlobalPropertyMap, JsObject, shape::Shape}, + object::{shape::Shape, FunctionObjectBuilder, GlobalPropertyMap, JsObject}, property::{Attribute, PropertyDescriptor, PropertyKey}, realm::Realm, vm::{CallFrame, CodeBlock, Vm}, diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index 8b21b7f6636..0e7a7e651bd 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -2,7 +2,7 @@ //! //! The `JsObject` is a garbage collected Object. -use super::{JsPrototype, NativeObject, Object, PropertyMap, shape::Shape}; +use super::{shape::Shape, JsPrototype, NativeObject, Object, PropertyMap}; use crate::{ error::JsNativeError, object::{ObjectData, ObjectKind}, @@ -33,7 +33,6 @@ pub struct JsObject { } impl JsObject { - /// TODO: doc pub(crate) fn set_shape(&self, shape: Shape) { let mut object = self.inner.borrow_mut(); diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index 83a880f70f0..7c1ab4a1e42 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -1,4 +1,7 @@ -use super::{PropertyDescriptor, PropertyKey, shape::{Shape, TransitionKey}}; +use super::{ + shape::{Shape, TransitionKey}, + PropertyDescriptor, PropertyKey, +}; use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; use boa_gc::{custom_trace, Finalize, Trace}; use indexmap::IndexMap; @@ -276,9 +279,12 @@ impl PropertyMap { if let Some(shape) = &mut self.shape { // println!("Insert!!!"); if let Some(slot) = shape.lookup(key) { - return Some(std::mem::replace(&mut self.storage[slot.index as usize], property)); + return Some(std::mem::replace( + &mut self.storage[slot.index as usize], + property, + )); } - + let transition_key = TransitionKey { property_key: key.clone(), // attributes: Attribute::all(), @@ -305,16 +311,16 @@ impl PropertyMap { // println!("Remove!!!"); if let Some(slot) = shape.lookup(key) { let result = self.storage.remove(slot.index as usize); - + let transition_key = TransitionKey { property_key: key.clone(), // attributes: Attribute::all(), }; *shape = shape.delete_property_transition(&transition_key); - + return Some(result); } - + return None; } match key { diff --git a/boa_engine/src/object/shape.rs b/boa_engine/src/object/shape.rs index f5be6bb4eb0..bdb1cc62af9 100644 --- a/boa_engine/src/object/shape.rs +++ b/boa_engine/src/object/shape.rs @@ -6,20 +6,20 @@ #![allow(clippy::needless_pass_by_value)] #![allow(unused)] - //! TODO: doc -use std::collections::VecDeque; +use std::{cell::RefCell, collections::VecDeque}; -use boa_gc::{Trace, Finalize, Gc, empty_trace, GcRefCell, WeakGc}; +use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc}; use rustc_hash::FxHashMap; -use crate::{JsString, property::{Attribute, PropertyKey}}; +use crate::{ + property::{Attribute, PropertyKey}, + JsString, +}; use super::JsPrototype; - - #[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] pub(crate) struct TransitionKey { pub(crate) property_key: PropertyKey, @@ -40,7 +40,6 @@ enum TransitionType { /// Change existing property attributes // TODO: Configure, - // TODO: // Prototype, } @@ -52,6 +51,10 @@ unsafe impl Trace for TransitionType { /// TODO: doc #[derive(Debug, Trace, Finalize, Clone)] struct Inner { + /// Cached property map, only constucted when a property is accesed through it. + #[unsafe_ignore_trace] + property_table: RefCell>>, + forward_property_transitions: GcRefCell>>, // forward_prototype_transitions: FxHashMap, previous: Option, @@ -60,12 +63,10 @@ struct Inner { property_key: PropertyKey, // #[unsafe_ignore_trace] // property_attribute: Attribute, - property_index: u32, // TODO: add prototype to shape // prototype: JsPrototype, - transition_type: TransitionType, } @@ -133,6 +134,7 @@ impl Shape { #[inline] pub(crate) fn root() -> Self { Self::new(Inner { + property_table: RefCell::new(None), forward_property_transitions: GcRefCell::new(FxHashMap::default()), // forward_prototype_transitions: FxHashMap::default(), property_key: PropertyKey::Index(0), @@ -146,9 +148,18 @@ impl Shape { } fn create_property_transition(&self, key: TransitionKey) -> Self { - println!("Shape: Create Transition: {} -------> {}", self.property_key(), key.property_key); - let next_property_index = if self.previous().is_none() {self.property_index()} else { self.property_index() + 1 }; + println!( + "Shape: Create Transition: {} -------> {}", + self.property_key(), + key.property_key + ); + let next_property_index = if self.previous().is_none() { + self.property_index() + } else { + self.property_index() + 1 + }; let new_inner_shape = Inner { + property_table: RefCell::new(None), forward_property_transitions: GcRefCell::new(FxHashMap::default()), // forward_prototype_transitions: FxHashMap::default(), property_index: next_property_index, @@ -177,7 +188,7 @@ impl Shape { // println!(" Shape: Resusing previous shape"); return Self { inner }; } - + println!(" Shape: Recreating shape because it got GC collected"); return self.create_property_transition(key); } @@ -186,7 +197,6 @@ impl Shape { self.create_property_transition(key) } - // Rollback to the transiotion before the propery was inserted. // // For example with the following chain: @@ -223,7 +233,7 @@ impl Shape { if shape.transition_type() == TransitionType::Insert && shape.property_key() == &key.property_key { - let mut base = if let Some(base) = shape.previous() { + let mut base = if let Some(base) = shape.previous() { base.clone() } else { // It's the root, because it doesn't have previous. @@ -250,21 +260,54 @@ impl Shape { #[inline] pub fn lookup(&self, key: &PropertyKey) -> Option { println!("Shape: lookup {key}"); + + // Use cheched property table, if exists + { + let property_table = self.inner.property_table.borrow(); + if let Some(property_table) = property_table.as_ref() { + println!(" Shape: Using cached property table"); + + if let Some(index) = property_table.get(key).copied() { + return Some(Slot { index }); + } + + return None; + } + } + + println!(" Shape: Creating cached property table"); + + let mut property_table = FxHashMap::default(); + + // Creating cached property table + let mut result = None; let mut p = Some(self); while let Some(shape) = p { // Root has no properties if shape.is_root() { - return None; + break; } if shape.property_key() == key { - println!(" Shape: found {} with {} property index", shape.property_key(), shape.property_index()); - return Some(Slot { + println!( + " Shape: found {} with {} property index", + shape.property_key(), + shape.property_index() + ); + result = Some(Slot { index: shape.property_index(), - // attribute: shape.property_attribute(), }); } + property_table.insert(shape.property_key().clone(), shape.property_index()); p = shape.previous(); } - None + + // Trim excess space + property_table.shrink_to_fit(); + + // Put into cheched + let mut property_table_borrow = self.inner.property_table.borrow_mut(); + property_table_borrow.insert(property_table); + + result } } diff --git a/boa_engine/src/property/mod.rs b/boa_engine/src/property/mod.rs index fd639b6857b..cb748e0b490 100644 --- a/boa_engine/src/property/mod.rs +++ b/boa_engine/src/property/mod.rs @@ -557,7 +557,7 @@ impl From for PropertyDescriptor { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-ispropertykey -#[derive(PartialEq, Debug, Clone, Eq, Hash)] +#[derive(Finalize, PartialEq, Debug, Clone, Eq, Hash)] pub enum PropertyKey { /// A string property key. String(JsString), From ce47b18c8d93b050f647911e4d91ae08495472ab Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Wed, 22 Mar 2023 18:37:07 +0100 Subject: [PATCH 04/19] Implement unique shape --- boa_engine/src/context/mod.rs | 11 +- boa_engine/src/environments/compile.rs | 14 +- boa_engine/src/object/jsobject.rs | 2 +- boa_engine/src/object/property_map.rs | 126 +++++------ boa_engine/src/object/shape/mod.rs | 125 +++++++++++ .../{shape.rs => shape/shared_shape.rs} | 205 ++++++++---------- boa_engine/src/object/shape/unique_shape.rs | 61 ++++++ boa_engine/src/realm.rs | 6 +- boa_engine/src/vm/opcode/define/mod.rs | 23 +- boa_engine/src/vm/opcode/get/name.rs | 9 +- boa_engine/src/vm/opcode/set/name.rs | 6 +- 11 files changed, 368 insertions(+), 220 deletions(-) create mode 100644 boa_engine/src/object/shape/mod.rs rename boa_engine/src/object/{shape.rs => shape/shared_shape.rs} (66%) create mode 100644 boa_engine/src/object/shape/unique_shape.rs diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index e98fe9b7063..d2649eb3363 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -22,7 +22,7 @@ use crate::{ class::{Class, ClassBuilder}, job::{IdleJobQueue, JobQueue, NativeJob}, native_function::NativeFunction, - object::{shape::Shape, FunctionObjectBuilder, GlobalPropertyMap, JsObject}, + object::{shape::Shape, FunctionObjectBuilder, JsObject, PropertyMap}, property::{Attribute, PropertyDescriptor, PropertyKey}, realm::Realm, vm::{CallFrame, CodeBlock, Vm}, @@ -325,7 +325,7 @@ impl Context<'_> { .build(); self.global_bindings_mut().insert( - name.into(), + &PropertyKey::String(name.into()), PropertyDescriptor::builder() .value(function) .writable(true) @@ -357,7 +357,7 @@ impl Context<'_> { .build(); self.global_bindings_mut().insert( - name.into(), + &PropertyKey::String(name.into()), PropertyDescriptor::builder() .value(function) .writable(true) @@ -395,7 +395,8 @@ impl Context<'_> { .configurable(T::ATTRIBUTES.configurable()) .build(); - self.global_bindings_mut().insert(T::NAME.into(), property); + self.global_bindings_mut() + .insert(&PropertyKey::String(T::NAME.into()), property); Ok(()) } @@ -462,7 +463,7 @@ impl Context<'_> { impl Context<'_> { /// Return a mutable reference to the global object string bindings. - pub(crate) fn global_bindings_mut(&mut self) -> &mut GlobalPropertyMap { + pub(crate) fn global_bindings_mut(&mut self) -> &mut PropertyMap { self.realm.global_bindings_mut() } diff --git a/boa_engine/src/environments/compile.rs b/boa_engine/src/environments/compile.rs index 5b7725295b8..cdb81d00162 100644 --- a/boa_engine/src/environments/compile.rs +++ b/boa_engine/src/environments/compile.rs @@ -1,5 +1,7 @@ use crate::{ - environments::runtime::BindingLocator, property::PropertyDescriptor, Context, JsString, JsValue, + environments::runtime::BindingLocator, + property::{PropertyDescriptor, PropertyKey}, + Context, JsString, JsValue, }; use boa_ast::expression::Identifier; use boa_gc::{Finalize, Gc, GcRefCell, Trace}; @@ -289,14 +291,12 @@ impl Context<'_> { .interner() .resolve_expect(name.sym()) .into_common::(false); - let desc = self - .realm - .global_property_map - .string_property_map() - .get(&name_str); + + let key = PropertyKey::String(name_str); + let desc = self.realm.global_property_map.get(&key); if desc.is_none() { self.global_bindings_mut().insert( - name_str, + &key, PropertyDescriptor::builder() .value(JsValue::Undefined) .writable(true) diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index 0e7a7e651bd..3cb5dbd4df4 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -36,7 +36,7 @@ impl JsObject { /// TODO: doc pub(crate) fn set_shape(&self, shape: Shape) { let mut object = self.inner.borrow_mut(); - object.properties_mut().shape = Some(shape); + object.properties_mut().shape = shape; } /// Creates a new ordinary object with its prototype set to the `Object` prototype. diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index 7c1ab4a1e42..f34f8fbfa76 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -8,9 +8,9 @@ use indexmap::IndexMap; use rustc_hash::{FxHashMap, FxHasher}; use std::{collections::hash_map, hash::BuildHasherDefault, iter::FusedIterator}; -/// Type alias to make it easier to work with the string properties on the global object. -pub(crate) type GlobalPropertyMap = - IndexMap>; +// /// Type alias to make it easier to work with the string properties on the global object. +// pub(crate) type GlobalPropertyMap = +// IndexMap>; /// Wrapper around `indexmap::IndexMap` for usage in `PropertyMap`. #[derive(Debug, Finalize)] @@ -228,14 +228,12 @@ pub struct PropertyMap { /// Properties stored with integers as keys. indexed_properties: IndexedProperties, - /// Properties stored with `String`s a keys. - string_properties: OrderedHashMap, + pub(crate) shape: Shape, + storage: Vec, - /// Properties stored with `Symbol`s a keys. + // TODO: remove these, since they will be replaced by the shape + string_properties: OrderedHashMap, symbol_properties: OrderedHashMap, - - pub(crate) shape: Option, - storage: Vec, } impl PropertyMap { @@ -247,7 +245,7 @@ impl PropertyMap { indexed_properties: IndexedProperties::default(), string_properties: OrderedHashMap::default(), symbol_properties: OrderedHashMap::default(), - shape: Some(shape), + shape, storage: Vec::default(), } } @@ -255,19 +253,14 @@ impl PropertyMap { /// Get the property with the given key from the [`PropertyMap`]. #[must_use] pub fn get(&self, key: &PropertyKey) -> Option { - if let Some(shape) = &self.shape { - // println!("Get!!!"); - if let Some(slot) = shape.lookup(key) { - return Some(self.storage[slot.index as usize].clone()); - } - - return None; + if let PropertyKey::Index(index) = key { + return self.indexed_properties.get(*index); } - match key { - 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(), + if let Some(slot) = self.shape.lookup(key) { + return Some(self.storage[slot.index as usize].clone()); } + + None } /// Insert the given property descriptor with the given key [`PropertyMap`]. @@ -276,58 +269,44 @@ impl PropertyMap { key: &PropertyKey, property: PropertyDescriptor, ) -> Option { - if let Some(shape) = &mut self.shape { - // println!("Insert!!!"); - if let Some(slot) = shape.lookup(key) { - return Some(std::mem::replace( - &mut self.storage[slot.index as usize], - property, - )); - } - - let transition_key = TransitionKey { - property_key: key.clone(), - // attributes: Attribute::all(), - }; - *shape = shape.insert_property_transition(transition_key); - self.storage.push(property); - - return None; + if let PropertyKey::Index(index) = key { + return self.indexed_properties.insert(*index, property); } - match &key { - PropertyKey::Index(index) => self.indexed_properties.insert(*index, property), - PropertyKey::String(string) => { - self.string_properties.0.insert(string.clone(), property) - } - PropertyKey::Symbol(symbol) => { - self.symbol_properties.0.insert(symbol.clone(), property) - } + if let Some(slot) = self.shape.lookup(key) { + return Some(std::mem::replace( + &mut self.storage[slot.index as usize], + property, + )); } + + let transition_key = TransitionKey { + property_key: key.clone(), + // attributes: Attribute::all(), + }; + self.shape = self.shape.insert_property_transition(transition_key); + self.storage.push(property); + + None } /// Remove the property with the given key from the [`PropertyMap`]. pub fn remove(&mut self, key: &PropertyKey) -> Option { - if let Some(shape) = &mut self.shape { - // println!("Remove!!!"); - if let Some(slot) = shape.lookup(key) { - let result = self.storage.remove(slot.index as usize); - - let transition_key = TransitionKey { - property_key: key.clone(), - // attributes: Attribute::all(), - }; - *shape = shape.delete_property_transition(&transition_key); - - return Some(result); - } - - return None; + if let PropertyKey::Index(index) = key { + return self.indexed_properties.remove(*index); } - match key { - 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), + 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); } + + None } /// Overrides all the indexed properties, setting it to dense storage. @@ -461,19 +440,14 @@ impl PropertyMap { #[inline] #[must_use] pub fn contains_key(&self, key: &PropertyKey) -> bool { - match key { - 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), + if let PropertyKey::Index(index) = key { + return self.indexed_properties.contains_key(*index); + } + if self.shape.lookup(key).is_some() { + return true; } - } - - pub(crate) const fn string_property_map(&self) -> &GlobalPropertyMap { - &self.string_properties.0 - } - pub(crate) fn string_property_map_mut(&mut self) -> &mut GlobalPropertyMap { - &mut self.string_properties.0 + false } } diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs new file mode 100644 index 00000000000..9c02cb21f28 --- /dev/null +++ b/boa_engine/src/object/shape/mod.rs @@ -0,0 +1,125 @@ +// TODO: remove, after prototyping +#![allow(dead_code)] +#![allow(unreachable_code)] +#![allow(unused_imports)] +#![allow(clippy::let_and_return)] +#![allow(clippy::needless_pass_by_value)] +#![allow(unused)] + +//! TODO: doc + +mod shared_shape; +mod unique_shape; + +use std::{ + any::Any, + cell::{Cell, RefCell}, + collections::VecDeque, + fmt::Debug, + rc::Rc, +}; + +use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc}; +use rustc_hash::FxHashMap; + +use crate::{ + property::{Attribute, PropertyKey}, + JsString, +}; + +use self::{shared_shape::SharedShape, unique_shape::UniqueShape}; + +use super::JsPrototype; + +#[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] +pub(crate) struct TransitionKey { + pub(crate) property_key: PropertyKey, + // attributes: Attribute, +} + +// SAFETY: non of the member of this struct are garbage collected, +// so this should be fine. +unsafe impl Trace for TransitionKey { + empty_trace!(); +} + +#[derive(Debug, Finalize, Clone, Copy, PartialEq, Eq)] +enum TransitionType { + /// Inserts a new property. + Insert, + // Change existing property attributes + // TODO: + // Configure, + + // TODO: + // Prototype, +} + +unsafe impl Trace for TransitionType { + empty_trace!(); +} + +#[derive(Debug, Trace, Finalize, Clone)] +enum Inner { + Unique(#[unsafe_ignore_trace] UniqueShape), + Shared(SharedShape), +} + +#[derive(Debug, Clone, Copy)] +pub struct Slot { + pub index: u32, + // pub attribute: Attribute, +} + +#[derive(Debug, Trace, Finalize, Clone)] +pub struct Shape { + inner: Inner, +} + +impl Default for Shape { + fn default() -> Self { + Shape::unique(UniqueShape::default()) + } +} + +impl Shape { + pub(crate) fn root() -> Self { + Self::shared(SharedShape::root()) + } + + #[inline] + fn shared(inner: SharedShape) -> Self { + Self { + inner: Inner::Shared(inner), + } + } + + #[inline] + pub(crate) const fn unique(shape: UniqueShape) -> Self { + Self { + inner: Inner::Unique(shape), + } + } + + pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { + match &self.inner { + Inner::Shared(shape) => Self::shared(shape.insert_property_transition(key)), + Inner::Unique(shape) => Self::unique(shape.insert_property_transition(key)), + } + } + + pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self { + match &self.inner { + Inner::Shared(shape) => Self::shared(shape.remove_property_transition(key)), + Inner::Unique(shape) => Self::unique(shape.remove_property_transition(key)), + } + } + + #[inline] + pub fn lookup(&self, key: &PropertyKey) -> Option { + match &self.inner { + Inner::Shared(shape) => shape.lookup(key), + Inner::Unique(shape) => shape.lookup(key), + } + } +} diff --git a/boa_engine/src/object/shape.rs b/boa_engine/src/object/shape/shared_shape.rs similarity index 66% rename from boa_engine/src/object/shape.rs rename to boa_engine/src/object/shape/shared_shape.rs index bdb1cc62af9..b379848c7cb 100644 --- a/boa_engine/src/object/shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -1,103 +1,43 @@ -// TODO: remove, after prototyping -#![allow(dead_code)] -#![allow(unreachable_code)] -#![allow(unused_imports)] -#![allow(clippy::let_and_return)] -#![allow(clippy::needless_pass_by_value)] -#![allow(unused)] +use std::cell::{Cell, RefCell}; -//! TODO: doc - -use std::{cell::RefCell, collections::VecDeque}; - -use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc}; +use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; use rustc_hash::FxHashMap; -use crate::{ - property::{Attribute, PropertyKey}, - JsString, -}; - -use super::JsPrototype; - -#[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] -pub(crate) struct TransitionKey { - pub(crate) property_key: PropertyKey, - // attributes: Attribute, -} - -// SAFETY: non of the member of this struct are garbage collected, -// so this should be fine. -unsafe impl Trace for TransitionKey { - empty_trace!(); -} +use crate::property::PropertyKey; -#[derive(Debug, Finalize, Clone, Copy, PartialEq, Eq)] -enum TransitionType { - /// Inserts a new property. - Insert, - - /// Change existing property attributes - // TODO: - Configure, - // TODO: - // Prototype, -} - -unsafe impl Trace for TransitionType { - empty_trace!(); -} +use super::{Shape, Slot, TransitionKey, TransitionType}; /// TODO: doc -#[derive(Debug, Trace, Finalize, Clone)] +#[derive(Debug, Trace, Finalize)] struct Inner { /// Cached property map, only constucted when a property is accesed through it. #[unsafe_ignore_trace] property_table: RefCell>>, + #[unsafe_ignore_trace] + property_access: Cell, + forward_property_transitions: GcRefCell>>, - // forward_prototype_transitions: FxHashMap, - previous: Option, + previous: Option, #[unsafe_ignore_trace] property_key: PropertyKey, - // #[unsafe_ignore_trace] - // property_attribute: Attribute, property_index: u32, // TODO: add prototype to shape - // prototype: JsPrototype, transition_type: TransitionType, } -#[derive(Debug, Clone, Copy)] -pub struct Slot { - pub index: u32, - // pub attribute: Attribute, -} - #[derive(Debug, Trace, Finalize, Clone)] -pub struct Shape { +pub(crate) struct SharedShape { inner: Gc, } -impl Default for Shape { - fn default() -> Self { - Shape::root() - } -} +impl SharedShape { + const DEBUG: bool = false; -// helper -impl Shape { #[inline] - fn new(inner: Inner) -> Self { - Self { - inner: Gc::new(inner), - } - } - - #[inline] - pub(crate) fn previous(&self) -> Option<&Shape> { + pub(crate) fn previous(&self) -> Option<&SharedShape> { self.inner.previous.as_ref() } #[inline] @@ -108,17 +48,9 @@ impl Shape { pub(crate) fn property_index(&self) -> u32 { self.inner.property_index } - // #[inline] - // pub(crate) fn property_attribute(&self) -> Attribute { - // self.inner.property_attribute - // } fn transition_type(&self) -> TransitionType { self.inner.transition_type } - // #[inline] - // pub(crate) fn prototype(&self) -> &JsPrototype { - // &self.inner.prototype - // } #[inline] pub(crate) fn is_root(&self) -> bool { self.inner.previous.is_none() @@ -128,31 +60,34 @@ impl Shape { let map = self.inner.forward_property_transitions.borrow(); map.get(key).cloned() } -} -impl Shape { + fn new(inner: Inner) -> Self { + Self { + inner: Gc::new(inner), + } + } + #[inline] pub(crate) fn root() -> Self { Self::new(Inner { property_table: RefCell::new(None), + property_access: Cell::new(0), forward_property_transitions: GcRefCell::new(FxHashMap::default()), - // forward_prototype_transitions: FxHashMap::default(), property_key: PropertyKey::Index(0), - // property_attribute: Attribute::default(), property_index: 0, - // prototype: None, - // The root object has previous set to None. previous: None, - transition_type: TransitionType::Configure, + transition_type: TransitionType::Insert, }) } fn create_property_transition(&self, key: TransitionKey) -> Self { - println!( - "Shape: Create Transition: {} -------> {}", - self.property_key(), - key.property_key - ); + if Self::DEBUG { + println!( + "Shape: Create Transition: {} -------> {}", + self.property_key(), + key.property_key + ); + } let next_property_index = if self.previous().is_none() { self.property_index() } else { @@ -160,16 +95,13 @@ impl Shape { }; let new_inner_shape = Inner { property_table: RefCell::new(None), + property_access: Cell::new(0), forward_property_transitions: GcRefCell::new(FxHashMap::default()), - // forward_prototype_transitions: FxHashMap::default(), property_index: next_property_index, property_key: key.property_key.clone(), - // property_attribute: key.attributes, - // prototype: self.prototype().clone(), previous: Some(self.clone()), transition_type: TransitionType::Insert, }; - // println!("Shape: {} -----> {}", self.property_key(), new_inner_shape.previous); let new_shape = Self::new(new_inner_shape); { @@ -183,17 +115,23 @@ impl Shape { pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { // Check if we have already creaded such a transition, if so use it! if let Some(shape) = self.get_forward_transition(&key) { - println!("Shape: Trying to resuse previous shape"); + if Self::DEBUG { + println!("Shape: Trying to resuse previous shape"); + } if let Some(inner) = shape.upgrade() { // println!(" Shape: Resusing previous shape"); return Self { inner }; } - println!(" Shape: Recreating shape because it got GC collected"); + if Self::DEBUG { + println!(" Shape: Recreating shape because it got GC collected"); + } return self.create_property_transition(key); } - println!("Shape: Creating new shape"); + if Self::DEBUG { + println!("Shape: Creating new shape"); + } self.create_property_transition(key) } @@ -220,8 +158,10 @@ impl Shape { // | INSERT(z) // \----------------> { x, z } <----- The shape we return :) // - pub(crate) fn delete_property_transition(&self, key: &TransitionKey) -> Self { - println!("Shape: deleting {}", key.property_key); + pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self { + if Self::DEBUG { + println!("Shape: deleting {}", key.property_key); + } let mut transitions = Vec::default(); let mut p = Some(self); @@ -258,15 +198,17 @@ impl Shape { } #[inline] - pub fn lookup(&self, key: &PropertyKey) -> Option { - println!("Shape: lookup {key}"); - + pub(crate) fn lookup(&self, key: &PropertyKey) -> Option { + if Self::DEBUG { + println!("Shape: lookup {key}"); + } // Use cheched property table, if exists - { + if self.inner.property_access.get() == 4 { let property_table = self.inner.property_table.borrow(); if let Some(property_table) = property_table.as_ref() { - println!(" Shape: Using cached property table"); - + if Self::DEBUG { + println!(" Shape: Using cached property table"); + } if let Some(index) = property_table.get(key).copied() { return Some(Slot { index }); } @@ -275,7 +217,38 @@ impl Shape { } } - println!(" Shape: Creating cached property table"); + if self.inner.property_access.get() < 4 { + self.inner + .property_access + .set(self.inner.property_access.get() + 1); + + let mut p = Some(self); + while let Some(shape) = p { + // Root has no properties + if shape.is_root() { + break; + } + if shape.property_key() == key { + if Self::DEBUG { + println!( + " Shape: found {} with {} property index", + shape.property_key(), + shape.property_index() + ); + } + return Some(Slot { + index: shape.property_index(), + }); + } + p = shape.previous(); + } + + return None; + } + + if Self::DEBUG { + println!(" Shape: Creating cached property table"); + } let mut property_table = FxHashMap::default(); @@ -288,11 +261,13 @@ impl Shape { break; } if shape.property_key() == key { - println!( - " Shape: found {} with {} property index", - shape.property_key(), - shape.property_index() - ); + if Self::DEBUG { + println!( + " Shape: found {} with {} property index", + shape.property_key(), + shape.property_index() + ); + } result = Some(Slot { index: shape.property_index(), }); @@ -304,7 +279,7 @@ impl Shape { // Trim excess space property_table.shrink_to_fit(); - // Put into cheched + // Put into shape let mut property_table_borrow = self.inner.property_table.borrow_mut(); property_table_borrow.insert(property_table); diff --git a/boa_engine/src/object/shape/unique_shape.rs b/boa_engine/src/object/shape/unique_shape.rs new file mode 100644 index 00000000000..0c56d80bb3e --- /dev/null +++ b/boa_engine/src/object/shape/unique_shape.rs @@ -0,0 +1,61 @@ +use std::{ + any::Any, + cell::{Cell, RefCell}, + collections::VecDeque, + fmt::Debug, + rc::Rc, +}; + +use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc}; +use rustc_hash::FxHashMap; + +use crate::{ + property::{Attribute, PropertyKey}, + JsString, +}; + +use super::{JsPrototype, Slot, TransitionKey}; + +#[derive(Default, Debug, Clone)] +pub(crate) struct UniqueShape { + property_table: Rc>>, + // /// Properties stored with `String`s a keys. + // string_properties: OrderedHashMap, + + // /// Properties stored with `Symbol`s a keys. + // symbol_properties: OrderedHashMap, +} + +impl UniqueShape { + pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { + { + // println!("Unique: insert {}", key.property_key); + let mut property_table = self.property_table.borrow_mut(); + let offset = property_table.len() as u32; + property_table.insert(key.property_key, offset); + } + + self.clone() + } + + pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self { + { + let mut property_table = self.property_table.borrow_mut(); + property_table.remove(&key.property_key); + } + + // self.clone() + todo!("implement deletion!") + } + + #[inline] + pub(crate) fn lookup(&self, key: &PropertyKey) -> Option { + // println!("Unique: lookup {key}"); + let mut property_table = self.property_table.borrow(); + if let Some(index) = property_table.get(key) { + return Some(Slot { index: *index }); + } + + None + } +} diff --git a/boa_engine/src/realm.rs b/boa_engine/src/realm.rs index e76c323cbf9..960cfbe8dd9 100644 --- a/boa_engine/src/realm.rs +++ b/boa_engine/src/realm.rs @@ -9,7 +9,7 @@ use crate::{ context::{intrinsics::Intrinsics, HostHooks}, environments::{CompileTimeEnvironment, DeclarativeEnvironmentStack}, - object::{GlobalPropertyMap, JsObject, PropertyMap}, + object::{JsObject, PropertyMap}, }; use boa_gc::{Gc, GcRefCell}; use boa_profiler::Profiler; @@ -62,8 +62,8 @@ impl Realm { &self.global_this } - pub(crate) fn global_bindings_mut(&mut self) -> &mut GlobalPropertyMap { - self.global_property_map.string_property_map_mut() + pub(crate) fn global_bindings_mut(&mut self) -> &mut PropertyMap { + &mut self.global_property_map } /// Set the number of bindings on the global environment. diff --git a/boa_engine/src/vm/opcode/define/mod.rs b/boa_engine/src/vm/opcode/define/mod.rs index 30adcab0665..dc6561df4ab 100644 --- a/boa_engine/src/vm/opcode/define/mod.rs +++ b/boa_engine/src/vm/opcode/define/mod.rs @@ -1,5 +1,5 @@ use crate::{ - property::PropertyDescriptor, + property::{PropertyDescriptor, PropertyKey}, vm::{opcode::Operation, CompletionType}, Context, JsResult, JsString, JsValue, }; @@ -30,14 +30,19 @@ impl Operation for DefVar { .interner() .resolve_expect(binding_locator.name().sym()) .into_common(false); - context.global_bindings_mut().entry(key).or_insert( - PropertyDescriptor::builder() - .value(JsValue::Undefined) - .writable(true) - .enumerable(true) - .configurable(true) - .build(), - ); + + let key = PropertyKey::String(key); + if context.global_bindings_mut().get(&key).is_none() { + context.global_bindings_mut().insert( + &key, + PropertyDescriptor::builder() + .value(JsValue::Undefined) + .writable(true) + .enumerable(true) + .configurable(true) + .build(), + ); + }; } else { context.realm.environments.put_value_if_uninitialized( binding_locator.environment_index(), diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index 7a128716bb2..6e5c078da7b 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -1,6 +1,6 @@ use crate::{ error::JsNativeError, - property::DescriptorKind, + property::{DescriptorKind, PropertyKey}, vm::{opcode::Operation, CompletionType}, Context, JsResult, JsString, JsValue, }; @@ -29,7 +29,10 @@ impl Operation for GetName { .interner() .resolve_expect(binding_locator.name().sym()) .into_common(false); - match context.global_bindings_mut().get(&key) { + match context + .global_bindings_mut() + .get(&PropertyKey::String(key.clone())) + { Some(desc) => match desc.kind() { DescriptorKind::Data { value: Some(value), .. @@ -98,7 +101,7 @@ impl Operation for GetNameOrUndefined { .interner() .resolve_expect(binding_locator.name().sym()) .into_common(false); - match context.global_bindings_mut().get(&key) { + match context.global_bindings_mut().get(&PropertyKey::String(key)) { Some(desc) => match desc.kind() { DescriptorKind::Data { value: Some(value), .. diff --git a/boa_engine/src/vm/opcode/set/name.rs b/boa_engine/src/vm/opcode/set/name.rs index e3f99f76a29..7ed206e22b7 100644 --- a/boa_engine/src/vm/opcode/set/name.rs +++ b/boa_engine/src/vm/opcode/set/name.rs @@ -1,5 +1,6 @@ use crate::{ error::JsNativeError, + property::PropertyKey, vm::{opcode::Operation, CompletionType}, Context, JsResult, JsString, }; @@ -30,7 +31,10 @@ impl Operation for SetName { .interner() .resolve_expect(binding_locator.name().sym()) .into_common(false); - let exists = context.global_bindings_mut().contains_key(&key); + let exists = context + .global_bindings_mut() + .get(&PropertyKey::String(key.clone())) + .is_some(); if !exists && context.vm.frame().code_block.strict { return Err(JsNativeError::reference() From 4cdab9eef465201febb780b42d58c48a24c72bf2 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Thu, 23 Mar 2023 20:23:07 +0100 Subject: [PATCH 05/19] Implement unique shape property deleting delete --- boa_engine/src/object/shape/unique_shape.rs | 66 +++++++++++++++------ 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/boa_engine/src/object/shape/unique_shape.rs b/boa_engine/src/object/shape/unique_shape.rs index 0c56d80bb3e..707900ad4ef 100644 --- a/boa_engine/src/object/shape/unique_shape.rs +++ b/boa_engine/src/object/shape/unique_shape.rs @@ -6,7 +6,10 @@ use std::{ rc::Rc, }; +use bitflags::bitflags; +use boa_ast::property; use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc}; +use indexmap::{IndexMap, IndexSet}; use rustc_hash::FxHashMap; use crate::{ @@ -16,44 +19,69 @@ use crate::{ use super::{JsPrototype, Slot, TransitionKey}; +/// TODO: doc +#[derive(Default, Debug)] +struct Inner { + property_table: RefCell>, +} + +/// TODO: doc #[derive(Default, Debug, Clone)] pub(crate) struct UniqueShape { - property_table: Rc>>, - // /// Properties stored with `String`s a keys. - // string_properties: OrderedHashMap, - - // /// Properties stored with `Symbol`s a keys. - // symbol_properties: OrderedHashMap, + inner: Rc, } impl UniqueShape { + /// Create shape from a pre initialized property table. + fn new(property_table: IndexSet) -> Self { + Self { + inner: Rc::new(Inner { + property_table: property_table.into(), + }), + } + } + + /// TODO: doc pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { { - // println!("Unique: insert {}", key.property_key); - let mut property_table = self.property_table.borrow_mut(); - let offset = property_table.len() as u32; - property_table.insert(key.property_key, offset); + let mut property_table = self.inner.property_table.borrow_mut(); + let (index, inserted) = property_table.insert_full(key.property_key); + + debug_assert!(inserted); + debug_assert!(index + 1 == property_table.len()); } self.clone() } + /// TODO: doc pub(crate) fn remove_property_transition(&self, key: &TransitionKey) -> Self { - { - let mut property_table = self.property_table.borrow_mut(); - property_table.remove(&key.property_key); + let mut property_table = self.inner.property_table.borrow_mut(); + let Some((index, _property_key)) = property_table.shift_remove_full(&key.property_key) else { + return self.clone(); + }; + + /// Check if it's the last property, that was deleted, + /// if so delete it and return self + if index == property_table.len() { + return self.clone(); } - // self.clone() - todo!("implement deletion!") + // 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. + let property_table = std::mem::take(&mut *property_table); + Self::new(property_table) } - #[inline] + /// TODO: doc pub(crate) fn lookup(&self, key: &PropertyKey) -> Option { // println!("Unique: lookup {key}"); - let mut property_table = self.property_table.borrow(); - if let Some(index) = property_table.get(key) { - return Some(Slot { index: *index }); + let mut property_table = self.inner.property_table.borrow(); + if let Some(index) = property_table.get_index_of(key) { + return Some(Slot { + index: index as u32, + }); } None From 86c5bda8ea0a6adfd7a4f2418ae8350dc973db8c Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Thu, 23 Mar 2023 22:06:01 +0100 Subject: [PATCH 06/19] Temporary fix for object iter --- .../src/object/internal_methods/global.rs | 25 +---- .../internal_methods/integer_indexed.rs | 23 ++--- boa_engine/src/object/internal_methods/mod.rs | 21 +---- .../src/object/internal_methods/string.rs | 21 +---- boa_engine/src/object/property_map.rs | 94 ++----------------- boa_engine/src/object/shape/mod.rs | 7 ++ boa_engine/src/object/shape/shared_shape.rs | 25 +++++ boa_engine/src/object/shape/unique_shape.rs | 21 +++++ boa_engine/src/value/display.rs | 26 ++--- boa_engine/src/value/serde_json.rs | 11 ++- 10 files changed, 98 insertions(+), 176 deletions(-) diff --git a/boa_engine/src/object/internal_methods/global.rs b/boa_engine/src/object/internal_methods/global.rs index 41be3c3d052..f9c699fef35 100644 --- a/boa_engine/src/object/internal_methods/global.rs +++ b/boa_engine/src/object/internal_methods/global.rs @@ -254,30 +254,15 @@ pub(crate) fn global_own_property_keys( }; // 2. For each own property key P of O such that P is an array index, in ascending numeric index order, do - // a. Add P as the last element of keys. + // a. Add P as the last element of keys. keys.extend(ordered_indexes.into_iter().map(Into::into)); // 3. For each own property key P of O such that Type(P) is String and P is not an array index, in ascending chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - context - .realm - .global_property_map - .string_property_keys() - .cloned() - .map(Into::into), - ); - + // a. Add P as the last element of keys. + // // 4. For each own property key P of O such that Type(P) is Symbol, in ascending chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - context - .realm - .global_property_map - .symbol_property_keys() - .cloned() - .map(Into::into), - ); + // a. Add P as the last element of keys. + keys.extend(context.realm.global_property_map.shape.keys()); // 5. Return keys. Ok(keys) diff --git a/boa_engine/src/object/internal_methods/integer_indexed.rs b/boa_engine/src/object/internal_methods/integer_indexed.rs index e5cf99063d5..2e9f5d13117 100644 --- a/boa_engine/src/object/internal_methods/integer_indexed.rs +++ b/boa_engine/src/object/internal_methods/integer_indexed.rs @@ -222,30 +222,19 @@ pub(crate) fn integer_indexed_exotic_own_property_keys( vec![] } else { // 2. If IsDetachedBuffer(O.[[ViewedArrayBuffer]]) is false, then - // a. For each integer i starting with 0 such that i < O.[[ArrayLength]], in ascending order, do - // i. Add ! ToString(𝔽(i)) as the last element of keys. + // a. For each integer i starting with 0 such that i < O.[[ArrayLength]], in ascending order, do + // i. Add ! ToString(𝔽(i)) as the last element of keys. (0..inner.array_length()) .map(|index| PropertyKey::Index(index as u32)) .collect() }; // 3. For each own property key P of O such that Type(P) is String and P is not an array index, in ascending chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - obj.properties - .string_property_keys() - .cloned() - .map(Into::into), - ); - + // a. Add P as the last element of keys. + // // 4. For each own property key P of O such that Type(P) is Symbol, in ascending chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - obj.properties - .symbol_property_keys() - .cloned() - .map(Into::into), - ); + // a. Add P as the last element of keys. + keys.extend(obj.properties.shape.keys()); // 5. Return keys. Ok(keys) diff --git a/boa_engine/src/object/internal_methods/mod.rs b/boa_engine/src/object/internal_methods/mod.rs index 00279675f66..fc5680bdf96 100644 --- a/boa_engine/src/object/internal_methods/mod.rs +++ b/boa_engine/src/object/internal_methods/mod.rs @@ -714,24 +714,11 @@ pub(crate) fn ordinary_own_property_keys( keys.extend(ordered_indexes.into_iter().map(Into::into)); // 3. For each own property key P of O such that Type(P) is String and P is not an array index, in ascending chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - obj.borrow() - .properties - .string_property_keys() - .cloned() - .map(Into::into), - ); - + // a. Add P as the last element of keys. + // // 4. For each own property key P of O such that Type(P) is Symbol, in ascending chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - obj.borrow() - .properties - .symbol_property_keys() - .cloned() - .map(Into::into), - ); + // a. Add P as the last element of keys. + keys.extend(obj.borrow().properties.shape.keys()); // 5. Return keys. Ok(keys) diff --git a/boa_engine/src/object/internal_methods/string.rs b/boa_engine/src/object/internal_methods/string.rs index c4151741694..0c919b5fab5 100644 --- a/boa_engine/src/object/internal_methods/string.rs +++ b/boa_engine/src/object/internal_methods/string.rs @@ -101,12 +101,12 @@ pub(crate) fn string_exotic_own_property_keys( let mut keys = Vec::with_capacity(len); // 5. For each integer i starting with 0 such that i < len, in ascending order, do - // a. Add ! ToString(𝔽(i)) as the last element of keys. + // a. Add ! ToString(𝔽(i)) as the last element of keys. keys.extend((0..len).map(Into::into)); // 6. For each own property key P of O such that P is an array index // and ! ToIntegerOrInfinity(P) ≥ len, in ascending numeric index order, do - // a. Add P as the last element of keys. + // a. Add P as the last element of keys. let mut remaining_indices: Vec<_> = obj .properties .index_property_keys() @@ -117,23 +117,12 @@ pub(crate) fn string_exotic_own_property_keys( // 7. For each own property key P of O such that Type(P) is String and P is not // an array index, in ascending chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - obj.properties - .string_property_keys() - .cloned() - .map(Into::into), - ); + // a. Add P as the last element of keys. // 8. For each own property key P of O such that Type(P) is Symbol, in ascending // chronological order of property creation, do - // a. Add P as the last element of keys. - keys.extend( - obj.properties - .symbol_property_keys() - .cloned() - .map(Into::into), - ); + // a. Add P as the last element of keys. + keys.extend(obj.properties.shape.keys()); // 9. Return keys. Ok(keys) diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index f34f8fbfa76..fafaa9ae23a 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, TransitionKey}, + shape::{Shape, Slot, TransitionKey}, PropertyDescriptor, PropertyKey, }; use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; @@ -263,6 +263,12 @@ impl PropertyMap { None } + /// 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() + } + /// Insert the given property descriptor with the given key [`PropertyMap`]. pub fn insert( &mut self, @@ -323,65 +329,6 @@ impl PropertyMap { } } - /// 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. - #[inline] - #[must_use] - pub fn iter(&self) -> Iter<'_> { - Iter { - indexed_properties: self.indexed_properties.iter(), - string_properties: self.string_properties.0.iter(), - symbol_properties: self.symbol_properties.0.iter(), - } - } - - /// An iterator visiting all keys in arbitrary order. The iterator element type is `PropertyKey`. - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn keys(&self) -> Keys<'_> { - Keys(self.iter()) - } - - /// An iterator visiting all values in arbitrary order. The iterator element type is `&'a Property`. - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn values(&self) -> Values<'_> { - Values(self.iter()) - } - - /// An iterator visiting all symbol key-value pairs in arbitrary order. The iterator element type is `(&'a RcSymbol, &'a Property)`. - /// - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn symbol_properties(&self) -> SymbolProperties<'_> { - SymbolProperties(self.symbol_properties.0.iter()) - } - - /// An iterator visiting all symbol keys in arbitrary order. The iterator element type is `&'a RcSymbol`. - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn symbol_property_keys(&self) -> SymbolPropertyKeys<'_> { - SymbolPropertyKeys(self.symbol_properties.0.keys()) - } - - /// An iterator visiting all symbol values in arbitrary order. The iterator element type is `&'a Property`. - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn symbol_property_values(&self) -> SymbolPropertyValues<'_> { - SymbolPropertyValues(self.symbol_properties.0.values()) - } - /// An iterator visiting all indexed key-value pairs in arbitrary order. The iterator element type is `(&'a u32, &'a Property)`. /// /// This iterator does not recurse down the prototype chain. @@ -409,33 +356,6 @@ impl PropertyMap { self.indexed_properties.values() } - /// An iterator visiting all string key-value pairs in arbitrary order. The iterator element type is `(&'a RcString, &'a Property)`. - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn string_properties(&self) -> StringProperties<'_> { - StringProperties(self.string_properties.0.iter()) - } - - /// An iterator visiting all string keys in arbitrary order. The iterator element type is `&'a RcString`. - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn string_property_keys(&self) -> StringPropertyKeys<'_> { - StringPropertyKeys(self.string_properties.0.keys()) - } - - /// An iterator visiting all string values in arbitrary order. The iterator element type is `&'a Property`. - /// - /// This iterator does not recurse down the prototype chain. - #[inline] - #[must_use] - pub fn string_property_values(&self) -> StringPropertyValues<'_> { - StringPropertyValues(self.string_properties.0.values()) - } - /// Returns `true` if the given key is contained in the [`PropertyMap`]. #[inline] #[must_use] diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs index 9c02cb21f28..c70860a51c7 100644 --- a/boa_engine/src/object/shape/mod.rs +++ b/boa_engine/src/object/shape/mod.rs @@ -122,4 +122,11 @@ impl Shape { Inner::Unique(shape) => shape.lookup(key), } } + + pub(crate) fn keys(&self) -> Vec { + match &self.inner { + Inner::Shared(shape) => shape.keys(), + Inner::Unique(shape) => shape.keys(), + } + } } diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index b379848c7cb..ed4064e5e00 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -285,4 +285,29 @@ impl SharedShape { result } + + // TODO: implement DescriptorArray to optimize this :) + pub(crate) fn keys(&self) -> Vec { + let mut keys_string = Vec::new(); + let mut keys_symbol = Vec::new(); + + let mut p = Some(self); + while let Some(shape) = p { + // Root has no properties + if shape.is_root() { + break; + } + if shape.transition_type() == TransitionType::Insert { + if matches!(shape.property_key(), PropertyKey::String(_)) { + keys_string.push(shape.property_key().clone()); + } else { + keys_symbol.push(shape.property_key().clone()); + } + } + p = shape.previous(); + } + keys_string.reverse(); + keys_string.extend(keys_symbol.into_iter().rev()); + keys_string + } } diff --git a/boa_engine/src/object/shape/unique_shape.rs b/boa_engine/src/object/shape/unique_shape.rs index 707900ad4ef..243e987d92a 100644 --- a/boa_engine/src/object/shape/unique_shape.rs +++ b/boa_engine/src/object/shape/unique_shape.rs @@ -86,4 +86,25 @@ impl UniqueShape { None } + + /// 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() + .filter(|x| matches!(x, PropertyKey::String(_))) + { + keys.push(key.clone()); + } + for key in property_table + .iter() + .filter(|x| matches!(x, PropertyKey::Symbol(_))) + { + keys.push(key.clone()); + } + + keys + } } diff --git a/boa_engine/src/value/display.rs b/boa_engine/src/value/display.rs index ea54b0ffc1c..313f31b2b4f 100644 --- a/boa_engine/src/value/display.rs +++ b/boa_engine/src/value/display.rs @@ -67,15 +67,19 @@ macro_rules! print_obj_value { } }; (props of $obj:expr, $display_fn:ident, $indent:expr, $encounters:expr, $print_internals:expr) => { - print_obj_value!(impl $obj, |(key, val)| { + {let mut keys: Vec<_> = $obj.borrow().properties().index_property_keys().map(crate::property::PropertyKey::Index).collect(); + keys.extend($obj.borrow().properties().shape.keys()); + let mut result = Vec::default(); + for key in keys { + let val = $obj.borrow().properties().get(&key).expect("There should be a value"); if val.is_data_descriptor() { let v = &val.expect_value(); - format!( + result.push(format!( "{:>width$}: {}", key, $display_fn(v, $encounters, $indent.wrapping_add(4), $print_internals), width = $indent, - ) + )); } else { let display = match (val.set().is_some(), val.get().is_some()) { (true, true) => "Getter & Setter", @@ -83,20 +87,10 @@ macro_rules! print_obj_value { (false, true) => "Getter", _ => "No Getter/Setter" }; - format!("{:>width$}: {}", key, display, width = $indent) + result.push(format!("{:>width$}: {}", key, display, width = $indent)); } - }) - }; - - // A private overload of the macro - // DO NOT use directly - (impl $v:expr, $f:expr) => { - $v - .borrow() - .properties() - .iter() - .map($f) - .collect::>() + } + result} }; } diff --git a/boa_engine/src/value/serde_json.rs b/boa_engine/src/value/serde_json.rs index 153c05c2680..7f2187a55b1 100644 --- a/boa_engine/src/value/serde_json.rs +++ b/boa_engine/src/value/serde_json.rs @@ -137,8 +137,8 @@ impl JsValue { Ok(Value::Array(arr)) } else { let mut map = Map::new(); - for (key, property) in obj.borrow().properties().iter() { - let key = match &key { + for property_key in obj.borrow().properties().shape.keys() { + let key = match &property_key { PropertyKey::String(string) => string.to_std_string_escaped(), PropertyKey::Index(i) => i.to_string(), PropertyKey::Symbol(_sym) => { @@ -148,7 +148,12 @@ impl JsValue { } }; - let value = match property.value() { + let value = match obj + .borrow() + .properties() + .get(&property_key) + .and_then(|x| x.value().cloned()) + { Some(val) => val.to_json(context)?, None => Value::Null, }; From 661ec922aa169beefb78c870ce2f3c16f0f6d177 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Fri, 24 Mar 2023 00:33:07 +0100 Subject: [PATCH 07/19] Move property desciptor flags to shape --- boa_engine/src/object/property_map.rs | 151 ++++++++++++++++---- boa_engine/src/object/shape/mod.rs | 31 +++- boa_engine/src/object/shape/shared_shape.rs | 74 ++++++++-- boa_engine/src/object/shape/unique_shape.rs | 37 +++-- 4 files changed, 233 insertions(+), 60 deletions(-) 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()); From d4e5137e707b1c88ea77b571e417ee4cdf42b850 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Fri, 24 Mar 2023 14:38:16 +0100 Subject: [PATCH 08/19] Remove unneeded Option in property_table --- boa_engine/src/object/property_map.rs | 23 ++-- boa_engine/src/object/shape/mod.rs | 18 ++- boa_engine/src/object/shape/shared_shape.rs | 142 ++++++++++++-------- 3 files changed, 108 insertions(+), 75 deletions(-) diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index cff54430ef1..e7fd4a24dbe 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -267,13 +267,11 @@ impl PropertyMap { 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) { + if attributes.is_accessor_descriptor() { + if attributes.has_get() { builder = builder.get(self.storage[index].clone()); } - if attributes.contains(PropertyDescriptorAttribute::HAS_SET) { + if attributes.has_set() { builder = builder.set(self.storage[index + 1].clone()); } } else { @@ -336,21 +334,18 @@ impl PropertyMap { property_key: key.clone(), attributes, }; - self.shape = self.shape.change_attributes_transition(key); + self.shape = self.shape.change_attributes_transition(key, slot.index); } - // 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) { + if attributes.is_accessor_descriptor() { + if attributes.has_get() { self.storage[index] = property .get() .cloned() .map(JsValue::new) .unwrap_or_default(); } - if attributes.contains(PropertyDescriptorAttribute::HAS_SET) { + if attributes.has_set() { self.storage[index + 1] = property .set() .cloned() @@ -369,9 +364,7 @@ impl PropertyMap { attributes, }; self.shape = self.shape.insert_property_transition(transition_key); - if attributes.contains(PropertyDescriptorAttribute::HAS_GET) - || attributes.contains(PropertyDescriptorAttribute::HAS_SET) - { + if attributes.is_accessor_descriptor() { self.storage.push( property .get() diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs index a2c5fba5741..90255539629 100644 --- a/boa_engine/src/object/shape/mod.rs +++ b/boa_engine/src/object/shape/mod.rs @@ -43,6 +43,22 @@ bitflags! { } } +impl PropertyDescriptorAttribute { + pub const fn is_accessor_descriptor(self) -> bool { + self.contains(Self::HAS_GET) || self.contains(Self::HAS_SET) + } + pub const fn is_data_descriptor(self) -> bool { + !self.is_accessor_descriptor() + } + + pub const fn has_get(self) -> bool { + self.contains(Self::HAS_GET) + } + pub const fn has_set(self) -> bool { + self.contains(Self::HAS_SET) + } +} + #[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] pub(crate) struct TransitionKey { pub(crate) property_key: PropertyKey, @@ -118,7 +134,7 @@ impl Shape { } } - pub(crate) fn change_attributes_transition(&self, key: TransitionKey) -> Self { + pub(crate) fn change_attributes_transition(&self, key: TransitionKey, index: u32) -> Self { match &self.inner { Inner::Shared(shape) => Self::shared(shape.change_attributes_transition(key)), Inner::Unique(shape) => Self::unique(shape.change_attributes_transition(key)), diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index 5e1a5abf32c..7b322ccd9ee 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -1,6 +1,10 @@ -use std::cell::{Cell, RefCell}; +use std::{ + cell::{Cell, RefCell}, + collections::hash_map::RandomState, +}; use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; +use indexmap::{IndexMap, IndexSet}; use rustc_hash::FxHashMap; use crate::property::{Attribute, PropertyDescriptor, PropertyKey}; @@ -12,10 +16,7 @@ use super::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey, TransitionT struct Inner { /// Cached property map, only constucted when a property is accesed through it. #[unsafe_ignore_trace] - property_table: RefCell>>, - - #[unsafe_ignore_trace] - property_access: Cell, + property_table: RefCell>, forward_property_transitions: GcRefCell>>, previous: Option, @@ -37,7 +38,7 @@ pub(crate) struct SharedShape { } impl SharedShape { - const DEBUG: bool = false; + const DEBUG: bool = true; #[inline] pub(crate) fn previous(&self) -> Option<&SharedShape> { @@ -73,8 +74,7 @@ impl SharedShape { #[inline] pub(crate) fn root() -> Self { Self::new(Inner { - property_table: RefCell::new(None), - property_access: Cell::new(0), + property_table: RefCell::default(), forward_property_transitions: GcRefCell::new(FxHashMap::default()), property_key: PropertyKey::Index(0), property_index: 0, @@ -98,8 +98,7 @@ impl SharedShape { self.property_index() + 2 }; let new_inner_shape = Inner { - property_table: RefCell::new(None), - property_access: Cell::new(0), + property_table: RefCell::default(), forward_property_transitions: GcRefCell::new(FxHashMap::default()), property_index: next_property_index, property_key: key.property_key.clone(), @@ -148,14 +147,8 @@ impl SharedShape { 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), + property_table: RefCell::default(), forward_property_transitions: GcRefCell::new(FxHashMap::default()), property_index: self.property_index(), property_key: key.property_key.clone(), @@ -200,7 +193,8 @@ impl SharedShape { if Self::DEBUG { println!("Shape: deleting {key}"); } - let mut transitions = Vec::default(); + let mut transitions: IndexMap = + IndexMap::default(); let mut p = Some(self); let mut base = loop { @@ -218,33 +212,46 @@ impl SharedShape { break base; } - transitions.push(TransitionKey { - property_key: shape.property_key().clone(), - attributes: shape.inner.property_attributes, - }); + // Do not add property that we are trying to delete. + // this can happen if a configure was called after inserting it into the shape + if shape.property_key() != key { + // Only take the latest changes to a property. To try to build a smaller tree. + transitions + .entry(shape.property_key().clone()) + .or_insert(shape.inner.property_attributes); + } p = shape.previous(); }; - for transition in transitions.into_iter().rev() { + for (property_key, attributes) in IntoIterator::into_iter(transitions).rev() { + let transition = TransitionKey { + property_key, + attributes, + }; base = base.insert_property_transition(transition); } base } + // TODO: Implement descriptor array. #[inline] pub(crate) fn lookup(&self, key: &PropertyKey) -> Option { if Self::DEBUG { println!("Shape: lookup {key}"); } + + self.previous()?; + // Use cheched property table, if exists - if self.inner.property_access.get() == 4 { + { let property_table = self.inner.property_table.borrow(); - if let Some(property_table) = property_table.as_ref() { + if property_table.len() != 0 { if Self::DEBUG { println!(" Shape: Using cached property table"); } + if let Some((index, attributes)) = property_table.get(key).copied() { return Some(Slot { index, attributes }); } @@ -253,35 +260,44 @@ impl SharedShape { } } - if self.inner.property_access.get() < 4 { - self.inner - .property_access - .set(self.inner.property_access.get() + 1); - - let mut p = Some(self); - while let Some(shape) = p { - // Root has no properties - if shape.is_root() { - break; - } - if shape.property_key() == key { - if Self::DEBUG { - println!( - " Shape: found {} with {} property index", - shape.property_key(), - shape.property_index() - ); - } - return Some(Slot { - index: shape.property_index(), - attributes: shape.inner.property_attributes, - }); - } - p = shape.previous(); - } - - return None; - } + // if self.inner.property_access.get() < 4 { + // self.inner + // .property_access + // .set(self.inner.property_access.get() + 1); + + // let mut p = Some(self); + + // let mut index = 0; + // let mut attributes = None; + // while let Some(shape) = p { + // // Root has no properties + // if shape.is_root() { + // break; + // } + // if shape.property_key() == key { + // if Self::DEBUG { + // println!( + // " Shape: found {} with {} property index", + // shape.property_key(), + // shape.property_index() + // ); + // } + // if attributes.is_none() { + // attributes = Some(shape.inner.property_attributes); + // } + + // if shape.transition_type() == TransitionType::Insert { + // return Some(Slot { + // index: shape.property_index(), + // attributes: attributes.expect("This should have already been set!"), + // }); + // } + // } + // p = shape.previous(); + // } + + // return None; + // } if Self::DEBUG { println!(" Shape: Creating cached property table"); @@ -291,6 +307,7 @@ impl SharedShape { // Creating cached property table let mut result = None; + let mut attributes = None; let mut p = Some(self); while let Some(shape) = p { // Root has no properties @@ -305,16 +322,23 @@ impl SharedShape { shape.property_index() ); } - result = Some(Slot { - index: shape.property_index(), - attributes: shape.inner.property_attributes, - }); + if attributes.is_none() { + attributes = Some(shape.inner.property_attributes); + } + + if shape.transition_type() == TransitionType::Insert { + result = Some(Slot { + index: shape.property_index(), + attributes: attributes.expect("This should have already been set!"), + }); + } } // 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()) + .and_modify(|(index, _)| *index = shape.property_index()) .or_insert((shape.property_index(), shape.inner.property_attributes)); p = shape.previous(); @@ -325,7 +349,7 @@ impl SharedShape { // Put into shape let mut property_table_borrow = self.inner.property_table.borrow_mut(); - property_table_borrow.insert(property_table); + std::mem::replace(&mut *property_table_borrow, property_table); result } From 086c9cc8522c15d23318fb39781a5b4935c0e09a Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 00:30:37 +0100 Subject: [PATCH 09/19] Implement shared property table --- boa_engine/src/object/shape/shared_shape.rs | 369 ++++++++++---------- 1 file changed, 183 insertions(+), 186 deletions(-) diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index 7b322ccd9ee..c4d8b919758 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -1,32 +1,120 @@ use std::{ cell::{Cell, RefCell}, collections::hash_map::RandomState, + rc::Rc, }; use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; use indexmap::{IndexMap, IndexSet}; use rustc_hash::FxHashMap; -use crate::property::{Attribute, PropertyDescriptor, PropertyKey}; +use crate::{ + object::JsPrototype, + property::{Attribute, PropertyDescriptor, PropertyKey}, +}; use super::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey, TransitionType}; +type SlotIndex = u32; + +#[derive(Default, Debug, Clone)] +struct PropertyTableInner { + properties: RefCell>, +} + +#[derive(Default, Debug, Clone)] +struct PropertyTable { + inner: Rc, +} + +impl PropertyTable { + fn properties( + &self, + ) -> &RefCell> { + &self.inner.properties + } + + fn add_property_deep_clone_if_needed( + &self, + key: PropertyKey, + attributes: PropertyDescriptorAttribute, + slot_index: u32, + property_count: u32, + ) -> (u32, Self) { + { + let mut properties = self.inner.properties.borrow_mut(); + if (property_count as usize) == properties.len() && !properties.contains_key(&key) { + // println!("Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}"); + let (index, value) = properties.insert_full(key, (slot_index, attributes)); + debug_assert!(value.is_none()); + return (index as u32, self.clone()); + } + } + + // println!("Creating fork PropertyTable with {key} - Slot {slot_index} - PC {property_count}"); + + // property is already present need to make deep clone of property table. + let this = self.deep_clone(property_count); + let index = { + let mut properties = this.inner.properties.borrow_mut(); + let (index, value) = properties.insert_full(key, (slot_index, attributes)); + debug_assert!(value.is_none()); + index + }; + (index as u32, this) + } + + fn deep_clone(&self, count: u32) -> Self { + let count = count as usize; + + let properties = { + let mut properties = self.inner.properties.borrow(); + let mut result = IndexMap::default(); + result.extend( + properties + .iter() + .take(count) + .map(|(key, (index, attributes))| (key.clone(), (*index, *attributes))), + ); + result + }; + Self { + inner: Rc::new(PropertyTableInner { + properties: RefCell::new(properties), + }), + } + } + + fn deep_clone_all(&self) -> Self { + Self { + inner: Rc::new((*self.inner).clone()), + } + } + + fn set_attributes( + &self, + property_key: &PropertyKey, + property_attributes: PropertyDescriptorAttribute, + ) { + let mut properties = self.inner.properties.borrow_mut(); + let Some((_index, attributes)) = properties.get_mut(property_key) else { + unreachable!("There should already be a property!") + }; + *attributes = property_attributes; + } +} + /// 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>, - forward_property_transitions: GcRefCell>>, - previous: Option, - #[unsafe_ignore_trace] - property_key: PropertyKey, property_index: u32, #[unsafe_ignore_trace] - property_attributes: PropertyDescriptorAttribute, + property_table: PropertyTable, + + previous: Option, // TODO: add prototype to shape transition_type: TransitionType, @@ -38,19 +126,19 @@ pub(crate) struct SharedShape { } impl SharedShape { - const DEBUG: bool = true; + const DEBUG: bool = false; #[inline] pub(crate) fn previous(&self) -> Option<&SharedShape> { self.inner.previous.as_ref() } #[inline] - pub(crate) fn property_key(&self) -> &PropertyKey { - &self.inner.property_key - } - #[inline] - pub(crate) fn property_index(&self) -> u32 { - self.inner.property_index + pub(crate) fn property(&self) -> (PropertyKey, SlotIndex, PropertyDescriptorAttribute) { + let properties = self.inner.property_table.properties().borrow(); + let (key, (index, attributes)) = properties + .get_index(self.inner.property_index as usize) + .expect("There should be a key in property table"); + (key.clone(), *index, *attributes) } fn transition_type(&self) -> TransitionType { self.inner.transition_type @@ -59,6 +147,9 @@ impl SharedShape { pub(crate) fn is_root(&self) -> bool { self.inner.previous.is_none() } + fn property_index(&self) -> u32 { + self.inner.property_index + } #[inline] fn get_forward_transition(&self, key: &TransitionKey) -> Option> { let map = self.inner.forward_property_transitions.borrow(); @@ -74,44 +165,46 @@ impl SharedShape { #[inline] pub(crate) fn root() -> Self { Self::new(Inner { - property_table: RefCell::default(), - forward_property_transitions: GcRefCell::new(FxHashMap::default()), - property_key: PropertyKey::Index(0), property_index: 0, - property_attributes: PropertyDescriptorAttribute::default(), + property_table: PropertyTable::default(), + forward_property_transitions: GcRefCell::new(FxHashMap::default()), previous: None, transition_type: TransitionType::Insert, }) } fn create_property_transition(&self, key: TransitionKey) -> Self { - if Self::DEBUG { - println!( - "Shape: Create Transition: {} -------> {}", - self.property_key(), - key.property_key - ); - } - let next_property_index = if self.previous().is_none() { - self.property_index() + let slot_index = if self.previous().is_none() { + 0 } else { - self.property_index() + 2 + // TODO: implement different sized storage + (self.property_index() + 1) * 2 }; + + let self_property_count = if self.previous().is_none() { + 0 + } else { + self.property_index() + 1 + }; + + let (property_index, property_table) = + self.inner.property_table.add_property_deep_clone_if_needed( + key.property_key.clone(), + key.attributes, + slot_index, + self_property_count, + ); let new_inner_shape = Inner { - property_table: RefCell::default(), forward_property_transitions: GcRefCell::new(FxHashMap::default()), - property_index: next_property_index, - property_key: key.property_key.clone(), - property_attributes: key.attributes, + property_table, + property_index, previous: Some(self.clone()), transition_type: TransitionType::Insert, }; 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)); - } + let mut map = self.inner.forward_property_transitions.borrow_mut(); + map.insert(key, WeakGc::new(&new_shape.inner)); new_shape } @@ -140,28 +233,19 @@ impl SharedShape { } pub(crate) fn change_attributes_transition(&self, key: TransitionKey) -> Self { - if Self::DEBUG { - println!( - "Shape: Change attributes transition: {} -------> {:?}", - self.property_key(), - key.attributes, - ); - } + let property_table = self.inner.property_table.deep_clone_all(); + property_table.set_attributes(&key.property_key, key.attributes); let new_inner_shape = Inner { - property_table: RefCell::default(), + property_table, + property_index: self.inner.property_index, 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)); - } + let mut map = self.inner.forward_property_transitions.borrow_mut(); + map.insert(key, WeakGc::new(&new_shape.inner)); new_shape } @@ -193,35 +277,40 @@ impl SharedShape { if Self::DEBUG { println!("Shape: deleting {key}"); } + let mut transitions: IndexMap = IndexMap::default(); - let mut p = Some(self); + let mut current = Some(self); let mut base = loop { - let Some(shape) = p else { + let Some(current_shape) = current else { unreachable!("The chain should have insert transition type!") }; - if shape.transition_type() == TransitionType::Insert && shape.property_key() == key { - let mut base = if let Some(base) = shape.previous() { + let (current_property_key, _, current_property_attributes) = current_shape.property(); + + if current_shape.transition_type() == TransitionType::Insert + && ¤t_property_key == key + { + let mut base = if let Some(base) = current_shape.previous() { base.clone() } else { // It's the root, because it doesn't have previous. - shape.clone() + current_shape.clone() }; break base; } // Do not add property that we are trying to delete. // this can happen if a configure was called after inserting it into the shape - if shape.property_key() != key { + if ¤t_property_key != key { // Only take the latest changes to a property. To try to build a smaller tree. transitions - .entry(shape.property_key().clone()) - .or_insert(shape.inner.property_attributes); + .entry(current_property_key) + .or_insert(current_property_attributes); } - p = shape.previous(); + current = current_shape.previous(); }; for (property_key, attributes) in IntoIterator::into_iter(transitions).rev() { @@ -235,147 +324,55 @@ impl SharedShape { base } - // TODO: Implement descriptor array. #[inline] pub(crate) fn lookup(&self, key: &PropertyKey) -> Option { if Self::DEBUG { println!("Shape: lookup {key}"); } + // Root has not element, return None. self.previous()?; - // Use cheched property table, if exists + let property_table = self.inner.property_table.properties().borrow(); + if let Some((property_table_index, property_key, (slot_index, attributes))) = + property_table.get_full(key) { - let property_table = self.inner.property_table.borrow(); - if property_table.len() != 0 { - if Self::DEBUG { - println!(" Shape: Using cached property table"); - } - - if let Some((index, attributes)) = property_table.get(key).copied() { - return Some(Slot { index, attributes }); - } + debug_assert_eq!(key, property_key); + if Self::DEBUG { + println!("Key {key}, PTI: {property_table_index}, SI: {slot_index}"); + } + // Check if we are trying to access properties that belong to another shape. + if (property_table_index as u32) > self.inner.property_index { return None; } + return Some(Slot { + index: *slot_index, + attributes: *attributes, + }); } + None + } - // if self.inner.property_access.get() < 4 { - // self.inner - // .property_access - // .set(self.inner.property_access.get() + 1); - - // let mut p = Some(self); - - // let mut index = 0; - // let mut attributes = None; - // while let Some(shape) = p { - // // Root has no properties - // if shape.is_root() { - // break; - // } - // if shape.property_key() == key { - // if Self::DEBUG { - // println!( - // " Shape: found {} with {} property index", - // shape.property_key(), - // shape.property_index() - // ); - // } - // if attributes.is_none() { - // attributes = Some(shape.inner.property_attributes); - // } - - // if shape.transition_type() == TransitionType::Insert { - // return Some(Slot { - // index: shape.property_index(), - // attributes: attributes.expect("This should have already been set!"), - // }); - // } - // } - // p = shape.previous(); - // } - - // return None; - // } - - if Self::DEBUG { - println!(" Shape: Creating cached property table"); - } - - let mut property_table = FxHashMap::default(); + pub(crate) fn keys(&self) -> Vec { + let property_table = self.inner.property_table.properties().borrow(); - // Creating cached property table - let mut result = None; - let mut attributes = None; - let mut p = Some(self); - while let Some(shape) = p { - // Root has no properties - if shape.is_root() { - break; - } - if result.is_none() && shape.property_key() == key { - if Self::DEBUG { - println!( - " Shape: found {} with {} property index", - shape.property_key(), - shape.property_index() - ); - } - if attributes.is_none() { - attributes = Some(shape.inner.property_attributes); - } - - if shape.transition_type() == TransitionType::Insert { - result = Some(Slot { - index: shape.property_index(), - attributes: attributes.expect("This should have already been set!"), - }); - } - } + let property_count = (self.property_index() + 1) as usize; - // If we already added a property we don't add the previous versions - // which can happens if a configure property transition occurs. + let mut keys: Vec = property_table + .keys() + .take(property_count) + .filter(|key| matches!(key, PropertyKey::String(_))) + .cloned() + .collect(); + keys.extend( property_table - .entry(shape.property_key().clone()) - .and_modify(|(index, _)| *index = shape.property_index()) - .or_insert((shape.property_index(), shape.inner.property_attributes)); + .keys() + .take(property_count) + .filter(|key| matches!(key, PropertyKey::Symbol(_))) + .cloned(), + ); - p = shape.previous(); - } - - // Trim excess space - property_table.shrink_to_fit(); - - // Put into shape - let mut property_table_borrow = self.inner.property_table.borrow_mut(); - std::mem::replace(&mut *property_table_borrow, property_table); - - result - } - - // TODO: implement DescriptorArray to optimize this :) - pub(crate) fn keys(&self) -> Vec { - let mut keys_string = Vec::new(); - let mut keys_symbol = Vec::new(); - - let mut p = Some(self); - while let Some(shape) = p { - // Root has no properties - if shape.is_root() { - break; - } - if shape.transition_type() == TransitionType::Insert { - if matches!(shape.property_key(), PropertyKey::String(_)) { - keys_string.push(shape.property_key().clone()); - } else { - keys_symbol.push(shape.property_key().clone()); - } - } - p = shape.previous(); - } - keys_string.reverse(); - keys_string.extend(keys_symbol.into_iter().rev()); - keys_string + keys } } From 068f7c86c4673cdb12f5cbfafcd309e5319f7de2 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 09:38:48 +0100 Subject: [PATCH 10/19] Remove unneeded return of insert and remove --- boa_engine/src/object/jsobject.rs | 8 ++-- boa_engine/src/object/mod.rs | 8 ++-- boa_engine/src/object/property_map.rs | 67 +++++++++------------------ 3 files changed, 31 insertions(+), 52 deletions(-) diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index 3cb5dbd4df4..6b6a47a71cd 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -706,7 +706,7 @@ Cannot both specify accessors and a value or writable attribute", /// Helper function for property insertion. #[track_caller] - pub(crate) fn insert(&self, key: K, property: P) -> Option + pub(crate) fn insert(&self, key: K, property: P) -> bool where K: Into, P: Into, @@ -716,9 +716,9 @@ Cannot both specify accessors and a value or writable attribute", /// 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 that a `Some` is returned - /// with that field, otherwise None is returned. - pub fn insert_property(&self, key: K, property: P) -> Option + /// If a field was already in the object with the same name, than `true` is returned + /// with that field, otherwise `false` is returned. + pub fn insert_property(&self, key: K, property: P) -> bool where K: Into, P: Into, diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index e4f26a4b49d..20f0eb510cc 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -1837,9 +1837,9 @@ impl Object { /// 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 - /// with that field's value, otherwise, `None` is returned. - pub(crate) fn insert(&mut self, key: K, property: P) -> Option + /// If a field was already in the object with the same name, then `true` is returned + /// otherwise, `false` is returned. + pub(crate) fn insert(&mut self, key: K, property: P) -> bool where K: Into, P: Into, @@ -1849,7 +1849,7 @@ impl Object { /// Helper function for property removal. #[inline] - pub(crate) fn remove(&mut self, key: &PropertyKey) -> Option { + pub(crate) fn remove(&mut self, key: &PropertyKey) -> bool { self.properties.remove(key) } diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index e7fd4a24dbe..426cb4a161f 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -101,9 +101,9 @@ impl IndexedProperties { } /// Inserts a property descriptor with the specified key. - fn insert(&mut self, key: u32, property: PropertyDescriptor) -> Option { + fn insert(&mut self, key: u32, property: PropertyDescriptor) -> bool { let vec = match self { - Self::Sparse(map) => return map.insert(key, property), + Self::Sparse(map) => return map.insert(key, property).is_some(), Self::Dense(vec) => { let len = vec.len() as u32; if key <= len @@ -124,19 +124,12 @@ impl IndexedProperties { // Since the previous key is the current key - 1. Meaning that the elements are continuos. if key == len { vec.push(value); - return None; + return false; } // 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(), - ); + return true; } vec @@ -145,37 +138,32 @@ impl IndexedProperties { // Slow path: converting to sparse storage. let mut map = Self::convert_dense_to_sparse(vec); - let old_property = map.insert(key, property); + let replaced = map.insert(key, property).is_some(); *self = Self::Sparse(map); - old_property + replaced } /// Inserts a property descriptor with the specified key. - fn remove(&mut self, key: u32) -> Option { + fn remove(&mut self, key: u32) -> bool { let vec = match self { - Self::Sparse(map) => return map.remove(&key), + Self::Sparse(map) => { + return map.remove(&key).is_some(); + } 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; + return false; } - // If the key is pointing at the last element, then we pop it and return it. + // If the key is pointing at the last element, then we pop it. // - // It does not make the storage sparse + // 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.pop().expect("Already checked if it is out of bounds"); + return true; } vec @@ -184,10 +172,10 @@ impl IndexedProperties { // Slow Path: conversion to sparse storage. let mut map = Self::convert_dense_to_sparse(vec); - let old_property = map.remove(&key); + let removed = map.remove(&key).is_some(); *self = Self::Sparse(map); - old_property + removed } /// Check if we contain the key to a property descriptor. @@ -284,11 +272,7 @@ impl PropertyMap { /// 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, - property: PropertyDescriptor, - ) -> Option { + pub fn insert(&mut self, key: &PropertyKey, property: PropertyDescriptor) -> bool { if let PropertyKey::Index(index) = key { return self.indexed_properties.insert(*index, property); } @@ -326,9 +310,6 @@ impl PropertyMap { if let Some(slot) = self.shape.lookup(key) { 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(), @@ -356,7 +337,7 @@ impl PropertyMap { // TODO: maybe take the value instead of cloning, we own it. self.storage[index] = property.expect_value().clone(); } - return Some(old); + return true; } let transition_key = TransitionKey { @@ -385,26 +366,24 @@ impl PropertyMap { self.storage.push(JsValue::undefined()); } - None + false } /// Remove the property with the given key from the [`PropertyMap`]. - pub fn remove(&mut self, key: &PropertyKey) -> Option { + pub fn remove(&mut self, key: &PropertyKey) -> bool { if let PropertyKey::Index(index) = key { return self.indexed_properties.remove(*index); } if let Some(slot) = self.shape.lookup(key) { - // 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); + return true; } - None + false } /// Overrides all the indexed properties, setting it to dense storage. From 2dd45bdb79a4139d24a2003084cf6eba954d4056 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 10:23:23 +0100 Subject: [PATCH 11/19] Fix deletion of a single property object --- boa_engine/src/object/shape/shared_shape.rs | 24 ++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index c4d8b919758..552b5f1ce6f 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -128,6 +128,15 @@ pub(crate) struct SharedShape { impl SharedShape { const DEBUG: bool = false; + fn property_table_count(&self) -> u32 { + if self.previous().is_some() { + return self.property_index() + 1; + } + + // root has no elements + 0 + } + #[inline] pub(crate) fn previous(&self) -> Option<&SharedShape> { self.inner.previous.as_ref() @@ -137,7 +146,7 @@ impl SharedShape { let properties = self.inner.property_table.properties().borrow(); let (key, (index, attributes)) = properties .get_index(self.inner.property_index as usize) - .expect("There should be a key in property table"); + .expect("There should be a property"); (key.clone(), *index, *attributes) } fn transition_type(&self) -> TransitionType { @@ -181,18 +190,12 @@ impl SharedShape { (self.property_index() + 1) * 2 }; - let self_property_count = if self.previous().is_none() { - 0 - } else { - self.property_index() + 1 - }; - let (property_index, property_table) = self.inner.property_table.add_property_deep_clone_if_needed( key.property_key.clone(), key.attributes, slot_index, - self_property_count, + self.property_table_count(), ); let new_inner_shape = Inner { forward_property_transitions: GcRefCell::new(FxHashMap::default()), @@ -313,7 +316,7 @@ impl SharedShape { current = current_shape.previous(); }; - for (property_key, attributes) in IntoIterator::into_iter(transitions).rev() { + for (property_key, attributes) in transitions.into_iter().rev() { let transition = TransitionKey { property_key, attributes, @@ -357,7 +360,7 @@ impl SharedShape { pub(crate) fn keys(&self) -> Vec { let property_table = self.inner.property_table.properties().borrow(); - let property_count = (self.property_index() + 1) as usize; + let property_count = self.property_table_count() as usize; let mut keys: Vec = property_table .keys() @@ -365,6 +368,7 @@ impl SharedShape { .filter(|key| matches!(key, PropertyKey::String(_))) .cloned() .collect(); + keys.extend( property_table .keys() From e1cceba5abefbeb732e3884245cb2757bd5eb273 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 11:13:12 +0100 Subject: [PATCH 12/19] Implement prototype transitions --- boa_engine/src/context/mod.rs | 12 +- boa_engine/src/object/internal_methods/mod.rs | 14 +-- boa_engine/src/object/jsobject.rs | 16 ++- boa_engine/src/object/mod.rs | 12 +- boa_engine/src/object/property_map.rs | 20 +++- boa_engine/src/object/shape/mod.rs | 27 ++++- boa_engine/src/object/shape/shared_shape.rs | 111 ++++++++++++++---- boa_engine/src/object/shape/unique_shape.rs | 34 ++++-- boa_engine/src/value/hash.rs | 2 +- boa_engine/src/vm/opcode/push/object.rs | 2 +- 10 files changed, 186 insertions(+), 64 deletions(-) diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index d2649eb3363..84bc462d367 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -103,6 +103,7 @@ pub struct Context<'host> { job_queue: &'host dyn JobQueue, pub(crate) root_shape: Shape, + pub(crate) empty_object_shape: Shape, } impl std::fmt::Debug for Context<'_> { @@ -631,6 +632,9 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> { { let host_hooks = self.host_hooks.unwrap_or(&DefaultHooks); + let root_shape = Shape::root(); + // let empty_object_shape = root_shape.change_prototype_transition(prototype); + let mut context = Context { realm: Realm::create(host_hooks), interner: self.interner.unwrap_or_default(), @@ -646,10 +650,16 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> { kept_alive: Vec::new(), host_hooks, job_queue: self.job_queue.unwrap_or(&IdleJobQueue), - root_shape: Shape::root(), + // TODO: probably should be moved to realm, maybe into intrinsics. + root_shape: root_shape.clone(), + empty_object_shape: root_shape, }; builtins::set_default_global_bindings(&mut context)?; + let object_prototype = context.intrinsics().constructors().object().prototype(); + context.empty_object_shape = context + .root_shape + .change_prototype_transition(Some(object_prototype)); Ok(context) } diff --git a/boa_engine/src/object/internal_methods/mod.rs b/boa_engine/src/object/internal_methods/mod.rs index fc5680bdf96..37976b2abd8 100644 --- a/boa_engine/src/object/internal_methods/mod.rs +++ b/boa_engine/src/object/internal_methods/mod.rs @@ -348,14 +348,12 @@ pub(crate) fn ordinary_set_prototype_of( _: &mut Context<'_>, ) -> JsResult { // 1. Assert: Either Type(V) is Object or Type(V) is Null. - { - // 2. Let current be O.[[Prototype]]. - let current = obj.prototype(); + // 2. Let current be O.[[Prototype]]. + let current = obj.prototype(); - // 3. If SameValue(V, current) is true, return true. - if val == *current { - return Ok(true); - } + // 3. If SameValue(V, current) is true, return true. + if val == current { + return Ok(true); } // 4. Let extensible be O.[[Extensible]]. @@ -384,7 +382,7 @@ pub(crate) fn ordinary_set_prototype_of( break; } // ii. Else, set p to p.[[Prototype]]. - p = proto.prototype().clone(); + p = proto.prototype(); } // 9. Set O.[[Prototype]] to V. diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index 6b6a47a71cd..d61e843c8e9 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -17,6 +17,7 @@ use std::{ collections::HashMap, error::Error, fmt::{self, Debug, Display}, + hash::Hash, result::Result as StdResult, }; @@ -77,9 +78,8 @@ impl JsObject { Self { inner: Gc::new(GcRefCell::new(Object { data, - prototype: prototype.into(), extensible: true, - properties: PropertyMap::default(), + properties: PropertyMap::from_prototype(prototype.into()), private_elements: Vec::new(), })), } @@ -278,8 +278,8 @@ impl JsObject { /// Panics if the object is currently mutably borrowed. #[inline] #[track_caller] - pub fn prototype(&self) -> Ref<'_, JsPrototype> { - Ref::map(self.borrow(), Object::prototype) + pub fn prototype(&self) -> JsPrototype { + self.borrow().prototype() } /// Get the extensibility of the object. @@ -786,6 +786,14 @@ impl PartialEq for JsObject { } } +impl Eq for JsObject {} + +impl Hash for JsObject { + fn hash(&self, state: &mut H) { + std::ptr::hash(self.as_ref(), state); + } +} + /// An error returned by [`JsObject::try_borrow`](struct.JsObject.html#method.try_borrow). #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct BorrowError; diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 20f0eb510cc..079b94eb7ad 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -124,8 +124,6 @@ pub struct Object { pub data: ObjectData, /// The collection of properties contained in the object properties: PropertyMap, - /// Instance prototype `__proto__`. - prototype: JsPrototype, /// Whether it can have new properties added to it. extensible: bool, /// The `[[PrivateElements]]` internal slot. @@ -136,7 +134,6 @@ unsafe impl Trace for Object { boa_gc::custom_trace!(this, { mark(&this.data); mark(&this.properties); - mark(&this.prototype); for (_, element) in &this.private_elements { mark(element); } @@ -781,7 +778,6 @@ impl Default for Object { Self { data: ObjectData::ordinary(), properties: PropertyMap::default(), - prototype: None, extensible: true, private_elements: Vec::default(), } @@ -1621,8 +1617,8 @@ impl Object { /// Gets the prototype instance of this object. #[inline] - pub const fn prototype(&self) -> &JsPrototype { - &self.prototype + pub fn prototype(&self) -> JsPrototype { + self.properties.shape.prototype() } /// Sets the prototype instance of the object. @@ -1634,12 +1630,12 @@ impl Object { pub fn set_prototype>(&mut self, prototype: O) -> bool { let prototype = prototype.into(); if self.extensible { - self.prototype = prototype; + self.properties.shape = self.properties.shape.change_prototype_transition(prototype); true } else { // If target is non-extensible, [[SetPrototypeOf]] must return false // unless V is the SameValue as the target's observed [[GetPrototypeOf]] value. - self.prototype == prototype + self.prototype() == prototype } } diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index 426cb4a161f..6ef368f7701 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -1,9 +1,9 @@ use super::{ - shape::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey}, - PropertyDescriptor, PropertyKey, + shape::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey, UniqueShape}, + JsPrototype, PropertyDescriptor, PropertyKey, }; use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; -use boa_gc::{custom_trace, Finalize, Trace}; +use boa_gc::{custom_trace, Finalize, GcRefCell, Trace}; use indexmap::IndexMap; use rustc_hash::{FxHashMap, FxHasher}; use std::{collections::hash_map, hash::BuildHasherDefault, iter::FusedIterator}; @@ -235,6 +235,20 @@ impl PropertyMap { } } + /// TOOD: doc + #[must_use] + #[inline] + pub fn from_prototype(prototype: JsPrototype) -> Self { + Self { + indexed_properties: IndexedProperties::default(), + shape: Shape::unique(UniqueShape::new( + GcRefCell::new(prototype), + IndexMap::default(), + )), + storage: Vec::default(), + } + } + /// Get the property with the given key from the [`PropertyMap`]. #[must_use] pub fn get(&self, key: &PropertyKey) -> Option { diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs index 90255539629..51650ac978a 100644 --- a/boa_engine/src/object/shape/mod.rs +++ b/boa_engine/src/object/shape/mod.rs @@ -11,6 +11,8 @@ mod shared_shape; mod unique_shape; +pub(crate) use unique_shape::UniqueShape; + use std::{ any::Any, cell::{Cell, RefCell}, @@ -28,7 +30,7 @@ use crate::{ JsString, }; -use self::{shared_shape::SharedShape, unique_shape::UniqueShape}; +use self::shared_shape::SharedShape; use super::JsPrototype; @@ -75,10 +77,12 @@ unsafe impl Trace for TransitionKey { enum TransitionType { /// Inserts a new property. Insert, - // Change existing property attributes. + + /// Change existing property attributes. Configure, - // TODO: - // Prototype, + + /// Change prototype. + Prototype, } unsafe impl Trace for TransitionType { @@ -87,7 +91,7 @@ unsafe impl Trace for TransitionType { #[derive(Debug, Trace, Finalize, Clone)] enum Inner { - Unique(#[unsafe_ignore_trace] UniqueShape), + Unique(UniqueShape), Shared(SharedShape), } @@ -148,6 +152,19 @@ impl Shape { } } + pub(crate) fn change_prototype_transition(&self, prototype: JsPrototype) -> Self { + match &self.inner { + Inner::Shared(shape) => Self::shared(shape.change_prototype_transition(prototype)), + Inner::Unique(shape) => Self::unique(shape.change_prototype_transition(prototype)), + } + } + pub(crate) fn prototype(&self) -> JsPrototype { + match &self.inner { + Inner::Shared(shape) => shape.prototype(), + Inner::Unique(shape) => shape.prototype(), + } + } + #[inline] pub fn lookup(&self, key: &PropertyKey) -> Option { match &self.inner { diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index 552b5f1ce6f..6a47931903b 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -41,17 +41,22 @@ impl PropertyTable { slot_index: u32, property_count: u32, ) -> (u32, Self) { + // TODO: possible optimization if we are the only ones holding a reference to self we can add the property directly. { let mut properties = self.inner.properties.borrow_mut(); if (property_count as usize) == properties.len() && !properties.contains_key(&key) { - // println!("Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}"); + println!( + "Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}" + ); let (index, value) = properties.insert_full(key, (slot_index, attributes)); debug_assert!(value.is_none()); return (index as u32, self.clone()); } } - // println!("Creating fork PropertyTable with {key} - Slot {slot_index} - PC {property_count}"); + println!( + "Creating fork PropertyTable with {key} - Slot {slot_index} - PC {property_count}" + ); // property is already present need to make deep clone of property table. let this = self.deep_clone(property_count); @@ -108,9 +113,13 @@ impl PropertyTable { #[derive(Debug, Trace, Finalize)] struct Inner { forward_property_transitions: GcRefCell>>, + forward_prototype_transitions: GcRefCell>>, property_index: u32, + /// Instance prototype `__proto__`. + prototype: JsPrototype, + #[unsafe_ignore_trace] property_table: PropertyTable, @@ -126,22 +135,23 @@ pub(crate) struct SharedShape { } impl SharedShape { - const DEBUG: bool = false; + const DEBUG: bool = true; fn property_table_count(&self) -> u32 { - if self.previous().is_some() { - return self.property_index() + 1; + if self.previous().is_none() { + // root has no elements + return 0; } - // root has no elements - 0 + self.property_index() + 1 } - #[inline] pub(crate) fn previous(&self) -> Option<&SharedShape> { self.inner.previous.as_ref() } - #[inline] + pub(crate) fn prototype(&self) -> JsPrototype { + self.inner.prototype.clone() + } pub(crate) fn property(&self) -> (PropertyKey, SlotIndex, PropertyDescriptorAttribute) { let properties = self.inner.property_table.properties().borrow(); let (key, (index, attributes)) = properties @@ -152,18 +162,20 @@ impl SharedShape { fn transition_type(&self) -> TransitionType { self.inner.transition_type } - #[inline] pub(crate) fn is_root(&self) -> bool { self.inner.previous.is_none() } fn property_index(&self) -> u32 { self.inner.property_index } - #[inline] - fn get_forward_transition(&self, key: &TransitionKey) -> Option> { + fn get_forward_property_transition(&self, key: &TransitionKey) -> Option> { let map = self.inner.forward_property_transitions.borrow(); map.get(key).cloned() } + fn get_forward_prototype_transition(&self, prototype: &JsPrototype) -> Option> { + let map = self.inner.forward_prototype_transitions.borrow(); + map.get(prototype).cloned() + } fn new(inner: Inner) -> Self { Self { @@ -171,24 +183,23 @@ impl SharedShape { } } - #[inline] pub(crate) fn root() -> Self { Self::new(Inner { + prototype: None, property_index: 0, property_table: PropertyTable::default(), - forward_property_transitions: GcRefCell::new(FxHashMap::default()), + forward_property_transitions: GcRefCell::default(), + forward_prototype_transitions: GcRefCell::default(), previous: None, transition_type: TransitionType::Insert, }) } fn create_property_transition(&self, key: TransitionKey) -> Self { - let slot_index = if self.previous().is_none() { - 0 - } else { - // TODO: implement different sized storage - (self.property_index() + 1) * 2 - }; + // TODO: implement different sized storage + let slot_index = (self.property_index() + + u32::from(self.transition_type() != TransitionType::Prototype)) + * 2; let (property_index, property_table) = self.inner.property_table.add_property_deep_clone_if_needed( @@ -198,7 +209,9 @@ impl SharedShape { self.property_table_count(), ); let new_inner_shape = Inner { - forward_property_transitions: GcRefCell::new(FxHashMap::default()), + prototype: self.prototype(), + forward_property_transitions: GcRefCell::default(), + forward_prototype_transitions: GcRefCell::default(), property_table, property_index, previous: Some(self.clone()), @@ -212,9 +225,35 @@ impl SharedShape { new_shape } + pub(crate) fn change_prototype_transition(&self, prototype: JsPrototype) -> Self { + if let Some(shape) = self.get_forward_prototype_transition(&prototype) { + if let Some(inner) = shape.upgrade() { + if Self::DEBUG { + println!(" Shape: Resusing previous prototype change shape"); + } + return Self { inner }; + } + } + let new_inner_shape = Inner { + prototype: prototype.clone(), + forward_property_transitions: GcRefCell::default(), + forward_prototype_transitions: GcRefCell::default(), + property_table: self.inner.property_table.clone(), + property_index: self.property_index(), + previous: Some(self.clone()), + transition_type: TransitionType::Prototype, + }; + let new_shape = Self::new(new_inner_shape); + + let mut map = self.inner.forward_prototype_transitions.borrow_mut(); + map.insert(prototype, WeakGc::new(&new_shape.inner)); + + new_shape + } + pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { // Check if we have already creaded such a transition, if so use it! - if let Some(shape) = self.get_forward_transition(&key) { + if let Some(shape) = self.get_forward_property_transition(&key) { if Self::DEBUG { println!("Shape: Trying to resuse previous shape"); } @@ -239,9 +278,11 @@ impl SharedShape { let property_table = self.inner.property_table.deep_clone_all(); property_table.set_attributes(&key.property_key, key.attributes); let new_inner_shape = Inner { + prototype: self.prototype(), property_table, property_index: self.inner.property_index, - forward_property_transitions: GcRefCell::new(FxHashMap::default()), + forward_property_transitions: GcRefCell::default(), + forward_prototype_transitions: GcRefCell::default(), previous: Some(self.clone()), transition_type: TransitionType::Configure, }; @@ -281,15 +322,27 @@ impl SharedShape { println!("Shape: deleting {key}"); } + let mut prototype = None; let mut transitions: IndexMap = IndexMap::default(); - let mut current = Some(self); + let mut current = Some(self); let mut base = loop { let Some(current_shape) = current else { unreachable!("The chain should have insert transition type!") }; + // We only take the latest prototype change it, if it exists. + if current_shape.transition_type() == TransitionType::Prototype { + if prototype.is_none() { + prototype = Some(current_shape.prototype().clone()); + } + + // Skip when it is a prototype transition. + current = current_shape.previous(); + continue; + } + let (current_property_key, _, current_property_attributes) = current_shape.property(); if current_shape.transition_type() == TransitionType::Insert @@ -316,6 +369,11 @@ impl SharedShape { current = current_shape.previous(); }; + // Apply prototype transition, if it was found. + if let Some(prototype) = prototype { + base = base.change_prototype_transition(prototype); + } + for (property_key, attributes) in transitions.into_iter().rev() { let transition = TransitionKey { property_key, @@ -333,8 +391,9 @@ impl SharedShape { println!("Shape: lookup {key}"); } - // Root has not element, return None. - self.previous()?; + if self.property_table_count() == 0 { + return None; + } let property_table = self.inner.property_table.properties().borrow(); if let Some((property_table_index, property_key, (slot_index, attributes))) = diff --git a/boa_engine/src/object/shape/unique_shape.rs b/boa_engine/src/object/shape/unique_shape.rs index adce2ebaa73..899f6536b7c 100644 --- a/boa_engine/src/object/shape/unique_shape.rs +++ b/boa_engine/src/object/shape/unique_shape.rs @@ -20,23 +20,34 @@ use crate::{ use super::{JsPrototype, PropertyDescriptorAttribute, Slot, TransitionKey}; /// TODO: doc -#[derive(Default, Debug)] +#[derive(Default, Debug, Trace, Finalize)] struct Inner { + #[unsafe_ignore_trace] property_table: RefCell>, + + prototype: GcRefCell, } /// TODO: doc -#[derive(Default, Debug, Clone)] +#[derive(Default, Debug, Clone, Trace, Finalize)] pub(crate) struct UniqueShape { - inner: Rc, + inner: Gc, } impl UniqueShape { - /// Create shape from a pre initialized property table. - fn new(property_table: IndexMap) -> Self { + pub(crate) fn prototype(&self) -> JsPrototype { + self.inner.prototype.borrow().clone() + } + + /// Create a new unique shape. + pub(crate) fn new( + prototype: GcRefCell, + property_table: IndexMap, + ) -> Self { Self { - inner: Rc::new(Inner { + inner: Gc::new(Inner { property_table: property_table.into(), + prototype, }), } } @@ -71,7 +82,8 @@ impl UniqueShape { // Therefore we need to create a new unique shape, // to invalidate any pointers to this shape i.e inline caches. let property_table = std::mem::take(&mut *property_table); - Self::new(property_table) + let prototype = self.inner.prototype.clone(); + Self::new(prototype, property_table) } /// TODO: doc @@ -99,6 +111,14 @@ impl UniqueShape { } self.clone() } + pub(crate) fn change_prototype_transition(&self, prototype: JsPrototype) -> Self { + let mut property_table = self.inner.property_table.borrow_mut(); + + // We need to create a new unique shape, + // to invalidate any pointers to this shape i.e inline caches. + let property_table = std::mem::take(&mut *property_table); + Self::new(GcRefCell::new(prototype), property_table) + } /// Gets all keys first strings then symbols. pub(crate) fn keys(&self) -> Vec { diff --git a/boa_engine/src/value/hash.rs b/boa_engine/src/value/hash.rs index 6536b1a77ae..c657037b1cd 100644 --- a/boa_engine/src/value/hash.rs +++ b/boa_engine/src/value/hash.rs @@ -45,7 +45,7 @@ impl Hash for JsValue { Self::BigInt(ref bigint) => bigint.hash(state), Self::Rational(rational) => RationalHashable(*rational).hash(state), Self::Symbol(ref symbol) => Hash::hash(symbol, state), - Self::Object(ref object) => std::ptr::hash(object.as_ref(), state), + Self::Object(ref object) => object.hash(state), } } } diff --git a/boa_engine/src/vm/opcode/push/object.rs b/boa_engine/src/vm/opcode/push/object.rs index 7a81c96e986..25a5a38cc06 100644 --- a/boa_engine/src/vm/opcode/push/object.rs +++ b/boa_engine/src/vm/opcode/push/object.rs @@ -17,7 +17,7 @@ impl Operation for PushEmptyObject { fn execute(context: &mut Context<'_>) -> JsResult { let o = JsObject::with_object_proto(context); - o.set_shape(context.root_shape.clone()); + o.set_shape(context.empty_object_shape.clone()); context.vm.push(o); Ok(CompletionType::Normal) } From 4ea5ada8c4a79eb87e5bafd42a21e5eb759191b1 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 16:03:29 +0100 Subject: [PATCH 13/19] Store property count instead of index --- boa_engine/src/object/jsobject.rs | 9 ++++ boa_engine/src/object/shape/shared_shape.rs | 52 ++++++++++----------- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index d61e843c8e9..ee53495faa9 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -55,6 +55,15 @@ impl JsObject { ) } + // #[inline] + // #[must_use] + // pub fn with_shape(shape: Shape) -> Self { + // Self::from_proto_and_data( + // context.intrinsics().constructors().object().prototype(), + // ObjectData::ordinary(), + // ) + // } + /// Creates a new ordinary object, with its prototype set to null. /// /// This is equivalent to calling the specification's abstract operation diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index 6a47931903b..41e4404adcf 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -115,7 +115,7 @@ struct Inner { forward_property_transitions: GcRefCell>>, forward_prototype_transitions: GcRefCell>>, - property_index: u32, + property_count: u32, /// Instance prototype `__proto__`. prototype: JsPrototype, @@ -137,13 +137,13 @@ pub(crate) struct SharedShape { impl SharedShape { const DEBUG: bool = true; - fn property_table_count(&self) -> u32 { - if self.previous().is_none() { - // root has no elements - return 0; - } - - self.property_index() + 1 + /// Return the property count that this shape owns in the [`PropertyTable`]. + fn property_count(&self) -> u32 { + self.inner.property_count + } + /// Return the index to the property in the the [`PropertyTable`]. + fn property_index(&self) -> u32 { + self.inner.property_count.saturating_sub(1) } pub(crate) fn previous(&self) -> Option<&SharedShape> { @@ -155,7 +155,7 @@ impl SharedShape { pub(crate) fn property(&self) -> (PropertyKey, SlotIndex, PropertyDescriptorAttribute) { let properties = self.inner.property_table.properties().borrow(); let (key, (index, attributes)) = properties - .get_index(self.inner.property_index as usize) + .get_index(self.property_index() as usize) .expect("There should be a property"); (key.clone(), *index, *attributes) } @@ -165,9 +165,6 @@ impl SharedShape { pub(crate) fn is_root(&self) -> bool { self.inner.previous.is_none() } - fn property_index(&self) -> u32 { - self.inner.property_index - } fn get_forward_property_transition(&self, key: &TransitionKey) -> Option> { let map = self.inner.forward_property_transitions.borrow(); map.get(key).cloned() @@ -186,7 +183,7 @@ impl SharedShape { pub(crate) fn root() -> Self { Self::new(Inner { prototype: None, - property_index: 0, + property_count: 0, property_table: PropertyTable::default(), forward_property_transitions: GcRefCell::default(), forward_prototype_transitions: GcRefCell::default(), @@ -196,9 +193,8 @@ impl SharedShape { } fn create_property_transition(&self, key: TransitionKey) -> Self { - // TODO: implement different sized storage - let slot_index = (self.property_index() - + u32::from(self.transition_type() != TransitionType::Prototype)) + let slot_index = self.property_count() + // TODO: implement different sized storage * 2; let (property_index, property_table) = @@ -206,14 +202,14 @@ impl SharedShape { key.property_key.clone(), key.attributes, slot_index, - self.property_table_count(), + self.property_count(), ); let new_inner_shape = Inner { prototype: self.prototype(), forward_property_transitions: GcRefCell::default(), forward_prototype_transitions: GcRefCell::default(), property_table, - property_index, + property_count: self.property_count() + 1, previous: Some(self.clone()), transition_type: TransitionType::Insert, }; @@ -239,7 +235,7 @@ impl SharedShape { forward_property_transitions: GcRefCell::default(), forward_prototype_transitions: GcRefCell::default(), property_table: self.inner.property_table.clone(), - property_index: self.property_index(), + property_count: self.property_count(), previous: Some(self.clone()), transition_type: TransitionType::Prototype, }; @@ -280,7 +276,7 @@ impl SharedShape { let new_inner_shape = Inner { prototype: self.prototype(), property_table, - property_index: self.inner.property_index, + property_count: self.property_count(), forward_property_transitions: GcRefCell::default(), forward_prototype_transitions: GcRefCell::default(), previous: Some(self.clone()), @@ -391,7 +387,8 @@ impl SharedShape { println!("Shape: lookup {key}"); } - if self.property_table_count() == 0 { + let property_count = self.property_count(); + if property_count == 0 { return None; } @@ -405,13 +402,12 @@ impl SharedShape { println!("Key {key}, PTI: {property_table_index}, SI: {slot_index}"); } // Check if we are trying to access properties that belong to another shape. - if (property_table_index as u32) > self.inner.property_index { - return None; + if (property_table_index as u32) < self.property_count() { + return Some(Slot { + index: *slot_index, + attributes: *attributes, + }); } - return Some(Slot { - index: *slot_index, - attributes: *attributes, - }); } None } @@ -419,7 +415,7 @@ impl SharedShape { pub(crate) fn keys(&self) -> Vec { let property_table = self.inner.property_table.properties().borrow(); - let property_count = self.property_table_count() as usize; + let property_count = self.property_count() as usize; let mut keys: Vec = property_table .keys() From ffaa3010fe8450dfae577257d72015d9a56c129e Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 20:17:06 +0100 Subject: [PATCH 14/19] Disable debug flag --- boa_engine/src/object/shape/shared_shape.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape.rs index 41e4404adcf..d9b7d871fc7 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape.rs @@ -42,21 +42,26 @@ impl PropertyTable { property_count: u32, ) -> (u32, Self) { // TODO: possible optimization if we are the only ones holding a reference to self we can add the property directly. + // TOOD: possible optimization if we already have the property in the exact position we want it to be with the same properties, + // then just return self, this might happen, if a branch is created than after not being used gets gc collected we still have the + // properties here. + // TODO: possible optimization, figure out if there **always** are as many properties as there are strong references. If so truncate + // on drop Rc ref drop, maybe? { let mut properties = self.inner.properties.borrow_mut(); if (property_count as usize) == properties.len() && !properties.contains_key(&key) { - println!( - "Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}" - ); + // println!( + // "Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}" + // ); let (index, value) = properties.insert_full(key, (slot_index, attributes)); debug_assert!(value.is_none()); return (index as u32, self.clone()); } } - println!( - "Creating fork PropertyTable with {key} - Slot {slot_index} - PC {property_count}" - ); + // println!( + // "Creating fork PropertyTable with {key} - Slot {slot_index} - PC {property_count}" + // ); // property is already present need to make deep clone of property table. let this = self.deep_clone(property_count); @@ -135,7 +140,7 @@ pub(crate) struct SharedShape { } impl SharedShape { - const DEBUG: bool = true; + const DEBUG: bool = false; /// Return the property count that this shape owns in the [`PropertyTable`]. fn property_count(&self) -> u32 { From 473f6f543a9c813cb08bdbeb83ddcfbfa5fb36ff Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 20:47:49 +0100 Subject: [PATCH 15/19] Use slot where ever possible --- boa_engine/src/object/property_map.rs | 2 +- boa_engine/src/object/shape/mod.rs | 9 +--- .../{shared_shape.rs => shared_shape/mod.rs} | 53 ++++++++++--------- boa_engine/src/object/shape/slot.rs | 9 ++++ 4 files changed, 41 insertions(+), 32 deletions(-) rename boa_engine/src/object/shape/{shared_shape.rs => shared_shape/mod.rs} (91%) create mode 100644 boa_engine/src/object/shape/slot.rs diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index 6ef368f7701..099746a9ca0 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -1,5 +1,5 @@ use super::{ - shape::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey, UniqueShape}, + shape::{slot::Slot, PropertyDescriptorAttribute, Shape, TransitionKey, UniqueShape}, JsPrototype, PropertyDescriptor, PropertyKey, }; use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs index 51650ac978a..f5163fcbb33 100644 --- a/boa_engine/src/object/shape/mod.rs +++ b/boa_engine/src/object/shape/mod.rs @@ -9,6 +9,7 @@ //! TODO: doc mod shared_shape; +pub(crate) mod slot; mod unique_shape; pub(crate) use unique_shape::UniqueShape; @@ -30,7 +31,7 @@ use crate::{ JsString, }; -use self::shared_shape::SharedShape; +use self::{shared_shape::SharedShape, slot::Slot}; use super::JsPrototype; @@ -95,12 +96,6 @@ enum Inner { Shared(SharedShape), } -#[derive(Debug, Clone, Copy)] -pub struct Slot { - pub index: u32, - pub attributes: PropertyDescriptorAttribute, -} - #[derive(Debug, Trace, Finalize, Clone)] pub struct Shape { inner: Inner, diff --git a/boa_engine/src/object/shape/shared_shape.rs b/boa_engine/src/object/shape/shared_shape/mod.rs similarity index 91% rename from boa_engine/src/object/shape/shared_shape.rs rename to boa_engine/src/object/shape/shared_shape/mod.rs index d9b7d871fc7..9ee24d973b1 100644 --- a/boa_engine/src/object/shape/shared_shape.rs +++ b/boa_engine/src/object/shape/shared_shape/mod.rs @@ -13,13 +13,13 @@ use crate::{ property::{Attribute, PropertyDescriptor, PropertyKey}, }; -use super::{PropertyDescriptorAttribute, Shape, Slot, TransitionKey, TransitionType}; - -type SlotIndex = u32; +use super::{ + slot::SlotIndex, PropertyDescriptorAttribute, Shape, Slot, TransitionKey, TransitionType, +}; #[derive(Default, Debug, Clone)] struct PropertyTableInner { - properties: RefCell>, + properties: RefCell>, } #[derive(Default, Debug, Clone)] @@ -28,9 +28,7 @@ struct PropertyTable { } impl PropertyTable { - fn properties( - &self, - ) -> &RefCell> { + fn properties(&self) -> &RefCell> { &self.inner.properties } @@ -53,7 +51,13 @@ impl PropertyTable { // println!( // "Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}" // ); - let (index, value) = properties.insert_full(key, (slot_index, attributes)); + let (index, value) = properties.insert_full( + key, + Slot { + index: slot_index, + attributes, + }, + ); debug_assert!(value.is_none()); return (index as u32, self.clone()); } @@ -67,7 +71,13 @@ impl PropertyTable { let this = self.deep_clone(property_count); let index = { let mut properties = this.inner.properties.borrow_mut(); - let (index, value) = properties.insert_full(key, (slot_index, attributes)); + let (index, value) = properties.insert_full( + key, + Slot { + index: slot_index, + attributes, + }, + ); debug_assert!(value.is_none()); index }; @@ -84,7 +94,7 @@ impl PropertyTable { properties .iter() .take(count) - .map(|(key, (index, attributes))| (key.clone(), (*index, *attributes))), + .map(|(key, slot)| (key.clone(), *slot)), ); result }; @@ -107,7 +117,7 @@ impl PropertyTable { property_attributes: PropertyDescriptorAttribute, ) { let mut properties = self.inner.properties.borrow_mut(); - let Some((_index, attributes)) = properties.get_mut(property_key) else { + let Some(Slot { attributes, .. }) = properties.get_mut(property_key) else { unreachable!("There should already be a property!") }; *attributes = property_attributes; @@ -157,12 +167,12 @@ impl SharedShape { pub(crate) fn prototype(&self) -> JsPrototype { self.inner.prototype.clone() } - pub(crate) fn property(&self) -> (PropertyKey, SlotIndex, PropertyDescriptorAttribute) { + pub(crate) fn property(&self) -> (PropertyKey, Slot) { let properties = self.inner.property_table.properties().borrow(); - let (key, (index, attributes)) = properties + let (key, slot) = properties .get_index(self.property_index() as usize) .expect("There should be a property"); - (key.clone(), *index, *attributes) + (key.clone(), *slot) } fn transition_type(&self) -> TransitionType { self.inner.transition_type @@ -344,7 +354,7 @@ impl SharedShape { continue; } - let (current_property_key, _, current_property_attributes) = current_shape.property(); + let (current_property_key, slot) = current_shape.property(); if current_shape.transition_type() == TransitionType::Insert && ¤t_property_key == key @@ -364,7 +374,7 @@ impl SharedShape { // Only take the latest changes to a property. To try to build a smaller tree. transitions .entry(current_property_key) - .or_insert(current_property_attributes); + .or_insert(slot.attributes); } current = current_shape.previous(); @@ -398,20 +408,15 @@ impl SharedShape { } let property_table = self.inner.property_table.properties().borrow(); - if let Some((property_table_index, property_key, (slot_index, attributes))) = - property_table.get_full(key) - { + if let Some((property_table_index, property_key, slot)) = property_table.get_full(key) { debug_assert_eq!(key, property_key); if Self::DEBUG { - println!("Key {key}, PTI: {property_table_index}, SI: {slot_index}"); + println!("Key {key}, PTI: {property_table_index}, SI: {}", slot.index); } // Check if we are trying to access properties that belong to another shape. if (property_table_index as u32) < self.property_count() { - return Some(Slot { - index: *slot_index, - attributes: *attributes, - }); + return Some(*slot); } } None diff --git a/boa_engine/src/object/shape/slot.rs b/boa_engine/src/object/shape/slot.rs new file mode 100644 index 00000000000..5251656e0ba --- /dev/null +++ b/boa_engine/src/object/shape/slot.rs @@ -0,0 +1,9 @@ +use super::PropertyDescriptorAttribute; + +pub(crate) type SlotIndex = u32; + +#[derive(Debug, Clone, Copy)] +pub struct Slot { + pub index: SlotIndex, + pub attributes: PropertyDescriptorAttribute, +} From b4aad429ce58d5c354ee174ddceb12361c199ae7 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 21:17:37 +0100 Subject: [PATCH 16/19] Refactor into files --- boa_engine/src/object/property_map.rs | 47 ++----- boa_engine/src/object/shape/mod.rs | 64 +-------- .../src/object/shape/shared_shape/mod.rs | 128 ++++-------------- .../shape/shared_shape/property_table.rs | 115 ++++++++++++++++ boa_engine/src/object/shape/slot.rs | 32 ++++- boa_engine/src/object/shape/unique_shape.rs | 6 +- 6 files changed, 195 insertions(+), 197 deletions(-) create mode 100644 boa_engine/src/object/shape/shared_shape/property_table.rs diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index 099746a9ca0..ff2a6bb10ee 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -1,5 +1,9 @@ use super::{ - shape::{slot::Slot, PropertyDescriptorAttribute, Shape, TransitionKey, UniqueShape}, + shape::{ + shared_shape::TransitionKey, + slot::{Slot, SlotAttribute}, + Shape, UniqueShape, + }, JsPrototype, PropertyDescriptor, PropertyKey, }; use crate::{property::PropertyDescriptorBuilder, JsString, JsSymbol, JsValue}; @@ -8,10 +12,6 @@ use indexmap::IndexMap; use rustc_hash::{FxHashMap, FxHasher}; use std::{collections::hash_map, hash::BuildHasherDefault, iter::FusedIterator}; -// /// Type alias to make it easier to work with the string properties on the global object. -// pub(crate) type GlobalPropertyMap = -// IndexMap>; - /// Wrapper around `indexmap::IndexMap` for usage in `PropertyMap`. #[derive(Debug, Finalize)] struct OrderedHashMap(IndexMap>); @@ -267,8 +267,8 @@ impl PropertyMap { 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)); + .configurable(attributes.contains(SlotAttribute::CONFIGURABLE)) + .enumerable(attributes.contains(SlotAttribute::ENUMERABLE)); if attributes.is_accessor_descriptor() { if attributes.has_get() { builder = builder.get(self.storage[index].clone()); @@ -277,7 +277,7 @@ impl PropertyMap { builder = builder.set(self.storage[index + 1].clone()); } } else { - builder = builder.writable(attributes.contains(PropertyDescriptorAttribute::WRITABLE)); + builder = builder.writable(attributes.contains(SlotAttribute::WRITABLE)); builder = builder.value(self.storage[index].clone()); } builder.build() @@ -292,35 +292,18 @@ impl PropertyMap { } // 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(), - ); + let mut attributes = SlotAttribute::empty(); + attributes.set(SlotAttribute::CONFIGURABLE, property.expect_configurable()); + attributes.set(SlotAttribute::ENUMERABLE, property.expect_enumerable()); if property.is_data_descriptor() { - attributes.set( - PropertyDescriptorAttribute::WRITABLE, - property.expect_writable(), - ); + attributes.set(SlotAttribute::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(), - ); + attributes.set(SlotAttribute::HAS_GET, property.get().is_some()); + attributes.set(SlotAttribute::HAS_SET, property.set().is_some()); } else { unreachable!() } - // println!("Attributes: {key} IN {attributes:?}"); - if let Some(slot) = self.shape.lookup(key) { let index = slot.index as usize; @@ -329,7 +312,7 @@ impl PropertyMap { property_key: key.clone(), attributes, }; - self.shape = self.shape.change_attributes_transition(key, slot.index); + self.shape = self.shape.change_attributes_transition(key); } if attributes.is_accessor_descriptor() { diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs index f5163fcbb33..781ec099eed 100644 --- a/boa_engine/src/object/shape/mod.rs +++ b/boa_engine/src/object/shape/mod.rs @@ -8,7 +8,7 @@ //! TODO: doc -mod shared_shape; +pub(crate) mod shared_shape; pub(crate) mod slot; mod unique_shape; @@ -31,65 +31,13 @@ use crate::{ JsString, }; -use self::{shared_shape::SharedShape, slot::Slot}; +use self::{ + shared_shape::{SharedShape, TransitionKey}, + slot::Slot, +}; 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; - } -} - -impl PropertyDescriptorAttribute { - pub const fn is_accessor_descriptor(self) -> bool { - self.contains(Self::HAS_GET) || self.contains(Self::HAS_SET) - } - pub const fn is_data_descriptor(self) -> bool { - !self.is_accessor_descriptor() - } - - pub const fn has_get(self) -> bool { - self.contains(Self::HAS_GET) - } - pub const fn has_set(self) -> bool { - self.contains(Self::HAS_SET) - } -} - -#[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] -pub(crate) struct TransitionKey { - pub(crate) property_key: PropertyKey, - pub(crate) attributes: PropertyDescriptorAttribute, -} - -// SAFETY: non of the member of this struct are garbage collected, -// so this should be fine. -unsafe impl Trace for TransitionKey { - empty_trace!(); -} - -#[derive(Debug, Finalize, Clone, Copy, PartialEq, Eq)] -enum TransitionType { - /// Inserts a new property. - Insert, - - /// Change existing property attributes. - Configure, - - /// Change prototype. - Prototype, -} - -unsafe impl Trace for TransitionType { - empty_trace!(); -} - #[derive(Debug, Trace, Finalize, Clone)] enum Inner { Unique(UniqueShape), @@ -133,7 +81,7 @@ impl Shape { } } - pub(crate) fn change_attributes_transition(&self, key: TransitionKey, index: u32) -> 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)), diff --git a/boa_engine/src/object/shape/shared_shape/mod.rs b/boa_engine/src/object/shape/shared_shape/mod.rs index 9ee24d973b1..3b3e931e31e 100644 --- a/boa_engine/src/object/shape/shared_shape/mod.rs +++ b/boa_engine/src/object/shape/shared_shape/mod.rs @@ -1,10 +1,12 @@ +pub(crate) mod property_table; + use std::{ cell::{Cell, RefCell}, collections::hash_map::RandomState, rc::Rc, }; -use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; +use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace, WeakGc}; use indexmap::{IndexMap, IndexSet}; use rustc_hash::FxHashMap; @@ -13,115 +15,39 @@ use crate::{ property::{Attribute, PropertyDescriptor, PropertyKey}, }; +use self::property_table::PropertyTable; + use super::{ - slot::SlotIndex, PropertyDescriptorAttribute, Shape, Slot, TransitionKey, TransitionType, + slot::{SlotAttribute, SlotIndex}, + Shape, Slot, }; -#[derive(Default, Debug, Clone)] -struct PropertyTableInner { - properties: RefCell>, +#[derive(Debug, Finalize, Clone, PartialEq, Eq, Hash)] +pub(crate) struct TransitionKey { + pub(crate) property_key: PropertyKey, + pub(crate) attributes: SlotAttribute, } -#[derive(Default, Debug, Clone)] -struct PropertyTable { - inner: Rc, +// SAFETY: non of the member of this struct are garbage collected, +// so this should be fine. +unsafe impl Trace for TransitionKey { + empty_trace!(); } -impl PropertyTable { - fn properties(&self) -> &RefCell> { - &self.inner.properties - } +#[derive(Debug, Finalize, Clone, Copy, PartialEq, Eq)] +pub(crate) enum TransitionType { + /// Inserts a new property. + Insert, - fn add_property_deep_clone_if_needed( - &self, - key: PropertyKey, - attributes: PropertyDescriptorAttribute, - slot_index: u32, - property_count: u32, - ) -> (u32, Self) { - // TODO: possible optimization if we are the only ones holding a reference to self we can add the property directly. - // TOOD: possible optimization if we already have the property in the exact position we want it to be with the same properties, - // then just return self, this might happen, if a branch is created than after not being used gets gc collected we still have the - // properties here. - // TODO: possible optimization, figure out if there **always** are as many properties as there are strong references. If so truncate - // on drop Rc ref drop, maybe? - { - let mut properties = self.inner.properties.borrow_mut(); - if (property_count as usize) == properties.len() && !properties.contains_key(&key) { - // println!( - // "Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}" - // ); - let (index, value) = properties.insert_full( - key, - Slot { - index: slot_index, - attributes, - }, - ); - debug_assert!(value.is_none()); - return (index as u32, self.clone()); - } - } + /// Change existing property attributes. + Configure, - // println!( - // "Creating fork PropertyTable with {key} - Slot {slot_index} - PC {property_count}" - // ); - - // property is already present need to make deep clone of property table. - let this = self.deep_clone(property_count); - let index = { - let mut properties = this.inner.properties.borrow_mut(); - let (index, value) = properties.insert_full( - key, - Slot { - index: slot_index, - attributes, - }, - ); - debug_assert!(value.is_none()); - index - }; - (index as u32, this) - } - - fn deep_clone(&self, count: u32) -> Self { - let count = count as usize; - - let properties = { - let mut properties = self.inner.properties.borrow(); - let mut result = IndexMap::default(); - result.extend( - properties - .iter() - .take(count) - .map(|(key, slot)| (key.clone(), *slot)), - ); - result - }; - Self { - inner: Rc::new(PropertyTableInner { - properties: RefCell::new(properties), - }), - } - } - - fn deep_clone_all(&self) -> Self { - Self { - inner: Rc::new((*self.inner).clone()), - } - } + /// Change prototype. + Prototype, +} - fn set_attributes( - &self, - property_key: &PropertyKey, - property_attributes: PropertyDescriptorAttribute, - ) { - let mut properties = self.inner.properties.borrow_mut(); - let Some(Slot { attributes, .. }) = properties.get_mut(property_key) else { - unreachable!("There should already be a property!") - }; - *attributes = property_attributes; - } +unsafe impl Trace for TransitionType { + empty_trace!(); } /// TODO: doc @@ -334,7 +260,7 @@ impl SharedShape { } let mut prototype = None; - let mut transitions: IndexMap = + let mut transitions: IndexMap = IndexMap::default(); let mut current = Some(self); diff --git a/boa_engine/src/object/shape/shared_shape/property_table.rs b/boa_engine/src/object/shape/shared_shape/property_table.rs new file mode 100644 index 00000000000..fd5daa26fa0 --- /dev/null +++ b/boa_engine/src/object/shape/shared_shape/property_table.rs @@ -0,0 +1,115 @@ +use std::{cell::RefCell, rc::Rc}; + +use indexmap::IndexMap; + +use crate::{ + object::shape::slot::{Slot, SlotAttribute}, + property::PropertyKey, +}; + +#[derive(Default, Debug, Clone)] +struct Inner { + properties: RefCell>, +} + +#[derive(Default, Debug, Clone)] +pub(crate) struct PropertyTable { + inner: Rc, +} + +impl PropertyTable { + pub(crate) fn properties(&self) -> &RefCell> { + &self.inner.properties + } + + pub(crate) fn add_property_deep_clone_if_needed( + &self, + key: PropertyKey, + attributes: SlotAttribute, + slot_index: u32, + property_count: u32, + ) -> (u32, Self) { + // TODO: possible optimization if we are the only ones holding a reference to self we can add the property directly. + // TOOD: possible optimization if we already have the property in the exact position we want it to be with the same properties, + // then just return self, this might happen, if a branch is created than after not being used gets gc collected we still have the + // properties here. + // TODO: possible optimization, figure out if there **always** are as many properties as there are strong references. If so truncate + // on drop Rc ref drop, maybe? + { + let mut properties = self.inner.properties.borrow_mut(); + if (property_count as usize) == properties.len() && !properties.contains_key(&key) { + // println!( + // "Extending PropertyTable with {key} - Slot {slot_index} - PC {property_count}" + // ); + let (index, value) = properties.insert_full( + key, + Slot { + index: slot_index, + attributes, + }, + ); + debug_assert!(value.is_none()); + return (index as u32, self.clone()); + } + } + + // println!( + // "Creating fork PropertyTable with {key} - Slot {slot_index} - PC {property_count}" + // ); + + // property is already present need to make deep clone of property table. + let this = self.deep_clone(property_count); + let index = { + let mut properties = this.inner.properties.borrow_mut(); + let (index, value) = properties.insert_full( + key, + Slot { + index: slot_index, + attributes, + }, + ); + debug_assert!(value.is_none()); + index + }; + (index as u32, this) + } + + pub(crate) fn deep_clone(&self, count: u32) -> Self { + let count = count as usize; + + let properties = { + let mut properties = self.inner.properties.borrow(); + let mut result = IndexMap::default(); + result.extend( + properties + .iter() + .take(count) + .map(|(key, slot)| (key.clone(), *slot)), + ); + result + }; + Self { + inner: Rc::new(Inner { + properties: RefCell::new(properties), + }), + } + } + + pub(crate) fn deep_clone_all(&self) -> Self { + Self { + inner: Rc::new((*self.inner).clone()), + } + } + + pub(crate) fn set_attributes( + &self, + property_key: &PropertyKey, + property_attributes: SlotAttribute, + ) { + let mut properties = self.inner.properties.borrow_mut(); + let Some(Slot { attributes, .. }) = properties.get_mut(property_key) else { + unreachable!("There should already be a property!") + }; + *attributes = property_attributes; + } +} diff --git a/boa_engine/src/object/shape/slot.rs b/boa_engine/src/object/shape/slot.rs index 5251656e0ba..1580d03e47a 100644 --- a/boa_engine/src/object/shape/slot.rs +++ b/boa_engine/src/object/shape/slot.rs @@ -1,9 +1,35 @@ -use super::PropertyDescriptorAttribute; - +use bitflags::bitflags; pub(crate) type SlotIndex = u32; +bitflags! { + #[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Hash)] + pub struct SlotAttribute: u8 { + const WRITABLE = 0b0000_0001; + const ENUMERABLE = 0b0000_0010; + const CONFIGURABLE = 0b0000_0100; + const HAS_GET = 0b0000_1000; + const HAS_SET = 0b0001_0000; + } +} + +impl SlotAttribute { + pub const fn is_accessor_descriptor(self) -> bool { + self.contains(Self::HAS_GET) || self.contains(Self::HAS_SET) + } + pub const fn is_data_descriptor(self) -> bool { + !self.is_accessor_descriptor() + } + + pub const fn has_get(self) -> bool { + self.contains(Self::HAS_GET) + } + pub const fn has_set(self) -> bool { + self.contains(Self::HAS_SET) + } +} + #[derive(Debug, Clone, Copy)] pub struct Slot { pub index: SlotIndex, - pub attributes: PropertyDescriptorAttribute, + pub attributes: SlotAttribute, } diff --git a/boa_engine/src/object/shape/unique_shape.rs b/boa_engine/src/object/shape/unique_shape.rs index 899f6536b7c..b83435568ff 100644 --- a/boa_engine/src/object/shape/unique_shape.rs +++ b/boa_engine/src/object/shape/unique_shape.rs @@ -17,13 +17,13 @@ use crate::{ JsString, }; -use super::{JsPrototype, PropertyDescriptorAttribute, Slot, TransitionKey}; +use super::{shared_shape::TransitionKey, slot::SlotAttribute, JsPrototype, Slot}; /// TODO: doc #[derive(Default, Debug, Trace, Finalize)] struct Inner { #[unsafe_ignore_trace] - property_table: RefCell>, + property_table: RefCell>, prototype: GcRefCell, } @@ -42,7 +42,7 @@ impl UniqueShape { /// Create a new unique shape. pub(crate) fn new( prototype: GcRefCell, - property_table: IndexMap, + property_table: IndexMap, ) -> Self { Self { inner: Gc::new(Inner { From 15c52d387ddd654ee9ea7346d767576e89f14115 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 21:49:01 +0100 Subject: [PATCH 17/19] Make all arrays use the same shape --- boa_engine/src/builtins/array/mod.rs | 8 +++++- boa_engine/src/object/jsobject.rs | 36 +++++++++++++++++++-------- boa_engine/src/object/property_map.rs | 14 ++++++++++- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/boa_engine/src/builtins/array/mod.rs b/boa_engine/src/builtins/array/mod.rs index bfea05fe982..de306a0d80f 100644 --- a/boa_engine/src/builtins/array/mod.rs +++ b/boa_engine/src/builtins/array/mod.rs @@ -245,7 +245,13 @@ impl Array { // 5. Set A.[[DefineOwnProperty]] as specified in 10.4.2.1. let prototype = prototype.unwrap_or_else(|| context.intrinsics().constructors().array().prototype()); - let array = JsObject::from_proto_and_data(prototype, ObjectData::array()); + + // TODO: possible optimization: directly initialize with pre-initialized array instance shape. + let array = JsObject::from_proto_and_data_with_shared_shape( + context.root_shape.clone(), + prototype, + ObjectData::array(), + ); // 6. Perform ! OrdinaryDefineOwnProperty(A, "length", PropertyDescriptor { [[Value]]: 𝔽(length), [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: false }). crate::object::internal_methods::ordinary_define_own_property( diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index ee53495faa9..e5e85c6ecda 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -55,15 +55,6 @@ impl JsObject { ) } - // #[inline] - // #[must_use] - // pub fn with_shape(shape: Shape) -> Self { - // Self::from_proto_and_data( - // context.intrinsics().constructors().object().prototype(), - // ObjectData::ordinary(), - // ) - // } - /// Creates a new ordinary object, with its prototype set to null. /// /// This is equivalent to calling the specification's abstract operation @@ -88,7 +79,32 @@ impl JsObject { inner: Gc::new(GcRefCell::new(Object { data, extensible: true, - properties: PropertyMap::from_prototype(prototype.into()), + properties: PropertyMap::from_prototype_unique_shape(prototype.into()), + private_elements: Vec::new(), + })), + } + } + + /// Creates a new object with the provided prototype and object data. + /// + /// This is equivalent to calling the specification's abstract operation [`OrdinaryObjectCreate`], + /// with the difference that the `additionalInternalSlotsList` parameter is automatically set by + /// the [`ObjectData`] provided. + /// + /// [`OrdinaryObjectCreate`]: https://tc39.es/ecma262/#sec-ordinaryobjectcreate + pub fn from_proto_and_data_with_shared_shape>>( + root_shape: Shape, + prototype: O, + data: ObjectData, + ) -> Self { + Self { + inner: Gc::new(GcRefCell::new(Object { + data, + extensible: true, + properties: PropertyMap::from_prototype_with_shared_shape( + root_shape, + prototype.into(), + ), private_elements: Vec::new(), })), } diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index ff2a6bb10ee..9e256f365c1 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -238,7 +238,7 @@ impl PropertyMap { /// TOOD: doc #[must_use] #[inline] - pub fn from_prototype(prototype: JsPrototype) -> Self { + pub fn from_prototype_unique_shape(prototype: JsPrototype) -> Self { Self { indexed_properties: IndexedProperties::default(), shape: Shape::unique(UniqueShape::new( @@ -249,6 +249,18 @@ impl PropertyMap { } } + /// TOOD: doc + #[must_use] + #[inline] + pub fn from_prototype_with_shared_shape(mut shape: Shape, prototype: JsPrototype) -> Self { + shape = shape.change_prototype_transition(prototype); + Self { + indexed_properties: IndexedProperties::default(), + shape, + storage: Vec::default(), + } + } + /// Get the property with the given key from the [`PropertyMap`]. #[must_use] pub fn get(&self, key: &PropertyKey) -> Option { From 752a0109ec5b2a4bfffe56a2e4cf35b870281df8 Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sat, 25 Mar 2023 23:33:37 +0100 Subject: [PATCH 18/19] Abstract forwad transitions --- .../shape/shared_shape/forward_transition.rs | 32 +++++++++ .../src/object/shape/shared_shape/mod.rs | 66 +++++++++++-------- 2 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 boa_engine/src/object/shape/shared_shape/forward_transition.rs diff --git a/boa_engine/src/object/shape/shared_shape/forward_transition.rs b/boa_engine/src/object/shape/shared_shape/forward_transition.rs new file mode 100644 index 00000000000..6c7e6703110 --- /dev/null +++ b/boa_engine/src/object/shape/shared_shape/forward_transition.rs @@ -0,0 +1,32 @@ +use std::hash::Hash; + +use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; +use rustc_hash::FxHashMap; + +use super::Inner as SharedShapeInner; + +/// Holds a forward reference to a previously created transition. +/// The reference is weak, therefore it can be garbage collected if it is not in use. +#[derive(Debug, Clone, Trace, Finalize)] +pub(super) struct ForwardTransition { + transitions: GcRefCell>>, +} + +impl Default for ForwardTransition { + fn default() -> Self { + Self { + transitions: GcRefCell::default(), + } + } +} + +impl ForwardTransition { + pub(super) fn insert(&self, key: T, value: &Gc) { + let mut transitions = self.transitions.borrow_mut(); + transitions.insert(key, WeakGc::new(value)); + } + pub(super) fn get(&self, key: &T) -> Option> { + let transitions = self.transitions.borrow(); + transitions.get(key).cloned() + } +} diff --git a/boa_engine/src/object/shape/shared_shape/mod.rs b/boa_engine/src/object/shape/shared_shape/mod.rs index 3b3e931e31e..2f72b864be6 100644 --- a/boa_engine/src/object/shape/shared_shape/mod.rs +++ b/boa_engine/src/object/shape/shared_shape/mod.rs @@ -1,8 +1,10 @@ -pub(crate) mod property_table; +mod forward_transition; +mod property_table; use std::{ cell::{Cell, RefCell}, collections::hash_map::RandomState, + hash::Hash, rc::Rc, }; @@ -15,7 +17,7 @@ use crate::{ property::{Attribute, PropertyDescriptor, PropertyKey}, }; -use self::property_table::PropertyTable; +use self::{forward_transition::ForwardTransition, property_table::PropertyTable}; use super::{ slot::{SlotAttribute, SlotIndex}, @@ -53,8 +55,8 @@ unsafe impl Trace for TransitionType { /// TODO: doc #[derive(Debug, Trace, Finalize)] struct Inner { - forward_property_transitions: GcRefCell>>, - forward_prototype_transitions: GcRefCell>>, + forward_property_transitions: ForwardTransition, + forward_prototype_transitions: ForwardTransition, property_count: u32, @@ -106,15 +108,12 @@ impl SharedShape { pub(crate) fn is_root(&self) -> bool { self.inner.previous.is_none() } - fn get_forward_property_transition(&self, key: &TransitionKey) -> Option> { - let map = self.inner.forward_property_transitions.borrow(); - map.get(key).cloned() + fn forward_property_transitions(&self) -> &ForwardTransition { + &self.inner.forward_property_transitions } - fn get_forward_prototype_transition(&self, prototype: &JsPrototype) -> Option> { - let map = self.inner.forward_prototype_transitions.borrow(); - map.get(prototype).cloned() + fn forward_prototype_transitions(&self) -> &ForwardTransition { + &self.inner.forward_prototype_transitions } - fn new(inner: Inner) -> Self { Self { inner: Gc::new(inner), @@ -122,12 +121,25 @@ impl SharedShape { } pub(crate) fn root() -> Self { + println!("sizeof Inner: {}", std::mem::size_of::()); + println!( + "sizeof Innerff: {}", + std::mem::size_of::>>>() + ); + println!( + "sizeof Innerff: {}", + std::mem::size_of::>() + ); + println!( + "sizeof JsObject: {}", + std::mem::size_of::() + ); Self::new(Inner { prototype: None, property_count: 0, property_table: PropertyTable::default(), - forward_property_transitions: GcRefCell::default(), - forward_prototype_transitions: GcRefCell::default(), + forward_property_transitions: ForwardTransition::default(), + forward_prototype_transitions: ForwardTransition::default(), previous: None, transition_type: TransitionType::Insert, }) @@ -147,8 +159,8 @@ impl SharedShape { ); let new_inner_shape = Inner { prototype: self.prototype(), - forward_property_transitions: GcRefCell::default(), - forward_prototype_transitions: GcRefCell::default(), + forward_property_transitions: ForwardTransition::default(), + forward_prototype_transitions: ForwardTransition::default(), property_table, property_count: self.property_count() + 1, previous: Some(self.clone()), @@ -156,14 +168,14 @@ impl SharedShape { }; 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)); + self.forward_property_transitions() + .insert(key, &new_shape.inner); new_shape } pub(crate) fn change_prototype_transition(&self, prototype: JsPrototype) -> Self { - if let Some(shape) = self.get_forward_prototype_transition(&prototype) { + if let Some(shape) = self.forward_prototype_transitions().get(&prototype) { if let Some(inner) = shape.upgrade() { if Self::DEBUG { println!(" Shape: Resusing previous prototype change shape"); @@ -173,8 +185,8 @@ impl SharedShape { } let new_inner_shape = Inner { prototype: prototype.clone(), - forward_property_transitions: GcRefCell::default(), - forward_prototype_transitions: GcRefCell::default(), + forward_property_transitions: ForwardTransition::default(), + forward_prototype_transitions: ForwardTransition::default(), property_table: self.inner.property_table.clone(), property_count: self.property_count(), previous: Some(self.clone()), @@ -182,15 +194,15 @@ impl SharedShape { }; let new_shape = Self::new(new_inner_shape); - let mut map = self.inner.forward_prototype_transitions.borrow_mut(); - map.insert(prototype, WeakGc::new(&new_shape.inner)); + self.forward_prototype_transitions() + .insert(prototype, &new_shape.inner); new_shape } pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { // Check if we have already creaded such a transition, if so use it! - if let Some(shape) = self.get_forward_property_transition(&key) { + if let Some(shape) = self.forward_property_transitions().get(&key) { if Self::DEBUG { println!("Shape: Trying to resuse previous shape"); } @@ -218,15 +230,15 @@ impl SharedShape { prototype: self.prototype(), property_table, property_count: self.property_count(), - forward_property_transitions: GcRefCell::default(), - forward_prototype_transitions: GcRefCell::default(), + forward_property_transitions: ForwardTransition::default(), + forward_prototype_transitions: ForwardTransition::default(), 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)); + self.forward_property_transitions() + .insert(key, &new_shape.inner); new_shape } From 05791d5fbd560ebda6e1b3ade38d784eafece46e Mon Sep 17 00:00:00 2001 From: Haled Odat Date: Sun, 26 Mar 2023 00:11:52 +0100 Subject: [PATCH 19/19] Shrink Shared Shape from 128 to 72 bytes --- .../shape/shared_shape/forward_transition.rs | 52 +++++++++++++------ .../src/object/shape/shared_shape/mod.rs | 51 ++++++------------ 2 files changed, 50 insertions(+), 53 deletions(-) diff --git a/boa_engine/src/object/shape/shared_shape/forward_transition.rs b/boa_engine/src/object/shape/shared_shape/forward_transition.rs index 6c7e6703110..b3063010da5 100644 --- a/boa_engine/src/object/shape/shared_shape/forward_transition.rs +++ b/boa_engine/src/object/shape/shared_shape/forward_transition.rs @@ -3,30 +3,48 @@ use std::hash::Hash; use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; use rustc_hash::FxHashMap; -use super::Inner as SharedShapeInner; +use crate::object::JsPrototype; + +use super::{Inner as SharedShapeInner, TransitionKey}; + +type TransitionMap = FxHashMap>; + +#[derive(Default, Debug, Trace, Finalize)] +struct Inner { + properties: Option>>, + prototypes: Option>>, +} /// Holds a forward reference to a previously created transition. /// The reference is weak, therefore it can be garbage collected if it is not in use. -#[derive(Debug, Clone, Trace, Finalize)] -pub(super) struct ForwardTransition { - transitions: GcRefCell>>, +#[derive(Default, Debug, Trace, Finalize)] +pub(super) struct ForwardTransition { + inner: GcRefCell, } -impl Default for ForwardTransition { - fn default() -> Self { - Self { - transitions: GcRefCell::default(), - } +impl ForwardTransition { + pub(super) fn insert_property(&self, key: TransitionKey, value: &Gc) { + let mut this = self.inner.borrow_mut(); + let properties = this.properties.get_or_insert_with(Box::default); + properties.insert(key, WeakGc::new(value)); } -} - -impl ForwardTransition { - pub(super) fn insert(&self, key: T, value: &Gc) { - let mut transitions = self.transitions.borrow_mut(); - transitions.insert(key, WeakGc::new(value)); + pub(super) fn insert_prototype(&self, key: JsPrototype, value: &Gc) { + let mut this = self.inner.borrow_mut(); + let prototypes = this.prototypes.get_or_insert_with(Box::default); + prototypes.insert(key, WeakGc::new(value)); + } + pub(super) fn get_property(&self, key: &TransitionKey) -> Option> { + let this = self.inner.borrow(); + let Some(transitions) = this.properties.as_ref() else { + return None; + }; + transitions.get(key).cloned() } - pub(super) fn get(&self, key: &T) -> Option> { - let transitions = self.transitions.borrow(); + pub(super) fn get_prototype(&self, key: &JsPrototype) -> Option> { + let this = self.inner.borrow(); + let Some(transitions) = this.prototypes.as_ref() else { + return None; + }; transitions.get(key).cloned() } } diff --git a/boa_engine/src/object/shape/shared_shape/mod.rs b/boa_engine/src/object/shape/shared_shape/mod.rs index 2f72b864be6..b8e970760ff 100644 --- a/boa_engine/src/object/shape/shared_shape/mod.rs +++ b/boa_engine/src/object/shape/shared_shape/mod.rs @@ -55,8 +55,7 @@ unsafe impl Trace for TransitionType { /// TODO: doc #[derive(Debug, Trace, Finalize)] struct Inner { - forward_property_transitions: ForwardTransition, - forward_prototype_transitions: ForwardTransition, + forward_transitions: ForwardTransition, property_count: u32, @@ -108,11 +107,8 @@ impl SharedShape { pub(crate) fn is_root(&self) -> bool { self.inner.previous.is_none() } - fn forward_property_transitions(&self) -> &ForwardTransition { - &self.inner.forward_property_transitions - } - fn forward_prototype_transitions(&self) -> &ForwardTransition { - &self.inner.forward_prototype_transitions + fn forward_transitions(&self) -> &ForwardTransition { + &self.inner.forward_transitions } fn new(inner: Inner) -> Self { Self { @@ -121,25 +117,11 @@ impl SharedShape { } pub(crate) fn root() -> Self { - println!("sizeof Inner: {}", std::mem::size_of::()); - println!( - "sizeof Innerff: {}", - std::mem::size_of::>>>() - ); - println!( - "sizeof Innerff: {}", - std::mem::size_of::>() - ); - println!( - "sizeof JsObject: {}", - std::mem::size_of::() - ); Self::new(Inner { + forward_transitions: ForwardTransition::default(), prototype: None, property_count: 0, property_table: PropertyTable::default(), - forward_property_transitions: ForwardTransition::default(), - forward_prototype_transitions: ForwardTransition::default(), previous: None, transition_type: TransitionType::Insert, }) @@ -159,8 +141,7 @@ impl SharedShape { ); let new_inner_shape = Inner { prototype: self.prototype(), - forward_property_transitions: ForwardTransition::default(), - forward_prototype_transitions: ForwardTransition::default(), + forward_transitions: ForwardTransition::default(), property_table, property_count: self.property_count() + 1, previous: Some(self.clone()), @@ -168,14 +149,14 @@ impl SharedShape { }; let new_shape = Self::new(new_inner_shape); - self.forward_property_transitions() - .insert(key, &new_shape.inner); + self.forward_transitions() + .insert_property(key, &new_shape.inner); new_shape } pub(crate) fn change_prototype_transition(&self, prototype: JsPrototype) -> Self { - if let Some(shape) = self.forward_prototype_transitions().get(&prototype) { + if let Some(shape) = self.forward_transitions().get_prototype(&prototype) { if let Some(inner) = shape.upgrade() { if Self::DEBUG { println!(" Shape: Resusing previous prototype change shape"); @@ -184,9 +165,8 @@ impl SharedShape { } } let new_inner_shape = Inner { + forward_transitions: ForwardTransition::default(), prototype: prototype.clone(), - forward_property_transitions: ForwardTransition::default(), - forward_prototype_transitions: ForwardTransition::default(), property_table: self.inner.property_table.clone(), property_count: self.property_count(), previous: Some(self.clone()), @@ -194,15 +174,15 @@ impl SharedShape { }; let new_shape = Self::new(new_inner_shape); - self.forward_prototype_transitions() - .insert(prototype, &new_shape.inner); + self.forward_transitions() + .insert_prototype(prototype, &new_shape.inner); new_shape } pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { // Check if we have already creaded such a transition, if so use it! - if let Some(shape) = self.forward_property_transitions().get(&key) { + if let Some(shape) = self.forward_transitions().get_property(&key) { if Self::DEBUG { println!("Shape: Trying to resuse previous shape"); } @@ -227,18 +207,17 @@ impl SharedShape { let property_table = self.inner.property_table.deep_clone_all(); property_table.set_attributes(&key.property_key, key.attributes); let new_inner_shape = Inner { + forward_transitions: ForwardTransition::default(), prototype: self.prototype(), property_table, property_count: self.property_count(), - forward_property_transitions: ForwardTransition::default(), - forward_prototype_transitions: ForwardTransition::default(), previous: Some(self.clone()), transition_type: TransitionType::Configure, }; let new_shape = Self::new(new_inner_shape); - self.forward_property_transitions() - .insert(key, &new_shape.inner); + self.forward_transitions() + .insert_property(key, &new_shape.inner); new_shape }