Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find roots when running GC rather than runtime #3109

Merged
merged 9 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Keep reference counts in Box
  • Loading branch information
tunz committed Jul 19, 2023
commit a12dc2791b4815770f6a473d54ec3b617cac1f36
18 changes: 6 additions & 12 deletions boa_engine/src/native_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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 {
Expand Down Expand Up @@ -262,14 +254,16 @@ impl NativeFunction {
{
// Hopefully, this unsafe operation will be replaced by the `CoerceUnsized` API in the
// future: https://github.com/rust-lang/rust/issues/18598
let (ptr, handle) = Gc::into_raw(Gc::new(Closure {
let ptr = Gc::into_raw(Gc::new(Closure {
f: closure,
captures,
}));
// SAFETY: The pointer returned by `into_raw` is only used to coerce to a trait object,
// meaning this is safe.
Self {
inner: Inner::Closure(Gc::from_raw(ptr, handle)),
unsafe {
Self {
inner: Inner::Closure(Gc::from_raw(ptr)),
}
}
}

Expand Down
47 changes: 45 additions & 2 deletions boa_gc/src/internals/ephemeron_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use std::{
/// `Collector` during the sweep phase.
pub(crate) struct EphemeronBoxHeader {
marked: Cell<bool>,
ref_count: Cell<u32>,
non_root_count: Cell<u32>,
pub(crate) next: Cell<Option<NonNull<dyn ErasedEphemeronBox>>>,
}

Expand All @@ -23,6 +25,8 @@ impl EphemeronBoxHeader {
pub(crate) fn new() -> Self {
Self {
marked: Cell::new(false),
ref_count: Cell::new(1),
non_root_count: Cell::new(0),
next: Cell::new(None),
}
}
Expand All @@ -47,6 +51,8 @@ impl core::fmt::Debug for EphemeronBoxHeader {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("EphemeronBoxHeader")
.field("marked", &self.is_marked())
.field("ref_count", &self.ref_count.get())
.field("non_root_count", &self.non_root_count.get())
.finish()
}
}
Expand Down Expand Up @@ -106,6 +112,23 @@ impl<K: Trace + ?Sized, V: Trace> EphemeronBox<K, V> {
pub(crate) unsafe fn mark(&self) {
self.header.mark();
}

#[inline]
pub(crate) fn inc_ref_count(&self) {
self.header.ref_count.set(self.header.ref_count.get() + 1);
}

#[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);
}
}

pub(crate) trait ErasedEphemeronBox {
Expand All @@ -120,6 +143,12 @@ pub(crate) trait ErasedEphemeronBox {

fn trace_non_roots(&self);

fn get_ref_count(&self) -> u32;

fn get_non_root_count(&self) -> u32;

fn reset_non_root_count(&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);
Expand Down Expand Up @@ -166,12 +195,26 @@ impl<K: Trace + ?Sized, V: Trace> ErasedEphemeronBox for EphemeronBox<K, V> {
};
}

#[inline]
fn get_ref_count(&self) -> u32 {
self.header.ref_count.get()
}

#[inline]
fn get_non_root_count(&self) -> u32 {
self.header.non_root_count.get()
}

#[inline]
fn reset_non_root_count(&self) {
self.header.non_root_count.set(0);
}

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()) };
}
}
}
37 changes: 37 additions & 0 deletions boa_gc/src/internals/gc_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use std::{
/// `Collector` during the sweep phase.
pub(crate) struct GcBoxHeader {
marked: Cell<bool>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts on using a bit in the ref_count here over a full bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hesitant to keep the mark bit in ref_count since there was 1~2% overhead, but it seems like putting the mark bit into non_root_count does not have that much of overhead. Updated to include the mark bit in non_root_count.

ref_count: Cell<u32>,
non_root_count: Cell<u32>,
pub(crate) next: Cell<Option<NonNull<GcBox<dyn Trace>>>>,
}

Expand All @@ -23,6 +25,8 @@ impl GcBoxHeader {
pub(crate) fn new() -> Self {
Self {
marked: Cell::new(false),
ref_count: Cell::new(1),
non_root_count: Cell::new(0),
next: Cell::new(None),
}
}
Expand All @@ -47,6 +51,8 @@ impl fmt::Debug for GcBoxHeader {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("GcBoxHeader")
.field("marked", &self.is_marked())
.field("ref_count", &self.ref_count.get())
.field("non_root_count", &self.non_root_count.get())
.finish()
}
}
Expand Down Expand Up @@ -99,4 +105,35 @@ impl<T: Trace + ?Sized> GcBox<T> {
pub(crate) fn is_marked(&self) -> bool {
self.header.is_marked()
}

#[inline]
pub(crate) fn get_ref_count(&self) -> u32 {
self.header.ref_count.get()
}

#[inline]
pub(crate) fn inc_ref_count(&self) {
self.header.ref_count.set(self.header.ref_count.get() + 1);
}

#[inline]
pub(crate) fn dec_ref_count(&self) {
self.header.ref_count.set(self.header.ref_count.get() - 1);
}

#[inline]
pub(crate) fn get_non_root_count(&self) -> u32 {
self.header.non_root_count.get()
}

#[inline]
pub(crate) fn inc_non_root_count(&self) {
self.header
.non_root_count
.set(self.header.non_root_count.get() + 1);
}

pub(crate) fn reset_non_root_count(&self) {
self.header.non_root_count.set(0);
}
}
Loading