`[[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

@@ -406,6 +406,7 @@ dependencies = [
  "fixed_decimal",
  "float-cmp",
  "futures-lite 2.0.1",
+ "hashbrown 0.14.2",
  "icu_calendar",
  "icu_casemap",
  "icu_collator",

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 }

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<TypeId, Box<dyn NativeObject>>;
-
 /// 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<HostDefinedMap>,
+    // INVARIANT: All key-value pairs `(id, obj)` satisfy:
+    //  `id == TypeId::of::<T>() && obj.is::<T>()`
+    // for some type `T : NativeObject`.
+    types: HashMap<TypeId, Box<dyn NativeObject>>,
+}
+
+// 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<T: NativeObject>(obj: Box<dyn NativeObject>) -> Box<T> {
+    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::<T>()) }
 }
 
 impl HostDefined {
     /// Insert a type into the [`HostDefined`].
-    ///
-    /// # Panics
-    ///
-    /// Panics if [`HostDefined`] field is borrowed.
     #[track_caller]
-    pub fn insert_default<T: NativeObject + Default>(&self) -> Option<Box<dyn NativeObject>> {
+    pub fn insert_default<T: NativeObject + Default>(&mut self) -> Option<Box<T>> {
-        self.state
+        self.types
-            .borrow_mut()
             .insert(TypeId::of::<T>(), Box::<T>::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<T: NativeObject>(&self, value: T) -> Option<Box<dyn NativeObject>> {
+    pub fn insert<T: NativeObject>(&mut self, value: T) -> Option<Box<T>> {
-        self.state
+        self.types
-            .borrow_mut()
             .insert(TypeId::of::<T>(), Box::new(value))
+            .map(downcast_boxed_native_object_unchecked)
     }
 
     /// Check if the [`HostDefined`] has type T.
-    ///
+    #[must_use]
-    /// # Panics
-    ///
-    /// Panics if [`HostDefined`] field is borrowed mutably.
     #[track_caller]
     pub fn has<T: NativeObject>(&self) -> bool {
-        self.state.borrow().contains_key(&TypeId::of::<T>())
+        self.types.contains_key(&TypeId::of::<T>())
     }
 
     /// 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<T: NativeObject>(&self) -> Option<Box<dyn NativeObject>> {
+    pub fn remove<T: NativeObject>(&mut self) -> Option<Box<T>> {
-        self.state.borrow_mut().remove(&TypeId::of::<T>())
+        self.types
+            .remove(&TypeId::of::<T>())
+            .map(downcast_boxed_native_object_unchecked)
     }
 
-    /// Get type T from [`HostDefined`], if it exits.
+    /// Get type T from [`HostDefined`], if it exists.
-    ///
-    /// # Panics
-    ///
-    /// Panics if [`HostDefined`] field is borrowed.
     #[track_caller]
-    pub fn get<T: NativeObject>(&self) -> Option<GcRef<'_, T>> {
+    pub fn get<T: NativeObject>(&self) -> Option<&T> {
-        GcRef::try_map(self.state.borrow(), |state| {
+        self.types
-            state
+            .get(&TypeId::of::<T>())
-                .get(&TypeId::of::<T>())
+            .map(Box::as_ref)
-                .map(Box::as_ref)
+            .and_then(<dyn NativeObject>::downcast_ref::<T>)
-                .and_then(<dyn NativeObject>::downcast_ref::<T>)
-        })
     }
 
-    /// Get type T from [`HostDefined`], if it exits.
+    /// Get type T from [`HostDefined`], if it exists.
-    ///
-    /// # Panics
-    ///
-    /// Panics if [`HostDefined`] field is borrowed.
     #[track_caller]
-    pub fn get_mut<T: NativeObject>(&self) -> Option<GcRefMut<'_, HostDefinedMap, T>> {
+    pub fn get_mut<T: NativeObject>(&mut self) -> Option<&mut T> {
-        GcRefMut::try_map(
+        self.types
-            self.state.borrow_mut(),
+            .get_mut(&TypeId::of::<T>())
-            |state: &mut FxHashMap<TypeId, Box<dyn NativeObject>>| {
+            .map(Box::as_mut)
-                state
+            .and_then(<dyn NativeObject>::downcast_mut::<T>)
-                    .get_mut(&TypeId::of::<T>())
+    }
-                    .map(Box::as_mut)
+
-                    .and_then(<dyn NativeObject>::downcast_mut::<T>)
+    /// Get type a tuple of types from [`HostDefined`], if they exist.
-            },
+    #[track_caller]
-        )
+    pub fn get_many_mut<T, const SIZE: usize>(&mut self) -> Option<T::NativeTupleMutRef<'_>>
+    where
+        T: NativeTuple<SIZE>,
+    {
+        let ids = T::as_type_ids();
+        let refs: [&TypeId; SIZE] = ids
+            .iter()
+            .collect::<Vec<_>>()
+            .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) {
+    pub fn clear(&mut self) {
-        self.state.borrow_mut().clear();
+        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<const SIZE: usize> {
+    type NativeTupleMutRef<'a>;
+
+    fn as_type_ids() -> Vec<TypeId>;
+
+    fn mut_ref_from_anys(
+        anys: [&'_ mut Box<dyn NativeObject>; SIZE],
+    ) -> Option<Self::NativeTupleMutRef<'_>>;
+}
+
+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<TypeId> {
+                vec![$(TypeId::of::<$name>(),)*]
+            }
+
+            fn mut_ref_from_anys(
+                anys: [&'_ mut Box<dyn NativeObject>; $size],
+            ) -> Option<Self::NativeTupleMutRef<'_>> {
+                #[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);

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<FxHashMap<JsString, Module>>,
     host_classes: GcRefCell<FxHashMap<TypeId, StandardConstructor>>,
 
-    host_defined: HostDefined,
+    host_defined: GcRefCell<HostDefined>,
 }
 
 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 {
+    pub fn host_defined_mut(&self) -> GcRefMut<'_, HostDefined> {
-        &self.inner.host_defined
+        self.inner.host_defined.borrow_mut()
     }
 
     /// Checks if this `Realm` has the class `C` registered into its class map.

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::<CustomHostDefinedStruct>();
 
     {
         assert!(realm.host_defined().has::<CustomHostDefinedStruct>());
 
         // Get the [[HostDefined]] field from the realm and downcast it to our concrete type.
-        let Some(host_defined) = realm.host_defined().get::<CustomHostDefinedStruct>() else {
+        let host_defined = realm.host_defined();
+        let Some(host_defined) = host_defined.get::<CustomHostDefinedStruct>() 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::<AnotherCustomHostDefinedStruct>());
 
         // Get the [[HostDefined]] field from the realm and downcast it to our concrete type.
-        let Some(host_defined) = realm.host_defined().get::<AnotherCustomHostDefinedStruct>()
+        let host_defined = realm.host_defined();
-        else {
+        let Some(host_defined) = host_defined.get::<AnotherCustomHostDefinedStruct>() 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::<AnotherCustomHostDefinedStruct>()
         .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 mut host_defined = context.realm().host_defined_mut();
-            let Some(mut host_defined) = host_defined.get_mut::<CustomHostDefinedStruct>() else {
+            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 mut host_defined = context.realm().host_defined_mut();
-            let Some(host_defined) = host_defined.get::<CustomHostDefinedStruct>() else {
+
+            let value: JsValue = {
+                let Some(host_defined) = host_defined.get::<CustomHostDefinedStruct>() 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::<HostDefinedMetrics>() 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::<HostDefinedMetrics>();
+
     // 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::<CustomHostDefinedStruct>() else {
+    let host_defined = realm.host_defined();
+    let Some(host_defined_value) = host_defined.get::<CustomHostDefinedStruct>() 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::<HostDefinedMetrics>() 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(())
 }

boa_gc/src/trace.rs

@@ -349,6 +349,22 @@ unsafe impl<T: Trace> Trace for BTreeSet<T> {
     });
 }
 
+impl<K: Eq + Hash + Trace, V: Trace, S: BuildHasher> Finalize
+    for hashbrown::hash_map::HashMap<K, V, S>
+{
+}
+// SAFETY: All the elements of the `HashMap` are correctly marked.
+unsafe impl<K: Eq + Hash + Trace, V: Trace, S: BuildHasher> Trace
+    for hashbrown::hash_map::HashMap<K, V, S>
+{
+    custom_trace!(this, {
+        for (k, v) in this {
+            mark(k);
+            mark(v);
+        }
+    });
+}
+
 impl<K: Eq + Hash + Trace, V: Trace, S: BuildHasher> Finalize for HashMap<K, V, S> {}
 // SAFETY: All the elements of the `HashMap` are correctly marked.
 unsafe impl<K: Eq + Hash + Trace, V: Trace, S: BuildHasher> Trace for HashMap<K, V, S> {