Skip to content

Remove Collect + Sized on Rootable::Root type. #102

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

Merged
merged 2 commits into from
Jun 30, 2024
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
102 changes: 66 additions & 36 deletions src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
/// `Rootable<'a>` for *any* possible `'a`. This is necessary so that the `Root` types can be
/// branded by the unique, invariant lifetimes that makes an `Arena` sound.
pub trait Rootable<'a>: 'static {
type Root: Collect + 'a;
type Root: ?Sized + 'a;
}

/// A marker type used by the `Rootable!` macro instead of a bare trait object.
Expand Down Expand Up @@ -115,12 +115,19 @@ pub enum CollectionPhase {
/// this way, incremental garbage collection can be achieved (assuming "sufficiently small" calls
/// to `mutate`) that is both extremely safe and zero overhead vs what you would write in C with raw
/// pointers and manually ensuring that invariants are held.
pub struct Arena<R: for<'a> Rootable<'a>> {
pub struct Arena<R>
where
R: for<'a> Rootable<'a>,
{
context: Box<Context>,
root: Root<'static, R>,
}

impl<R: for<'a> Rootable<'a>> Arena<R> {
impl<R> Arena<R>
where
R: for<'a> Rootable<'a>,
for<'a> Root<'a, R>: Sized,
{
/// Create a new arena with the given garbage collector tuning parameters. You must provide a
/// closure that accepts a `&Mutation<'gc>` and returns the appropriate root.
pub fn new<F>(f: F) -> Arena<R>
Expand Down Expand Up @@ -155,6 +162,51 @@ impl<R: for<'a> Rootable<'a>> Arena<R> {
}
}

#[inline]
pub fn map_root<R2>(
self,
f: impl for<'gc> FnOnce(&'gc Mutation<'gc>, Root<'gc, R>) -> Root<'gc, R2>,
) -> Arena<R2>
where
R2: for<'a> Rootable<'a>,
for<'a> Root<'a, R2>: Sized,
{
self.context.root_barrier();
let new_root: Root<'static, R2> = unsafe {
let mc: &'static Mutation<'_> = &*(self.context.mutation_context() as *const _);
f(mc, self.root)
};
Arena {
context: self.context,
root: new_root,
}
}

#[inline]
pub fn try_map_root<R2, E>(
self,
f: impl for<'gc> FnOnce(&'gc Mutation<'gc>, Root<'gc, R>) -> Result<Root<'gc, R2>, E>,
) -> Result<Arena<R2>, E>
where
R2: for<'a> Rootable<'a>,
for<'a> Root<'a, R2>: Sized,
{
self.context.root_barrier();
let new_root: Root<'static, R2> = unsafe {
let mc: &'static Mutation<'_> = &*(self.context.mutation_context() as *const _);
f(mc, self.root)?
};
Ok(Arena {
context: self.context,
root: new_root,
})
}
}

impl<R> Arena<R>
where
R: for<'a> Rootable<'a>,
{
/// The primary means of interacting with a garbage collected arena. Accepts a callback which
/// receives a `&Mutation<'gc>` and a reference to the root, and can return any non garbage
/// collected value. The callback may "mutate" any part of the object graph during this call,
Expand Down Expand Up @@ -186,38 +238,6 @@ impl<R: for<'a> Rootable<'a>> Arena<R> {
}
}

#[inline]
pub fn map_root<R2: for<'a> Rootable<'a>>(
self,
f: impl for<'gc> FnOnce(&'gc Mutation<'gc>, Root<'gc, R>) -> Root<'gc, R2>,
) -> Arena<R2> {
self.context.root_barrier();
let new_root: Root<'static, R2> = unsafe {
let mc: &'static Mutation<'_> = &*(self.context.mutation_context() as *const _);
f(mc, self.root)
};
Arena {
context: self.context,
root: new_root,
}
}

