From e335c54926a26873bbc4a1a571d646accf6659bb Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Fri, 19 May 2023 05:43:17 +0200 Subject: [PATCH] Type safe root shape (#2940) * Implement type safe root object shape * Implement `From` for Unique and Shared shapes --- boa_engine/benches/full.rs | 4 +- boa_engine/src/builtins/weak/weak_ref.rs | 4 +- boa_engine/src/builtins/weak_map/mod.rs | 4 +- boa_engine/src/builtins/weak_set/mod.rs | 4 +- boa_engine/src/context/intrinsics.rs | 8 ++- boa_engine/src/context/mod.rs | 12 ++-- boa_engine/src/object/jsobject.rs | 6 +- boa_engine/src/object/property_map.rs | 13 +++-- boa_engine/src/object/shape/mod.rs | 57 ++++++++++--------- boa_engine/src/object/shape/root_shape.rs | 27 +++++++++ .../src/object/shape/shared_shape/mod.rs | 2 +- .../src/object/shape/shared_shape/template.rs | 17 +++--- boa_engine/src/object/shape/unique_shape.rs | 4 +- boa_engine/src/realm.rs | 4 +- 14 files changed, 104 insertions(+), 62 deletions(-) create mode 100644 boa_engine/src/object/shape/root_shape.rs diff --git a/boa_engine/benches/full.rs b/boa_engine/benches/full.rs index 5317bbdb65..f59a92dca5 100644 --- a/boa_engine/benches/full.rs +++ b/boa_engine/benches/full.rs @@ -1,7 +1,7 @@ //! Benchmarks of the whole execution engine in Boa. use boa_engine::{ - context::DefaultHooks, object::shape::SharedShape, optimizer::OptimizerOptions, realm::Realm, + context::DefaultHooks, object::shape::RootShape, optimizer::OptimizerOptions, realm::Realm, Context, Source, }; use criterion::{criterion_group, criterion_main, Criterion}; @@ -16,7 +16,7 @@ static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; fn create_realm(c: &mut Criterion) { c.bench_function("Create Realm", move |b| { - let root_shape = SharedShape::root(); + let root_shape = RootShape::default(); b.iter(|| Realm::create(&DefaultHooks, &root_shape)) }); } diff --git a/boa_engine/src/builtins/weak/weak_ref.rs b/boa_engine/src/builtins/weak/weak_ref.rs index a12ef32d32..4ec3bbe1ce 100644 --- a/boa_engine/src/builtins/weak/weak_ref.rs +++ b/boa_engine/src/builtins/weak/weak_ref.rs @@ -80,9 +80,11 @@ impl BuiltInConstructor for WeakRef { // 3. Let weakRef be ? OrdinaryCreateFromConstructor(NewTarget, "%WeakRef.prototype%", « [[WeakRefTarget]] »). // 5. Set weakRef.[[WeakRefTarget]] to target. + let prototype = + get_prototype_from_constructor(new_target, StandardConstructors::weak_ref, context)?; let weak_ref = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), - get_prototype_from_constructor(new_target, StandardConstructors::weak_ref, context)?, + prototype, ObjectData::weak_ref(WeakGc::new(target.inner())), ); diff --git a/boa_engine/src/builtins/weak_map/mod.rs b/boa_engine/src/builtins/weak_map/mod.rs index 788cef1ea3..66f5e5aed4 100644 --- a/boa_engine/src/builtins/weak_map/mod.rs +++ b/boa_engine/src/builtins/weak_map/mod.rs @@ -82,9 +82,11 @@ impl BuiltInConstructor for WeakMap { // 2. Let map be ? OrdinaryCreateFromConstructor(NewTarget, "%WeakMap.prototype%", « [[WeakMapData]] »). // 3. Set map.[[WeakMapData]] to a new empty List. + let prototype = + get_prototype_from_constructor(new_target, StandardConstructors::weak_map, context)?; let map = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), - get_prototype_from_constructor(new_target, StandardConstructors::weak_map, context)?, + prototype, ObjectData::weak_map(boa_gc::WeakMap::new()), ); diff --git a/boa_engine/src/builtins/weak_set/mod.rs b/boa_engine/src/builtins/weak_set/mod.rs index 68f2bb4aee..13304a39f8 100644 --- a/boa_engine/src/builtins/weak_set/mod.rs +++ b/boa_engine/src/builtins/weak_set/mod.rs @@ -78,9 +78,11 @@ impl BuiltInConstructor for WeakSet { // 2. Let set be ? OrdinaryCreateFromConstructor(NewTarget, "%WeakSet.prototype%", « [[WeakSetData]] »). // 3. Set set.[[WeakSetData]] to a new empty List. + let prototype = + get_prototype_from_constructor(new_target, StandardConstructors::weak_set, context)?; let weak_set = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), - get_prototype_from_constructor(new_target, StandardConstructors::weak_set, context)?, + prototype, ObjectData::weak_set(WeakMap::new()), ); diff --git a/boa_engine/src/context/intrinsics.rs b/boa_engine/src/context/intrinsics.rs index 9d2ff4e0b1..92d3c825d5 100644 --- a/boa_engine/src/context/intrinsics.rs +++ b/boa_engine/src/context/intrinsics.rs @@ -5,7 +5,7 @@ use boa_gc::{Finalize, Trace}; use crate::{ builtins::{iterable::IteratorPrototypes, uri::UriFunctions}, object::{ - shape::shared_shape::{template::ObjectTemplate, SharedShape}, + shape::{shared_shape::template::ObjectTemplate, RootShape}, JsFunction, JsObject, ObjectData, CONSTRUCTOR, PROTOTYPE, }, property::{Attribute, PropertyKey}, @@ -27,7 +27,7 @@ pub struct Intrinsics { } impl Intrinsics { - pub(crate) fn new(root_shape: &SharedShape) -> Self { + pub(crate) fn new(root_shape: &RootShape) -> Self { let constructors = StandardConstructors::default(); let templates = ObjectTemplates::new(root_shape, &constructors); @@ -1004,7 +1004,9 @@ pub(crate) struct ObjectTemplates { } impl ObjectTemplates { - pub(crate) fn new(root_shape: &SharedShape, constructors: &StandardConstructors) -> Self { + pub(crate) fn new(root_shape: &RootShape, constructors: &StandardConstructors) -> Self { + let root_shape = root_shape.shape(); + // pre-initialize used shapes. let ordinary_object = ObjectTemplate::with_prototype(root_shape, constructors.object().prototype()); diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index 8bedb55205..5d8551a820 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -23,7 +23,7 @@ use crate::{ job::{JobQueue, NativeJob, SimpleJobQueue}, module::{ModuleLoader, SimpleModuleLoader}, native_function::NativeFunction, - object::{shape::SharedShape, FunctionObjectBuilder, JsObject}, + object::{shape::RootShape, FunctionObjectBuilder, JsObject}, optimizer::{Optimizer, OptimizerOptions, OptimizerStatistics}, property::{Attribute, PropertyDescriptor, PropertyKey}, realm::Realm, @@ -109,7 +109,7 @@ pub struct Context<'host> { module_loader: MaybeShared<'host, dyn ModuleLoader>, optimizer_options: OptimizerOptions, - root_shape: SharedShape, + root_shape: RootShape, /// Unique identifier for each parser instance used during the context lifetime. parser_identifier: u32, @@ -496,8 +496,10 @@ impl<'host> Context<'host> { std::mem::replace(&mut self.realm, realm) } - pub(crate) fn root_shape(&self) -> SharedShape { - self.root_shape.clone() + /// Get the [`RootShape`]. + #[inline] + pub const fn root_shape(&self) -> &RootShape { + &self.root_shape } /// Gets the host hooks. @@ -916,7 +918,7 @@ impl<'icu, 'hooks, 'queue, 'module> ContextBuilder<'icu, 'hooks, 'queue, 'module 'queue: 'host, 'module: 'host, { - let root_shape = SharedShape::root(); + let root_shape = RootShape::default(); let host_hooks = self.host_hooks.unwrap_or_else(|| { let hooks: &dyn HostHooks = &DefaultHooks; diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index b2e6f51c7d..56eed87144 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -4,7 +4,7 @@ use super::{ internal_methods::{InternalObjectMethods, ARRAY_EXOTIC_INTERNAL_METHODS}, - shape::{shared_shape::SharedShape, Shape}, + shape::RootShape, JsPrototype, NativeObject, Object, PrivateName, PropertyMap, }; use crate::{ @@ -128,7 +128,7 @@ impl JsObject { /// /// [`OrdinaryObjectCreate`]: https://tc39.es/ecma262/#sec-ordinaryobjectcreate pub(crate) fn from_proto_and_data_with_shared_shape>>( - root_shape: SharedShape, + root_shape: &RootShape, prototype: O, data: ObjectData, ) -> Self { @@ -137,7 +137,7 @@ impl JsObject { object: GcRefCell::new(Object { kind: data.kind, properties: PropertyMap::from_prototype_with_shared_shape( - Shape::shared(root_shape), + root_shape, prototype.into(), ), extensible: true, diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index 90ffb50989..9a06080a13 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -3,7 +3,7 @@ use super::{ property_table::PropertyTableInner, shared_shape::TransitionKey, slot::{Slot, SlotAttributes}, - ChangeTransitionAction, Shape, UniqueShape, + ChangeTransitionAction, RootShape, Shape, UniqueShape, }, JsPrototype, ObjectStorage, PropertyDescriptor, PropertyKey, }; @@ -244,7 +244,7 @@ impl PropertyMap { pub fn from_prototype_unique_shape(prototype: JsPrototype) -> Self { Self { indexed_properties: IndexedProperties::default(), - shape: Shape::unique(UniqueShape::new(prototype, PropertyTableInner::default())), + shape: UniqueShape::new(prototype, PropertyTableInner::default()).into(), storage: Vec::default(), } } @@ -252,11 +252,14 @@ impl PropertyMap { /// Construct a [`PropertyMap`] from with the given prototype with a shared shape [`Shape`]. #[must_use] #[inline] - pub fn from_prototype_with_shared_shape(mut root_shape: Shape, prototype: JsPrototype) -> Self { - root_shape = root_shape.change_prototype_transition(prototype); + pub fn from_prototype_with_shared_shape( + root_shape: &RootShape, + prototype: JsPrototype, + ) -> Self { + let shape = root_shape.shape().change_prototype_transition(prototype); Self { indexed_properties: IndexedProperties::default(), - shape: root_shape, + shape: shape.into(), storage: Vec::default(), } } diff --git a/boa_engine/src/object/shape/mod.rs b/boa_engine/src/object/shape/mod.rs index 31fe800317..352ca91567 100644 --- a/boa_engine/src/object/shape/mod.rs +++ b/boa_engine/src/object/shape/mod.rs @@ -1,10 +1,12 @@ //! Implements object shapes. pub(crate) mod property_table; +mod root_shape; pub(crate) mod shared_shape; pub(crate) mod slot; pub(crate) mod unique_shape; +pub use root_shape::RootShape; pub use shared_shape::SharedShape; pub(crate) use unique_shape::UniqueShape; @@ -63,8 +65,9 @@ pub struct Shape { } impl Default for Shape { + #[inline] fn default() -> Self { - Self::unique(UniqueShape::default()) + UniqueShape::default().into() } } @@ -75,20 +78,6 @@ impl Shape { /// NOTE: This only applies to [`SharedShape`]. const TRANSITION_COUNT_MAX: u16 = 1024; - /// Create a [`Shape`] from a [`SharedShape`]. - pub(crate) fn shared(inner: SharedShape) -> Self { - Self { - inner: Inner::Shared(inner), - } - } - - /// Create a [`Shape`] from a [`UniqueShape`]. - pub(crate) const fn unique(shape: UniqueShape) -> Self { - Self { - inner: Inner::Unique(shape), - } - } - /// Returns `true` if it's a shared shape, `false` otherwise. #[inline] pub const fn is_shared(&self) -> bool { @@ -116,11 +105,11 @@ impl Shape { Inner::Shared(shape) => { let shape = shape.insert_property_transition(key); if shape.transition_count() >= Self::TRANSITION_COUNT_MAX { - return Self::unique(shape.to_unique()); + return shape.to_unique().into(); } - Self::shared(shape) + shape.into() } - Inner::Unique(shape) => Self::unique(shape.insert_property_transition(key)), + Inner::Unique(shape) => shape.insert_property_transition(key).into(), } } @@ -137,9 +126,9 @@ impl Shape { let change_transition = shape.change_attributes_transition(key); let shape = if change_transition.shape.transition_count() >= Self::TRANSITION_COUNT_MAX { - Self::unique(change_transition.shape.to_unique()) + change_transition.shape.to_unique().into() } else { - Self::shared(change_transition.shape) + change_transition.shape.into() }; ChangeTransition { shape, @@ -158,11 +147,11 @@ impl Shape { Inner::Shared(shape) => { let shape = shape.remove_property_transition(key); if shape.transition_count() >= Self::TRANSITION_COUNT_MAX { - return Self::unique(shape.to_unique()); + return shape.to_unique().into(); } - Self::shared(shape) + shape.into() } - Inner::Unique(shape) => Self::unique(shape.remove_property_transition(key)), + Inner::Unique(shape) => shape.remove_property_transition(key).into(), } } @@ -172,11 +161,11 @@ impl Shape { Inner::Shared(shape) => { let shape = shape.change_prototype_transition(prototype); if shape.transition_count() >= Self::TRANSITION_COUNT_MAX { - return Self::unique(shape.to_unique()); + return shape.to_unique().into(); } - Self::shared(shape) + shape.into() } - Inner::Unique(shape) => Self::unique(shape.change_prototype_transition(prototype)), + Inner::Unique(shape) => shape.change_prototype_transition(prototype).into(), } } @@ -215,3 +204,19 @@ impl Shape { } } } + +impl From for Shape { + fn from(shape: UniqueShape) -> Self { + Self { + inner: Inner::Unique(shape), + } + } +} + +impl From for Shape { + fn from(shape: SharedShape) -> Self { + Self { + inner: Inner::Shared(shape), + } + } +} diff --git a/boa_engine/src/object/shape/root_shape.rs b/boa_engine/src/object/shape/root_shape.rs new file mode 100644 index 0000000000..da64191ee5 --- /dev/null +++ b/boa_engine/src/object/shape/root_shape.rs @@ -0,0 +1,27 @@ +use boa_macros::{Finalize, Trace}; + +use super::SharedShape; + +/// This is a wrapper around [`SharedShape`] that ensures it's root shape. +/// +/// Represent the root shape that [`SharedShape`] transitions start from. +#[derive(Debug, Clone, Trace, Finalize)] +pub struct RootShape { + shape: SharedShape, +} + +impl Default for RootShape { + #[inline] + fn default() -> Self { + Self { + shape: SharedShape::root(), + } + } +} + +impl RootShape { + /// Gets the inner [`SharedShape`]. + pub const fn shape(&self) -> &SharedShape { + &self.shape + } +} diff --git a/boa_engine/src/object/shape/shared_shape/mod.rs b/boa_engine/src/object/shape/shared_shape/mod.rs index 0da7b69b47..02931998d0 100644 --- a/boa_engine/src/object/shape/shared_shape/mod.rs +++ b/boa_engine/src/object/shape/shared_shape/mod.rs @@ -174,7 +174,7 @@ impl SharedShape { /// Create a root [`SharedShape`]. #[must_use] - pub fn root() -> Self { + pub(crate) fn root() -> Self { Self::new(Inner { forward_transitions: ForwardTransition::default(), prototype: None, diff --git a/boa_engine/src/object/shape/shared_shape/template.rs b/boa_engine/src/object/shape/shared_shape/template.rs index 1f9d3b4141..14316cd21f 100644 --- a/boa_engine/src/object/shape/shared_shape/template.rs +++ b/boa_engine/src/object/shape/shared_shape/template.rs @@ -2,10 +2,7 @@ use boa_gc::{Finalize, Trace}; use thin_vec::ThinVec; use crate::{ - object::{ - shape::{slot::SlotAttributes, Shape}, - JsObject, Object, ObjectData, PropertyMap, - }, + object::{shape::slot::SlotAttributes, JsObject, Object, ObjectData, PropertyMap}, property::{Attribute, PropertyKey}, JsValue, }; @@ -21,15 +18,15 @@ pub(crate) struct ObjectTemplate { impl ObjectTemplate { /// Create a new [`ObjectTemplate`] - pub(crate) fn new(root_shape: &SharedShape) -> Self { + pub(crate) fn new(shape: &SharedShape) -> Self { Self { - shape: root_shape.clone(), + shape: shape.clone(), } } /// Create and [`ObjectTemplate`] with a prototype. - pub(crate) fn with_prototype(root_shape: &SharedShape, prototype: JsObject) -> Self { - let shape = root_shape.change_prototype_transition(Some(prototype)); + pub(crate) fn with_prototype(shape: &SharedShape, prototype: JsObject) -> Self { + let shape = shape.change_prototype_transition(Some(prototype)); Self { shape } } @@ -111,7 +108,7 @@ impl ObjectTemplate { let mut object = Object { kind: data.kind, extensible: true, - properties: PropertyMap::new(Shape::shared(self.shape.clone()), ThinVec::default()), + properties: PropertyMap::new(self.shape.clone().into(), ThinVec::default()), private_elements: ThinVec::new(), }; @@ -133,7 +130,7 @@ impl ObjectTemplate { let mut object = Object { kind: data.kind, extensible: true, - properties: PropertyMap::new(Shape::shared(self.shape.clone()), elements), + properties: PropertyMap::new(self.shape.clone().into(), elements), private_elements: ThinVec::new(), }; diff --git a/boa_engine/src/object/shape/unique_shape.rs b/boa_engine/src/object/shape/unique_shape.rs index 5e052fb02c..68872a96c8 100644 --- a/boa_engine/src/object/shape/unique_shape.rs +++ b/boa_engine/src/object/shape/unique_shape.rs @@ -149,7 +149,7 @@ impl UniqueShape { property_table.keys[index].1.attributes = key.attributes; // TODO: invalidate the pointer. return ChangeTransition { - shape: Shape::unique(self.clone()), + shape: self.clone().into(), action: ChangeTransitionAction::Nothing, }; } @@ -211,7 +211,7 @@ impl UniqueShape { let shape = Self::new(prototype, property_table); ChangeTransition { - shape: Shape::unique(shape), + shape: shape.into(), action, } } diff --git a/boa_engine/src/realm.rs b/boa_engine/src/realm.rs index f777289972..95ff5941f3 100644 --- a/boa_engine/src/realm.rs +++ b/boa_engine/src/realm.rs @@ -10,7 +10,7 @@ use crate::{ context::{intrinsics::Intrinsics, HostHooks}, environments::DeclarativeEnvironment, module::Module, - object::{shape::shared_shape::SharedShape, JsObject}, + object::{shape::RootShape, JsObject}, JsString, }; use boa_gc::{Finalize, Gc, GcRefCell, Trace}; @@ -57,7 +57,7 @@ struct Inner { impl Realm { /// Create a new Realm. #[inline] - pub fn create(hooks: &dyn HostHooks, root_shape: &SharedShape) -> Self { + pub fn create(hooks: &dyn HostHooks, root_shape: &RootShape) -> Self { let _timer = Profiler::global().start_event("Realm::create", "realm"); let intrinsics = Intrinsics::new(root_shape);