kyren / gc-arena

Incremental garbage collection from safe Rust
Creative Commons Zero v1.0 Universal
438 stars 36 forks source link

Fine-grained interior mutability #33

Closed moulins closed 1 year ago

moulins commented 1 year ago

The current GcCell API for interior mutability is very coarse-grained: each GC'd object is either mutable or immutable (ignoring 'static fields that don't participate in the object graph), and the only safe way to mutate the object graph is to use the provided RefCell-like API, which incurs runtime checks. Anything else requires somewhat tricky unsafe code and a bespoke Collect implementation, which is suboptimal.

This PR reworks the mutation API by separating the type providing the unit of allocation (Gc) and the cell types providing interior mutability (cell::{Cell, RefCell}). The special cell::Mutable wrapper acts as a bridge between the two by enforcing the rules around write barriers.

Here's some example code I wrote to give a feel on how everything fits together:

use gc_arena::{Collect, Gc, MutationContext};
use gc_arena::cell::{Cell, Mutable};

// The infamous doubly-linked list!
#[derive(Collect)]
#[collect(no_drop)]
struct Node<'gc> {
    // The mutable parts are stored inline...
    prev: NodeLink<'gc>,
    next: NodeLink<'gc>,
    // ... and the rest stays immutable.
    value: Gc<'gc, /* whatever */>,
}

type NodeLink<'gc> = Cell<Option<Gc<'gc, Node<'gc>>>>;

// Ideally, this would be generated by some kind of proc-macro.
fn project_links<'a, 'gc>(
    node: &'a Mutable<Node<'gc>>,
) -> (&'a Mutable<NodeLink<'gc>>, &'a Mutable<NodeLink<'gc>>) {
    // Safe, this is just projecting fields.
    unsafe { (Mutable::assume(&node.prev), Mutable::assume(&node.next)) }
}

// Example usage. 100% safe code and no extra runtime checks!
fn detach_node<'gc>(mc: MutationContext<'gc, '_>, node: Gc<'gc, Node<'gc>>) {
    let (prev, next) = project_links(Gc::mutate(mc, &node));
    let prev = prev.replace(None);
    let next = next.replace(None);

    if let Some(p) = prev {
        let (_, prev_next) = project_links(Gc::mutate(mc, &p));
        prev_next.set(next);
    }

    if let Some(n) = next {
        let (next_prev, _) = project_links(Gc::mutate(mc, &n));
        next_prev.set(prev);
    }
}

This is still a work-in-progress (e.g. I'd like to implement the macro stuff), but now is the good time to get feedback on the core API :)

moulins commented 1 year ago

This took much more time than what I'd have liked, but I've finished implementing the #[derive(Mutable)] macro for safe projections! (see src/gc_arena/tests/ui-pass/mutable_linked_list.rs for an updated version of the linked-list example)

I have still some more ideas and/or unresolved questions (see the following), but I believe this is enough for an initial MVP.


This PR is mostly backwards-compatible; the only change is that GcCell::allocate is now GcCell::allocate_cell.

Here are some further API changes I would propose if backward-compatibility isn't an issue:


Finally, a list of ideas for future improvements:

Bale001 commented 1 year ago

Is there anything more you wanted to do here? This feature would be really helpful

moulins commented 1 year ago

Is there anything more you wanted to do here? This feature would be really helpful

I'm really not happy with the Mutable projection stuff, the #[derive(Mutable)] macro is quite complex and doesn't work at all for third-party types. I'd like an alternative version where you don't need to annotate types at all, by providing a macro to type "mutable destructuring", e.g.:

mut_patterns! {
    let Node { prev, .. } = Gc::mutate(mc, &node);
}

// `prev`, `next`, `value` have type `&Mut<_>` here.
prev.replace(None);

This should be doable soundly, with a desugaring somewhat like this:

let _tmp: &Mutable<_> = Gc::mutate(mc, &node);
let &Mutable {
    __inner: Node { ref prev, .. },
    ..
} = _tmp;
// Sound because ref patterns never invoke Deref impls
let prev = unsafe { Mutable::assume(prev) };
moulins commented 1 year ago

Closing for now, in favor of the in-tree PR in Ruffle. I intend on opening a new PR once the proposed API has proven its worth in "real code", so to speak.