#[inline]
pub fn try_map_root<R2: for<'a> Rootable<'a>, E>(
self,
f: impl for<'gc> FnOnce(&'gc Mutation<'gc>, Root<'gc, R>) -> Result<Root<'gc, R2>, E>,
) -> Result<Arena<R2>, E> {
self.context.root_barrier();
let new_root: Root<'static, R2> = unsafe {
let mc: &'static Mutation<'_> = &*(self.context.mutation_context() as *const _);
f(mc, self.root)?
};
Ok(Arena {
context: self.context,
root: new_root,
})
}

#[inline]
pub fn metrics(&self) -> &Metrics {
self.context.metrics()
Expand All @@ -238,7 +258,13 @@ impl<R: for<'a> Rootable<'a>> Arena<R> {
Phase::Drop => unreachable!(),
}
}
}

impl<R> Arena<R>
where
R: for<'a> Rootable<'a>,
for<'a> Root<'a, R>: Collect,
{
/// Run incremental garbage collection until the allocation debt is <= 0.0.
///
/// There is no minimum unit of work enforced here, so it may be faster to only call this method
Expand Down Expand Up @@ -314,7 +340,11 @@ impl<R: for<'a> Rootable<'a>> Arena<R> {

pub struct MarkedArena<'a, R: for<'b> Rootable<'b>>(&'a mut Arena<R>);

impl<'a, R: for<'b> Rootable<'b>> MarkedArena<'a, R> {
impl<'a, R> MarkedArena<'a, R>
where
R: for<'b> Rootable<'b>,
for<'b> Root<'b, R>: Collect,
{
/// Examine the state of a fully marked arena.
///
/// Allows you to determine whether `GcWeak` pointers are "dead" (aka, soon-to-be-dropped) and
Expand Down
4 changes: 2 additions & 2 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl Context {
// reachable from the given root object.
//
// If we are currently in `Phase::Sleep`, this will transition the collector to `Phase::Mark`.
pub(crate) unsafe fn do_collection<R: Collect>(
pub(crate) unsafe fn do_collection<R: Collect + ?Sized>(
&self,
root: &R,
target_debt: f64,
Expand All @@ -238,7 +238,7 @@ impl Context {
self.do_collection_inner(root, target_debt, early_stop)
}

fn do_collection_inner<R: Collect>(
fn do_collection_inner<R: Collect + ?Sized>(
&self,
root: &R,
mut target_debt: f64,
Expand Down
7 changes: 3 additions & 4 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ fn test_dynamic_roots() {
let rc_a = Rc::new(12);
let rc_b = Rc::new("hello".to_owned());

let mut arena: Arena<Rootable![DynamicRootSet<'_>]> = Arena::new(|mc| DynamicRootSet::new(mc));
let mut arena = Arena::<Rootable![DynamicRootSet<'_>]>::new(|mc| DynamicRootSet::new(mc));

let root_a = arena
.mutate(|mc, root_set| root_set.stash::<Rootable![Rc<i32>]>(mc, Gc::new(mc, rc_a.clone())));
Expand Down Expand Up @@ -456,9 +456,8 @@ fn test_dynamic_roots() {
#[test]
#[should_panic]
fn test_dynamic_bad_set() {
let arena1: Arena<Rootable![DynamicRootSet<'_>]> = Arena::new(|mc| DynamicRootSet::new(mc));

let arena2: Arena<Rootable![DynamicRootSet<'_>]> = Arena::new(|mc| DynamicRootSet::new(mc));
Comment on lines -459 to -461
Copy link
Owner Author

Choose a reason for hiding this comment

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

One thing I noticed here is that let arena: Arena<Rootable![MyRoot<'_>]> = Arena::new(|mc| ...); no longer works, but let arena = Arena::<Rootable![MyRoot<'_>]>::new(|mc| ...); does, and I have no idea why.

It's a very minor paper cut, but it'd be nice not to break it.

let arena1 = Arena::<Rootable![DynamicRootSet<'_>]>::new(|mc| DynamicRootSet::new(mc));
let arena2 = Arena::<Rootable![DynamicRootSet<'_>]>::new(|mc| DynamicRootSet::new(mc));

let dyn_root = arena1.mutate(|mc, root| root.stash::<Rootable![i32]>(mc, Gc::new(mc, 44)));

Expand Down