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

Reduce WeakGc<T> memory usage #3492

Merged
merged 2 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
18 changes: 15 additions & 3 deletions boa_gc/src/internals/ephemeron_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,21 +139,33 @@ impl<K: Trace, V: Trace> EphemeronBox<K, V> {
data.as_ref().map(|data| &data.value)
}

/// Returns a reference to the ephemeron's key or None.
/// Returns the pointer to the ephemeron's key or None.
///
/// # Safety
///
/// The caller must ensure there are no live mutable references to the ephemeron box's data
/// before calling this method.
pub(crate) unsafe fn key(&self) -> Option<&GcBox<K>> {
pub(crate) unsafe fn key_ptr(&self) -> Option<NonNull<GcBox<K>>> {
// SAFETY: the garbage collector ensures the ephemeron doesn't mutate until
// finalization.
unsafe {
let data = &*self.data.get();
data.as_ref().map(|data| data.key.as_ref())
data.as_ref().map(|data| data.key)
}
}

/// Returns a reference to the ephemeron's key or None.
///
/// # Safety
///
/// The caller must ensure there are no live mutable references to the ephemeron box's data
/// before calling this method.
pub(crate) unsafe fn key(&self) -> Option<&GcBox<K>> {
// SAFETY: the garbage collector ensures the ephemeron doesn't mutate until
// finalization.
unsafe { self.key_ptr().map(|data| data.as_ref()) }
}

/// Marks this `EphemeronBox` as live.
///
/// This doesn't mark the inner value of the ephemeron. [`ErasedEphemeronBox::trace`]
Expand Down
17 changes: 17 additions & 0 deletions boa_gc/src/pointers/ephemeron.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,23 @@ impl<K: Trace, V: Trace + Clone> Ephemeron<K, V> {
unsafe { self.inner_ptr.as_ref().value().cloned() }
}

/// Gets the stored key of this `Ephemeron`, or `None` if the key was already garbage collected.
#[inline]
#[must_use]
pub fn key(&self) -> Option<Gc<K>> {
// SAFETY: this is safe because `Ephemeron` is tracked to always point to a valid pointer
// `inner_ptr`.
let key_ptr = unsafe { self.inner_ptr.as_ref().key_ptr() }?;

// SAFETY: Returned pointer is valid, so this is safe.
unsafe {
key_ptr.as_ref().inc_ref_count();
}

// SAFETY: The gc pointer's reference count has been incremented, so this is safe.
Some(unsafe { Gc::from_raw(key_ptr) })
}

/// Checks if the [`Ephemeron`] has a value.
#[must_use]
pub fn has_value(&self) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion boa_gc/src/pointers/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<T: Trace> Gc<T> {
// `unsafe` block.
// - `set_kv`: `weak` is a newly created `EphemeronBox`, meaning it isn't possible to
// collect it since `weak` is still live.
unsafe { weak.inner().inner_ptr().as_mut().set(&gc, gc.clone()) }
unsafe { weak.inner().inner_ptr().as_mut().set(&gc, ()) }

gc
}
Expand Down
15 changes: 9 additions & 6 deletions boa_gc/src/pointers/weak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,36 @@ use std::hash::{Hash, Hasher};
#[derive(Debug, Trace, Finalize)]
#[repr(transparent)]
pub struct WeakGc<T: Trace + 'static> {
inner: Ephemeron<T, Gc<T>>,
inner: Ephemeron<T, ()>,
}

impl<T: Trace> WeakGc<T> {
/// Creates a new weak pointer for a garbage collected value.
#[inline]
#[must_use]
pub fn new(value: &Gc<T>) -> Self {
Self {
inner: Ephemeron::new(value, value.clone()),
inner: Ephemeron::new(value, ()),
}
}

/// Upgrade returns a `Gc` pointer for the internal value if the pointer is still live, or `None`
/// if the value was already garbage collected.
#[inline]
#[must_use]
pub fn upgrade(&self) -> Option<Gc<T>> {
self.inner.value()
self.inner.key()
}

/// Check if the [`WeakGc`] can be upgraded.
#[inline]
#[must_use]
pub fn is_upgradable(&self) -> bool {
self.inner.has_value()
}

#[must_use]
pub(crate) const fn inner(&self) -> &Ephemeron<T, Gc<T>> {
pub(crate) const fn inner(&self) -> &Ephemeron<T, ()> {
&self.inner
}
}
Expand All @@ -47,8 +50,8 @@ impl<T: Trace> Clone for WeakGc<T> {
}
}

impl<T: Trace> From<Ephemeron<T, Gc<T>>> for WeakGc<T> {
fn from(inner: Ephemeron<T, Gc<T>>) -> Self {
impl<T: Trace> From<Ephemeron<T, ()>> for WeakGc<T> {
fn from(inner: Ephemeron<T, ()>) -> Self {
Self { inner }
}
}
Expand Down
Loading