Skip to content

Commit

Permalink
Reduce WeakGc<T> memory usage (#3492)
Browse files Browse the repository at this point in the history
* Reduce `WeakGc<T>` memory usage

Makes inner storage of `WeakGc<T>` store `()` in place of `Ephemeron` value,
insteadthe of a clone of the `Gc<T>` which is already stored in the key.

Reducing the size by the width of one pointer.

* Apply review
  • Loading branch information
HalidOdat authored Dec 4, 2023
1 parent d8e8c7a commit 13ba869
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 10 deletions.
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

0 comments on commit 13ba869

Please sign in to comment.