StickyImmix nursery GC moves objects pointed by pinning roots

[wks]

In default setting, if a nursery GC is triggered in StickyImmix, and an object is pointed by a pinning root (transitive or not), it will still attempt to copy the object. This contradicts with the contract of "pinning roots".

Details:

The VM binding can deliver lists of pinning roots via RootsWorkFactory::create_process_pinning_roots_work(nodes: Vec<ObjectReference>). The object graph edges pointing from roots (stack slots or global variables) to heap objects are represented as ObjectReference of the heap objects. That's sufficient because the objects won't be moved, therefore there is no need to update the stack slots or global variables.

impl<VM: VMBinding, E: ProcessEdgesWork<VM = VM>, I: ProcessEdgesWork<VM = VM>>
    RootsWorkFactory<EdgeOf<E>> for ProcessEdgesWorkRootsWorkFactory<VM, E, I>
{
    // ...
    fn create_process_pinning_roots_work(&mut self, nodes: Vec<ObjectReference>) {
        crate::memory_manager::add_work_packet(
            self.mmtk,
            WorkBucketStage::PinningRootsTrace,
            ProcessRootNode::<VM, I, E>::new(nodes, WorkBucketStage::Closure),
        );
    }
    // ...

Here, E and I are type parameters. E is used for non-pinning edges, while I is used for pinning edges. In this case, ProcessRootNode::<VM, I, E>::new(nodes, ... tells the newly created ProcessRootNode work packet that the edges represented by the nodes vector shall be traced by I, and their transitively reached edges shall be traced by E.

For StickyImmix, the I and E come from the TPProcessEdges and ProcessEdgesWorkType members of StickyImmixNurseryGCWorkContext:

pub struct StickyImmixNurseryGCWorkContext<VM: VMBinding>(std::marker::PhantomData<VM>);
impl<VM: VMBinding> crate::scheduler::GCWorkContext for StickyImmixNurseryGCWorkContext<VM> {
    type VM = VM;
    type PlanType = StickyImmix<VM>;
    type ProcessEdgesWorkType = GenNurseryProcessEdges<VM, Self::PlanType>;
    type TPProcessEdges = GenNurseryProcessEdges<VM, Self::PlanType>;
}

Here ProcessEdgesWorkType and TPProcessEdges are identical. The same work packet will be used for tracing both pinning and movable roots.

GenNurseryProcessEdges::trace_object will eventually call StickyImmix::trace_object_nursery.

impl<VM: VMBinding> crate::plan::generational::global::GenerationalPlanExt<VM> for StickyImmix<VM> {
    fn trace_object_nursery<Q: crate::ObjectQueue>(
        &self,
        queue: &mut Q,
        object: crate::util::ObjectReference,
        worker: &mut crate::scheduler::GCWorker<VM>,
    ) -> crate::util::ObjectReference {
        if self.immix.immix_space.in_space(object) {
            if !self.is_object_in_nursery(object) {
                // Mature object
                trace!("Immix mature object {}, skip", object);
                return object;
            } else {
                let object = if crate::policy::immix::PREFER_COPY_ON_NURSERY_GC {
                    let ret = self.immix.immix_space.trace_object_with_opportunistic_copy(..., object, ...);
                    // ...
                    ret
                } else {

By default, the constant PREFER_COPY_ON_NURSERY_GC is true, unless disabled by the "immix_non_moving" or "sticky_immix_non_moving_nursery" Cargo features. This means even if object is from the ProcessRootNode work packet, it will still call immix_space.trace_object_with_opportunistic_copy, and that'll copy the object.

Desired behavior

Anyway, objects pointed by pinning roots should never be moved. There are two ways to realize that in StickyImmix

1. Never move objects in nursery GC.
2. May move ordinary objects in nursery GC, but must not move objects pointed by pinning roots.

The former can be achieved by setting the cargo feature "sticky_immix_non_moving_nursery". It is an easy workaround.

The latter may need some effort to implement.

StickyImmixNurseryGCWorkContext needs to have two different type members for ProcessEdgesWorkType and TPProcessEdges. Currently they are both GenNurseryProcessEdges.

We may need to add a KIND: TraceKind type parameter to GenNurseryProcessEdges, just like PlanProcessEdges<VM, Self::PlanType, KIND>. The KIND argument can be used to distinguish between non-moving and moving work packets.

Then GenerationalPlanExt::trace_object_nursery will also need a KIND: TraceKind type parameter, or simply a pinning: bool value parameter. This will either (1) override PREFER_COPY_ON_NURSERY_GC so that it will call trace_object_without_moving instead, or (2) raise an assertion error, showing that there cannot be pinning roots when PREFER_COPY_ON_NURSERY_GC is true.

[k-sareen]

Oh did I not upstream my fix for this? I encountered this in my ART port and fixed it. Unfortunately, it was not a complete fix as I didn't properly implement transitively pinning roots.

EDIT: Discussion here

[k-sareen]

Closed with #1108