Find roots when running GC rather than runtime

boa_engine/src/builtins/array/mod.rs

@@ -1064,19 +1064,21 @@ impl Array {
             // d. Let lowerExists be ? HasProperty(O, lowerP).
             let lower_exists = o.has_property(lower, context)?;
             // e. If lowerExists is true, then
-            let mut lower_value = JsValue::undefined();
-            if lower_exists {
+            let lower_value = if lower_exists {
                 // i. Let lowerValue be ? Get(O, lowerP).
-                lower_value = o.get(lower, context)?;
-            }
+                o.get(lower, context)?
+            } else {
+                JsValue::undefined()
+            };
             // f. Let upperExists be ? HasProperty(O, upperP).
             let upper_exists = o.has_property(upper, context)?;
             // g. If upperExists is true, then
-            let mut upper_value = JsValue::undefined();
-            if upper_exists {
+            let upper_value = if upper_exists {
                 // i. Let upperValue be ? Get(O, upperP).
-                upper_value = o.get(upper, context)?;
-            }
+                o.get(upper, context)?
+            } else {
+                JsValue::undefined()
+            };
             match (lower_exists, upper_exists) {
                 // h. If lowerExists is true and upperExists is true, then
                 (true, true) => {

boa_engine/src/lib.rs

@@ -118,7 +118,8 @@
     clippy::cast_possible_truncation,
     clippy::cast_sign_loss,
     clippy::cast_precision_loss,
-    clippy::cast_possible_wrap
+    clippy::cast_possible_wrap,
+    clippy::must_use_candidate,
 )]
 
 extern crate static_assertions as sa;

boa_engine/src/native_function.rs

@@ -69,7 +69,7 @@ where
 /// to use. All other closures can also be stored in a `NativeFunction`, albeit by using an `unsafe`
 /// API, but note that passing closures implicitly capturing traceable types could cause
 /// **Undefined Behaviour**.
-#[derive(Clone)]
+#[derive(Clone, Finalize)]
 pub struct NativeFunction {
     inner: Inner,
 }
@@ -80,14 +80,6 @@ enum Inner {
     Closure(Gc<dyn TraceableClosure>),
 }
 
-impl Finalize for NativeFunction {
-    fn finalize(&self) {
-        if let Inner::Closure(c) = &self.inner {
-            c.finalize();
-        }
-    }
-}
-
 // Manual implementation because deriving `Trace` triggers the `single_use_lifetimes` lint.
 // SAFETY: Only closures can contain `Trace` captures, so this implementation is safe.
 unsafe impl Trace for NativeFunction {

boa_gc/src/cell.rs

@@ -26,53 +26,35 @@ enum BorrowState {
     Unused,
 }
 
-const ROOT: usize = 1;
-const WRITING: usize = !1;
+const WRITING: usize = !0;
 const UNUSED: usize = 0;
 
 /// The base borrowflag init is rooted, and has no outstanding borrows.
-const BORROWFLAG_INIT: BorrowFlag = BorrowFlag(ROOT);
+const BORROWFLAG_INIT: BorrowFlag = BorrowFlag(UNUSED);
 
 impl BorrowFlag {
     /// Check the current `BorrowState` of `BorrowFlag`.
     const fn borrowed(self) -> BorrowState {
-        match self.0 & !ROOT {
+        match self.0 {
             UNUSED => BorrowState::Unused,
             WRITING => BorrowState::Writing,
             _ => BorrowState::Reading,
         }
     }
 
-    /// Check whether the borrow bit is flagged.
-    const fn rooted(self) -> bool {
-        self.0 & ROOT > 0
-    }
-
     /// Set the `BorrowFlag`'s state to writing.
     const fn set_writing(self) -> Self {
-        // Set every bit other than the root bit, which is preserved
         Self(self.0 | WRITING)
     }
 
-    /// Remove the root flag on `BorrowFlag`
-    const fn set_unused(self) -> Self {
-        // Clear every bit other than the root bit, which is preserved
-        Self(self.0 & ROOT)
-    }
-
     /// Increments the counter for a new borrow.
     ///
     /// # Panic
     ///  - This method will panic if the current `BorrowState` is writing.
     ///  - This method will panic after incrementing if the borrow count overflows.
     fn add_reading(self) -> Self {
         assert!(self.borrowed() != BorrowState::Writing);
-        // Add 1 to the integer starting at the second binary digit. As our
-        // borrowstate is not writing, we know that overflow cannot happen, so
-        // this is equivalent to the following, more complicated, expression:
-        //
-        // BorrowFlag((self.0 & ROOT) | (((self.0 >> 1) + 1) << 1))
-        let flags = Self(self.0 + 0b10);
+        let flags = Self(self.0 + 1);
 
         // This will fail if the borrow count overflows, which shouldn't happen,
         // but let's be safe
@@ -88,26 +70,13 @@ impl BorrowFlag {
     ///  - This method will panic if the current `BorrowState` is not reading.
     fn sub_reading(self) -> Self {
         assert!(self.borrowed() == BorrowState::Reading);
-        // Subtract 1 from the integer starting at the second binary digit. As
-        // our borrowstate is not writing or unused, we know that overflow or
-        // undeflow cannot happen, so this is equivalent to the following, more
-        // complicated, expression:
-        //
-        // BorrowFlag((self.0 & ROOT) | (((self.0 >> 1) - 1) << 1))
-        Self(self.0 - 0b10)
-    }
-
-    /// Set the root flag on the `BorrowFlag`.
-    fn set_rooted(self, rooted: bool) -> Self {
-        // Preserve the non-root bits
-        Self((self.0 & !ROOT) | (usize::from(rooted)))
+        Self(self.0 - 1)
     }
 }
 
 impl Debug for BorrowFlag {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("BorrowFlag")
-            .field("Rooted", &self.rooted())
             .field("State", &self.borrowed())
             .finish()
     }
@@ -214,12 +183,6 @@ impl<T: Trace + ?Sized> GcRefCell<T> {
         // SAFETY: This is safe as the value is rooted if it was not previously rooted,
         // so it cannot be dropped.
         unsafe {
-            // Force the val_ref's contents to be rooted for the duration of the
-            // mutable borrow
-            if !self.flags.get().rooted() {
-                (*self.cell.get()).root();
-            }
-
             Ok(GcRefMut {
                 gc_cell: self,
                 value: &mut *self.cell.get(),
@@ -263,25 +226,11 @@ unsafe impl<T: Trace + ?Sized> Trace for GcRefCell<T> {
         }
     }
 
-    unsafe fn root(&self) {
-        assert!(!self.flags.get().rooted(), "Can't root a GcCell twice!");
-        self.flags.set(self.flags.get().set_rooted(true));
-
-        match self.flags.get().borrowed() {
-            BorrowState::Writing => (),
-            // SAFETY: Please see GcCell's Trace impl Safety note.
-            _ => unsafe { (*self.cell.get()).root() },
-        }
-    }
-
-    unsafe fn unroot(&self) {
-        assert!(self.flags.get().rooted(), "Can't unroot a GcCell twice!");
-        self.flags.set(self.flags.get().set_rooted(false));
-
+    fn trace_non_roots(&self) {
         match self.flags.get().borrowed() {
             BorrowState::Writing => (),
             // SAFETY: Please see GcCell's Trace impl Safety note.
-            _ => unsafe { (*self.cell.get()).unroot() },
+            _ => unsafe { (*self.cell.get()).trace_non_roots() },
         }
     }
 
@@ -407,12 +356,12 @@ impl<T: ?Sized + Display> Display for GcRef<'_, T> {
 }
 
 /// A wrapper type for a mutably borrowed value from a `GcCell<T>`.
-pub struct GcRefMut<'a, T: Trace + ?Sized + 'static, U: ?Sized = T> {
+pub struct GcRefMut<'a, T: ?Sized + 'static, U: ?Sized = T> {
     pub(crate) gc_cell: &'a GcRefCell<T>,
     pub(crate) value: &'a mut U,
 }
 
-impl<'a, T: Trace + ?Sized, U: ?Sized> GcRefMut<'a, T, U> {
+impl<'a, T: ?Sized, U: ?Sized> GcRefMut<'a, T, U> {
     /// Makes a new `GcCellRefMut` for a component of the borrowed data, e.g., an enum
     /// variant.
     ///
@@ -457,21 +406,10 @@ impl<T: Trace + ?Sized, U: ?Sized> DerefMut for GcRefMut<'_, T, U> {
     }
 }
 
-impl<T: Trace + ?Sized, U: ?Sized> Drop for GcRefMut<'_, T, U> {
+impl<T: ?Sized, U: ?Sized> Drop for GcRefMut<'_, T, U> {
     fn drop(&mut self) {
         debug_assert!(self.gc_cell.flags.get().borrowed() == BorrowState::Writing);
-        // Restore the rooted state of the GcCell's contents to the state of the GcCell.
-        // During the lifetime of the GcCellRefMut, the GcCell's contents are rooted.
-        if !self.gc_cell.flags.get().rooted() {
-            // SAFETY: If `GcCell` is no longer rooted, then unroot it. This should be safe
-            // as the internal `GcBox` should be guaranteed to have at least 1 root.
-            unsafe {
-                (*self.gc_cell.cell.get()).unroot();
-            }
-        }
-        self.gc_cell
-            .flags
-            .set(self.gc_cell.flags.get().set_unused());
+        self.gc_cell.flags.set(BorrowFlag(UNUSED));
     }
 }
 

boa_gc/src/internals/ephemeron_box.rs

@@ -4,11 +4,6 @@ use std::{
     ptr::{self, NonNull},
 };
 
-// Age and Weak Flags
-const MARK_MASK: usize = 1 << (usize::BITS - 1);
-const ROOTS_MASK: usize = !MARK_MASK;
-const ROOTS_MAX: usize = ROOTS_MASK;
-
 /// The `EphemeronBoxHeader` contains the `EphemeronBoxHeader`'s current state for the `Collector`'s
 /// Mark/Sweep as well as a pointer to the next ephemeron in the heap.
 ///
@@ -19,65 +14,60 @@ const ROOTS_MAX: usize = ROOTS_MASK;
 /// The next node is set by the `Allocator` during initialization and by the
 /// `Collector` during the sweep phase.
 pub(crate) struct EphemeronBoxHeader {
-    roots: Cell<usize>,
+    marked: Cell<bool>,
+    ref_count: Cell<u32>,
+    non_root_count: Cell<u32>,
     pub(crate) next: Cell<Option<NonNull<dyn ErasedEphemeronBox>>>,
 }
 
 impl EphemeronBoxHeader {
     /// Creates a new `EphemeronBoxHeader` with a root of 1 and next set to None.
     pub(crate) fn new() -> Self {
         Self {
-            roots: Cell::new(1),
+            marked: Cell::new(false),
+            ref_count: Cell::new(1),
+            non_root_count: Cell::new(0),
             next: Cell::new(None),
         }
     }
 
-    /// Returns the `EphemeronBoxHeader`'s current root count
-    pub(crate) fn roots(&self) -> usize {
-        self.roots.get() & ROOTS_MASK
-    }
-
-    /// Increments `EphemeronBoxHeader`'s root count.
-    pub(crate) fn inc_roots(&self) {
-        let roots = self.roots.get();
-
-        if (roots & ROOTS_MASK) < ROOTS_MAX {
-            self.roots.set(roots + 1);
-        } else {
-            // TODO: implement a better way to handle root overload.
-            panic!("roots counter overflow");
-        }
-    }
-
-    /// Decreases `EphemeronBoxHeader`'s current root count.
-    pub(crate) fn dec_roots(&self) {
-        // Underflow check as a stop gap for current issue when dropping.
-        if self.roots.get() > 0 {
-            self.roots.set(self.roots.get() - 1);
-        }
-    }
-
     /// Returns a bool for whether `EphemeronBoxHeader`'s mark bit is 1.
     pub(crate) fn is_marked(&self) -> bool {
-        self.roots.get() & MARK_MASK != 0
+        self.marked.get()
     }
 
     /// Sets `EphemeronBoxHeader`'s mark bit to 1.
     pub(crate) fn mark(&self) {
-        self.roots.set(self.roots.get() | MARK_MASK);
+        self.marked.set(true);
     }
 
     /// Sets `EphemeronBoxHeader`'s mark bit to 0.
     pub(crate) fn unmark(&self) {
-        self.roots.set(self.roots.get() & !MARK_MASK);
+        self.marked.set(false);
+    }
+
+    /// Returns a reference count.
+    pub(crate) fn get_ref_count(&self) -> u32 {
+        self.ref_count.get()
+    }
+
+    /// Returns a count for non-roots.
+    pub(crate) fn get_non_root_count(&self) -> u32 {
+        self.non_root_count.get()
+    }
+
+    /// Reset `non_root_count` to zero.
+    pub(crate) fn reset_non_root_count(&self) {
+        self.non_root_count.set(0);
     }
 }
 
 impl core::fmt::Debug for EphemeronBoxHeader {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         f.debug_struct("EphemeronBoxHeader")
-            .field("roots", &self.roots())
             .field("marked", &self.is_marked())
+            .field("ref_count", &self.ref_count.get())
+            .field("non_root_count", &self.non_root_count.get())
             .finish()
     }
 }
@@ -138,18 +128,21 @@ impl<K: Trace + ?Sized, V: Trace> EphemeronBox<K, V> {
         self.header.mark();
     }
 
-    /// Increases the root count on this `EphemeronBox`.
-    ///
-    /// Roots prevent the `EphemeronBox` from being destroyed by the garbage collector.
-    pub(crate) fn root(&self) {
-        self.header.inc_roots();
+    #[inline]
+    pub(crate) fn inc_ref_count(&self) {
+        self.header.ref_count.set(self.header.ref_count.get() + 1);
     }
 
-    /// Decreases the root count on this `EphemeronBox`.
-    ///
-    /// Roots prevent the `EphemeronBox` from being destroyed by the garbage collector.
-    pub(crate) fn unroot(&self) {
-        self.header.dec_roots();
+    #[inline]
+    pub(crate) fn dec_ref_count(&self) {
+        self.header.ref_count.set(self.header.ref_count.get() - 1);
+    }
+
+    #[inline]
+    pub(crate) fn inc_non_root_count(&self) {
+        self.header
+            .non_root_count
+            .set(self.header.non_root_count.get() + 1);
     }
 }
 
@@ -163,6 +156,8 @@ pub(crate) trait ErasedEphemeronBox {
     /// "successfully traced".
     unsafe fn trace(&self) -> bool;
 
+    fn trace_non_roots(&self);
+
     /// Runs the finalization logic of the `EphemeronBox`'s held value, if the key is still live,
     /// and clears its contents.
     fn finalize_and_clear(&self);
@@ -199,12 +194,21 @@ impl<K: Trace + ?Sized, V: Trace> ErasedEphemeronBox for EphemeronBox<K, V> {
         is_key_marked
     }
 
+    fn trace_non_roots(&self) {
+        let Some(data) = self.data.get() else {
+            return;
+        };
+        // SAFETY: `data` comes from a `Box`, so it is safe to dereference.
+        unsafe {
+            data.as_ref().value.trace_non_roots();
+        };
+    }
+
     fn finalize_and_clear(&self) {
         if let Some(data) = self.data.take() {
             // SAFETY: `data` comes from an `into_raw` call, so this pointer is safe to pass to
             // `from_raw`.
-            let contents = unsafe { Box::from_raw(data.as_ptr()) };
-            Trace::run_finalizer(&contents.value);
+            let _contents = unsafe { Box::from_raw(data.as_ptr()) };
         }
     }
 }