From 5ea6d0a010dfbaddd16f677691d35cccbcef4af6 Mon Sep 17 00:00:00 2001 From: Alistair Date: Fri, 10 Nov 2023 12:56:35 +0000 Subject: [PATCH] `[[HostDefined]]` Improvements (#3460) * Refactor the `[[HostDefined]]` implementation. Currently `[[HostDefined]]` doesn't permit you to mutably borrow two objects from the `[[HostDefined]]` field since the `FxHashMap` is wrapped under a `GcRefCell`. This commit implements a `get_mut_many` (from `hashbrown`'s `HashMap`) method to permit accessing several `NativeObject`s using a `NativeTuple`. Additionally, this commit takes the opportunity to provide automatic downcasting on the `insert` and `remove` methods for `[[HostDefined]]`. * Update `[[HostDefined]]` example. --- Cargo.lock | 1 + boa_engine/Cargo.toml | 1 + boa_engine/src/host_defined.rs | 173 +++++++++++++++++---------- boa_engine/src/realm.rs | 31 ++++- boa_examples/src/bin/host_defined.rs | 67 ++++++++--- boa_gc/src/trace.rs | 16 +++ 6 files changed, 204 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8b33c35b755..e223e13944e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -406,6 +406,7 @@ dependencies = [ "fixed_decimal", "float-cmp", "futures-lite 2.0.1", + "hashbrown 0.14.2", "icu_calendar", "icu_casemap", "icu_collator", diff --git a/boa_engine/Cargo.toml b/boa_engine/Cargo.toml index ab06d980518..8252dae5597 100644 --- a/boa_engine/Cargo.toml +++ b/boa_engine/Cargo.toml @@ -110,6 +110,7 @@ writeable = { workspace = true, optional = true } yoke = { workspace = true, optional = true } zerofrom = { workspace = true, optional = true } fixed_decimal = { workspace = true, features = ["ryu"], optional = true} +hashbrown.workspace = true [target.'cfg(all(target_family = "wasm", not(any(target_os = "emscripten", target_os = "wasi"))))'.dependencies] web-time = { version = "0.2.3", optional = true } diff --git a/boa_engine/src/host_defined.rs b/boa_engine/src/host_defined.rs index ff66267fbf5..2af22570d1a 100644 --- a/boa_engine/src/host_defined.rs +++ b/boa_engine/src/host_defined.rs @@ -1,111 +1,154 @@ use std::any::TypeId; -use boa_gc::{GcRef, GcRefCell, GcRefMut}; use boa_macros::{Finalize, Trace}; -use rustc_hash::FxHashMap; +use hashbrown::hash_map::HashMap; use crate::object::NativeObject; -/// Map used to store the host defined objects. -#[doc(hidden)] -type HostDefinedMap = FxHashMap>; - /// This represents a `ECMASCript` specification \[`HostDefined`\] field. /// /// This allows storing types which are mapped by their [`TypeId`]. #[derive(Default, Trace, Finalize)] #[allow(missing_debug_implementations)] pub struct HostDefined { - state: GcRefCell, + // INVARIANT: All key-value pairs `(id, obj)` satisfy: + // `id == TypeId::of::() && obj.is::()` + // for some type `T : NativeObject`. + types: HashMap>, +} + +// TODO: Track https://github.com/rust-lang/rust/issues/65991 and +// https://github.com/rust-lang/rust/issues/90850 to remove this +// when those are stabilized. +fn downcast_boxed_native_object_unchecked(obj: Box) -> Box { + let raw: *mut dyn NativeObject = Box::into_raw(obj); + + // SAFETY: We know that `obj` is of type `T` (due to the INVARIANT of `HostDefined`). + // See `HostDefined::insert`, `HostDefined::insert_default` and `HostDefined::remove`. + unsafe { Box::from_raw(raw.cast::()) } } impl HostDefined { /// Insert a type into the [`HostDefined`]. - /// - /// # Panics - /// - /// Panics if [`HostDefined`] field is borrowed. #[track_caller] - pub fn insert_default(&self) -> Option> { - self.state - .borrow_mut() + pub fn insert_default(&mut self) -> Option> { + self.types .insert(TypeId::of::(), Box::::default()) + .map(downcast_boxed_native_object_unchecked) } /// Insert a type into the [`HostDefined`]. - /// - /// # Panics - /// - /// Panics if [`HostDefined`] field is borrowed. #[track_caller] - pub fn insert(&self, value: T) -> Option> { - self.state - .borrow_mut() + pub fn insert(&mut self, value: T) -> Option> { + self.types .insert(TypeId::of::(), Box::new(value)) + .map(downcast_boxed_native_object_unchecked) } /// Check if the [`HostDefined`] has type T. - /// - /// # Panics - /// - /// Panics if [`HostDefined`] field is borrowed mutably. + #[must_use] #[track_caller] pub fn has(&self) -> bool { - self.state.borrow().contains_key(&TypeId::of::()) + self.types.contains_key(&TypeId::of::()) } /// Remove type T from [`HostDefined`], if it exists. /// /// Returns [`Some`] with the object if it exits, [`None`] otherwise. - /// - /// # Panics - /// - /// Panics if [`HostDefined`] field is borrowed. #[track_caller] - pub fn remove(&self) -> Option> { - self.state.borrow_mut().remove(&TypeId::of::()) + pub fn remove(&mut self) -> Option> { + self.types + .remove(&TypeId::of::()) + .map(downcast_boxed_native_object_unchecked) } - /// Get type T from [`HostDefined`], if it exits. - /// - /// # Panics - /// - /// Panics if [`HostDefined`] field is borrowed. + /// Get type T from [`HostDefined`], if it exists. #[track_caller] - pub fn get(&self) -> Option> { - GcRef::try_map(self.state.borrow(), |state| { - state - .get(&TypeId::of::()) - .map(Box::as_ref) - .and_then(::downcast_ref::) - }) + pub fn get(&self) -> Option<&T> { + self.types + .get(&TypeId::of::()) + .map(Box::as_ref) + .and_then(::downcast_ref::) } - /// Get type T from [`HostDefined`], if it exits. - /// - /// # Panics - /// - /// Panics if [`HostDefined`] field is borrowed. + /// Get type T from [`HostDefined`], if it exists. #[track_caller] - pub fn get_mut(&self) -> Option> { - GcRefMut::try_map( - self.state.borrow_mut(), - |state: &mut FxHashMap>| { - state - .get_mut(&TypeId::of::()) - .map(Box::as_mut) - .and_then(::downcast_mut::) - }, - ) + pub fn get_mut(&mut self) -> Option<&mut T> { + self.types + .get_mut(&TypeId::of::()) + .map(Box::as_mut) + .and_then(::downcast_mut::) + } + + /// Get type a tuple of types from [`HostDefined`], if they exist. + #[track_caller] + pub fn get_many_mut(&mut self) -> Option> + where + T: NativeTuple, + { + let ids = T::as_type_ids(); + let refs: [&TypeId; SIZE] = ids + .iter() + .collect::>() + .try_into() + .expect("tuple should be of size `SIZE`"); + + self.types.get_many_mut(refs).and_then(T::mut_ref_from_anys) } /// Clears all the objects. - /// - /// # Panics - /// - /// Panics if [`HostDefined`] field is borrowed. #[track_caller] - pub fn clear(&self) { - self.state.borrow_mut().clear(); + pub fn clear(&mut self) { + self.types.clear(); + } +} + +/// This trait represents a tuple of [`NativeObject`]s capable of being +/// used in [`HostDefined`]. +/// +/// This allows accessing multiple types from [`HostDefined`] at once. +pub trait NativeTuple { + type NativeTupleMutRef<'a>; + + fn as_type_ids() -> Vec; + + fn mut_ref_from_anys( + anys: [&'_ mut Box; SIZE], + ) -> Option>; +} + +macro_rules! impl_native_tuple { + ($size:literal $(,$name:ident)* ) => { + impl<$($name: NativeObject,)*> NativeTuple<$size> for ($($name,)*) { + type NativeTupleMutRef<'a> = ($(&'a mut $name,)*); + + fn as_type_ids() -> Vec { + vec![$(TypeId::of::<$name>(),)*] + } + + fn mut_ref_from_anys( + anys: [&'_ mut Box; $size], + ) -> Option> { + #[allow(unused_variables, unused_mut)] + let mut anys = anys.into_iter(); + Some(($( + anys.next().expect("Expect `anys` to be of length `SIZE`").downcast_mut::<$name>()?, + )*)) + } + } } } + +impl_native_tuple!(0); +impl_native_tuple!(1, A); +impl_native_tuple!(2, A, B); +impl_native_tuple!(3, A, B, C); +impl_native_tuple!(4, A, B, C, D); +impl_native_tuple!(5, A, B, C, D, E); +impl_native_tuple!(6, A, B, C, D, E, F); +impl_native_tuple!(7, A, B, C, D, E, F, G); +impl_native_tuple!(8, A, B, C, D, E, F, G, H); +impl_native_tuple!(9, A, B, C, D, E, F, G, H, I); +impl_native_tuple!(10, A, B, C, D, E, F, G, H, I, J); +impl_native_tuple!(11, A, B, C, D, E, F, G, H, I, J, K); +impl_native_tuple!(12, A, B, C, D, E, F, G, H, I, J, K, L); diff --git a/boa_engine/src/realm.rs b/boa_engine/src/realm.rs index f0ad3c88da5..69c9f15f029 100644 --- a/boa_engine/src/realm.rs +++ b/boa_engine/src/realm.rs @@ -21,7 +21,7 @@ use crate::{ object::shape::RootShape, HostDefined, JsObject, JsString, }; -use boa_gc::{Finalize, Gc, GcRefCell, Trace}; +use boa_gc::{Finalize, Gc, GcRef, GcRefCell, GcRefMut, Trace}; use boa_profiler::Profiler; /// Representation of a Realm. @@ -61,7 +61,7 @@ struct Inner { loaded_modules: GcRefCell>, host_classes: GcRefCell>, - host_defined: HostDefined, + host_defined: GcRefCell, } impl Realm { @@ -86,7 +86,7 @@ impl Realm { template_map: GcRefCell::default(), loaded_modules: GcRefCell::default(), host_classes: GcRefCell::default(), - host_defined: HostDefined::default(), + host_defined: GcRefCell::default(), }), }; @@ -102,13 +102,32 @@ impl Realm { &self.inner.intrinsics } - /// Returns the [`ECMAScript specification`][spec] defined [`\[\[\HostDefined]\]`][`HostDefined`] field of the [`Realm`]. + /// Returns an immutable reference to the [`ECMAScript specification`][spec] defined + /// [`\[\[\HostDefined]\]`][`HostDefined`] field of the [`Realm`]. /// /// [spec]: https://tc39.es/ecma262/#table-realm-record-fields + /// + /// # Panics + /// + /// Panics if [`HostDefined`] field is mutably borrowed. + #[inline] + #[must_use] + pub fn host_defined(&self) -> GcRef<'_, HostDefined> { + self.inner.host_defined.borrow() + } + + /// Returns a mutable reference to [`ECMAScript specification`][spec] defined + /// [`\[\[\HostDefined]\]`][`HostDefined`] field of the [`Realm`]. + /// + /// [spec]: https://tc39.es/ecma262/#table-realm-record-fields + /// + /// # Panics + /// + /// Panics if [`HostDefined`] field is borrowed. #[inline] #[must_use] - pub fn host_defined(&self) -> &HostDefined { - &self.inner.host_defined + pub fn host_defined_mut(&self) -> GcRefMut<'_, HostDefined> { + self.inner.host_defined.borrow_mut() } /// Checks if this `Realm` has the class `C` registered into its class map. diff --git a/boa_examples/src/bin/host_defined.rs b/boa_examples/src/bin/host_defined.rs index c60d4c4dcfb..69399f0cf1d 100644 --- a/boa_examples/src/bin/host_defined.rs +++ b/boa_examples/src/bin/host_defined.rs @@ -1,7 +1,8 @@ // This example goes into the details on how to store user defined structs/state that is shared. use boa_engine::{ - js_string, native_function::NativeFunction, Context, JsArgs, JsError, JsNativeError, Source, + js_string, native_function::NativeFunction, Context, JsArgs, JsError, JsNativeError, JsValue, + Source, }; use boa_gc::{Finalize, Trace}; @@ -25,6 +26,13 @@ impl AnotherCustomHostDefinedStruct { } } +/// Custom host-defined struct that tracks the number of calls to the `getRealmValue` and `setRealmValue` functions. +#[derive(Default, Trace, Finalize)] +struct HostDefinedMetrics { + #[unsafe_ignore_trace] + counter: usize, +} + fn main() -> Result<(), JsError> { // We create a new `Context` to create a new Javascript executor.. let mut context = Context::default(); @@ -34,14 +42,15 @@ fn main() -> Result<(), JsError> { // Insert a default CustomHostDefinedStruct. realm - .host_defined() + .host_defined_mut() .insert_default::(); { assert!(realm.host_defined().has::()); // Get the [[HostDefined]] field from the realm and downcast it to our concrete type. - let Some(host_defined) = realm.host_defined().get::() else { + let host_defined = realm.host_defined(); + let Some(host_defined) = host_defined.get::() else { return Err(JsNativeError::typ() .with_message("Realm does not have HostDefined field") .into()); @@ -53,15 +62,15 @@ fn main() -> Result<(), JsError> { // Insert another struct with state into [[HostDefined]] field. realm - .host_defined() + .host_defined_mut() .insert(AnotherCustomHostDefinedStruct::new(10)); { assert!(realm.host_defined().has::()); // Get the [[HostDefined]] field from the realm and downcast it to our concrete type. - let Some(host_defined) = realm.host_defined().get::() - else { + let host_defined = realm.host_defined(); + let Some(host_defined) = host_defined.get::() else { return Err(JsNativeError::typ() .with_message("Realm does not have HostDefined field") .into()); @@ -73,7 +82,7 @@ fn main() -> Result<(), JsError> { // Remove a type from the [[HostDefined]] field. assert!(realm - .host_defined() + .host_defined_mut() .remove::() .is_some()); @@ -86,14 +95,17 @@ fn main() -> Result<(), JsError> { NativeFunction::from_fn_ptr(|_, args, context| { let value: usize = args.get_or_undefined(0).try_js_into(context)?; - let host_defined = context.realm().host_defined(); - let Some(mut host_defined) = host_defined.get_mut::() else { + let mut host_defined = context.realm().host_defined_mut(); + let Some((host_defined, metrics)) = + host_defined.get_many_mut::<(CustomHostDefinedStruct, HostDefinedMetrics), 2>() + else { return Err(JsNativeError::typ() .with_message("Realm does not have HostDefined field") .into()); }; host_defined.counter = value; + metrics.counter += 1; Ok(value.into()) }), @@ -103,17 +115,34 @@ fn main() -> Result<(), JsError> { js_string!("getRealmValue"), 0, NativeFunction::from_fn_ptr(|_, _, context| { - let host_defined = context.realm().host_defined(); - let Some(host_defined) = host_defined.get::() else { + let mut host_defined = context.realm().host_defined_mut(); + + let value: JsValue = { + let Some(host_defined) = host_defined.get::() else { + return Err(JsNativeError::typ() + .with_message("Realm does not have HostDefined field") + .into()); + }; + host_defined.counter.into() + }; + + let Some(metrics) = host_defined.get_mut::() else { return Err(JsNativeError::typ() .with_message("Realm does not have HostDefined field") .into()); }; - Ok(host_defined.counter.into()) + metrics.counter += 1; + + Ok(value) }), )?; + // Insert HostDefinedMetrics into the [[HostDefined]] field. + realm + .host_defined_mut() + .insert_default::(); + // Run code in JavaScript that mutates the host-defined field on the Realm. context.eval(Source::from_bytes( r" @@ -122,14 +151,24 @@ fn main() -> Result<(), JsError> { ", ))?; - let Some(host_defined) = realm.host_defined().get::() else { + let host_defined = realm.host_defined(); + let Some(host_defined_value) = host_defined.get::() else { return Err(JsNativeError::typ() .with_message("Realm does not have HostDefined field") .into()); }; // Assert that the host-defined field changed. - assert_eq!(host_defined.counter, 100); + assert_eq!(host_defined_value.counter, 100); + + let Some(metrics) = host_defined.get::() else { + return Err(JsNativeError::typ() + .with_message("Realm does not have HostDefined field") + .into()); + }; + + // Assert that we called the getRealmValue and setRealmValue functions (3 times in total) + assert_eq!(metrics.counter, 3); Ok(()) } diff --git a/boa_gc/src/trace.rs b/boa_gc/src/trace.rs index 070a93ffc58..c1de3d44dbb 100644 --- a/boa_gc/src/trace.rs +++ b/boa_gc/src/trace.rs @@ -349,6 +349,22 @@ unsafe impl Trace for BTreeSet { }); } +impl Finalize + for hashbrown::hash_map::HashMap +{ +} +// SAFETY: All the elements of the `HashMap` are correctly marked. +unsafe impl Trace + for hashbrown::hash_map::HashMap +{ + custom_trace!(this, { + for (k, v) in this { + mark(k); + mark(v); + } + }); +} + impl Finalize for HashMap {} // SAFETY: All the elements of the `HashMap` are correctly marked. unsafe impl Trace for HashMap {