From 5f5c1d06c47d35fa68a573723d3f7a77d3e5b36a Mon Sep 17 00:00:00 2001 From: Kunal Sareen Date: Wed, 17 Apr 2024 12:44:49 +1000 Subject: [PATCH] Allow roots to be pinned for StickyImmix nursery collections (#1108) This PR allows StickyImmix to properly deal with transitive pinning trace. Before this PR, StickyImmix may move objects in the transitive pinning trace, as it simply uses `GenNurseryProcessEdges` which allows moving. With this PR, for transitive pinning trace, StickyImmix uses `GenNurseryProcessEdges<..., TRACE_KIND_TRANSITIVE_PIN>`. This PR fixes https://github.com/mmtk/mmtk-core/issues/1097. --- src/plan/generational/barrier.rs | 5 ++-- src/plan/generational/copying/gc_work.rs | 2 +- src/plan/generational/copying/global.rs | 6 +++-- src/plan/generational/gc_work.rs | 29 ++++++++++++++++-------- src/plan/generational/global.rs | 10 ++++++-- src/plan/generational/immix/gc_work.rs | 3 ++- src/plan/generational/immix/global.rs | 6 +++-- src/plan/sticky/immix/gc_work.rs | 7 ++++-- src/plan/sticky/immix/global.rs | 19 ++++++++++++---- 9 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/plan/generational/barrier.rs b/src/plan/generational/barrier.rs index 1fe17c93b0..9402d2470d 100644 --- a/src/plan/generational/barrier.rs +++ b/src/plan/generational/barrier.rs @@ -3,6 +3,7 @@ use crate::plan::barriers::BarrierSemantics; use crate::plan::PlanTraceObject; use crate::plan::VectorQueue; +use crate::policy::gc_work::DEFAULT_TRACE; use crate::scheduler::WorkBucketStage; use crate::util::constants::BYTES_IN_INT; use crate::util::*; @@ -45,7 +46,7 @@ impl + PlanTraceObject> let buf = self.modbuf.take(); if !buf.is_empty() { self.mmtk.scheduler.work_buckets[WorkBucketStage::Closure] - .add(ProcessModBuf::>::new(buf)); + .add(ProcessModBuf::>::new(buf)); } } @@ -54,7 +55,7 @@ impl + PlanTraceObject> if !buf.is_empty() { debug_assert!(!buf.is_empty()); self.mmtk.scheduler.work_buckets[WorkBucketStage::Closure].add(ProcessRegionModBuf::< - GenNurseryProcessEdges, + GenNurseryProcessEdges, >::new(buf)); } } diff --git a/src/plan/generational/copying/gc_work.rs b/src/plan/generational/copying/gc_work.rs index d0a2fb7f4e..f9e8b2b8fb 100644 --- a/src/plan/generational/copying/gc_work.rs +++ b/src/plan/generational/copying/gc_work.rs @@ -9,7 +9,7 @@ pub struct GenCopyNurseryGCWorkContext(std::marker::PhantomData crate::scheduler::GCWorkContext for GenCopyNurseryGCWorkContext { type VM = VM; type PlanType = GenCopy; - type DefaultProcessEdges = GenNurseryProcessEdges; + type DefaultProcessEdges = GenNurseryProcessEdges; type PinningProcessEdges = UnsupportedProcessEdges; } diff --git a/src/plan/generational/copying/global.rs b/src/plan/generational/copying/global.rs index dd517fd607..faa4d73266 100644 --- a/src/plan/generational/copying/global.rs +++ b/src/plan/generational/copying/global.rs @@ -12,6 +12,7 @@ use crate::plan::AllocationSemantics; use crate::plan::Plan; use crate::plan::PlanConstraints; use crate::policy::copyspace::CopySpace; +use crate::policy::gc_work::TraceKind; use crate::policy::space::Space; use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; @@ -183,13 +184,14 @@ impl GenerationalPlan for GenCopy { } impl GenerationalPlanExt for GenCopy { - fn trace_object_nursery( + fn trace_object_nursery( &self, queue: &mut Q, object: ObjectReference, worker: &mut GCWorker, ) -> ObjectReference { - self.gen.trace_object_nursery(queue, object, worker) + self.gen + .trace_object_nursery::(queue, object, worker) } } diff --git a/src/plan/generational/gc_work.rs b/src/plan/generational/gc_work.rs index 5ebd7cf8f1..5862e4a00e 100644 --- a/src/plan/generational/gc_work.rs +++ b/src/plan/generational/gc_work.rs @@ -1,6 +1,8 @@ use atomic::Ordering; use crate::plan::PlanTraceObject; +use crate::plan::VectorObjectQueue; +use crate::policy::gc_work::TraceKind; use crate::scheduler::{gc_work::*, GCWork, GCWorker, WorkBucketStage}; use crate::util::ObjectReference; use crate::vm::edge_shape::{Edge, MemorySlice}; @@ -14,13 +16,17 @@ use super::global::GenerationalPlanExt; /// Process edges for a nursery GC. This type is provided if a generational plan does not use /// [`crate::scheduler::gc_work::SFTProcessEdges`]. If a plan uses `SFTProcessEdges`, /// it does not need to use this type. -pub struct GenNurseryProcessEdges + PlanTraceObject> { +pub struct GenNurseryProcessEdges< + VM: VMBinding, + P: GenerationalPlanExt + PlanTraceObject, + const KIND: TraceKind, +> { plan: &'static P, base: ProcessEdgesBase, } -impl + PlanTraceObject> ProcessEdgesWork - for GenNurseryProcessEdges +impl + PlanTraceObject, const KIND: TraceKind> + ProcessEdgesWork for GenNurseryProcessEdges { type VM = VM; type ScanObjectsWorkType = PlanScanObjects; @@ -35,14 +41,19 @@ impl + PlanTraceObject> ProcessEdg let plan = base.plan().downcast_ref().unwrap(); Self { plan, base } } + fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { debug_assert!(!object.is_null()); // We cannot borrow `self` twice in a call, so we extract `worker` as a local variable. let worker = self.worker(); - self.plan - .trace_object_nursery(&mut self.base.nodes, object, worker) + self.plan.trace_object_nursery::( + &mut self.base.nodes, + object, + worker, + ) } + fn process_edge(&mut self, slot: EdgeOf) { let object = slot.load(); if object.is_null() { @@ -62,8 +73,8 @@ impl + PlanTraceObject> ProcessEdg } } -impl + PlanTraceObject> Deref - for GenNurseryProcessEdges +impl + PlanTraceObject, const KIND: TraceKind> Deref + for GenNurseryProcessEdges { type Target = ProcessEdgesBase; fn deref(&self) -> &Self::Target { @@ -71,8 +82,8 @@ impl + PlanTraceObject> Deref } } -impl + PlanTraceObject> DerefMut - for GenNurseryProcessEdges +impl + PlanTraceObject, const KIND: TraceKind> + DerefMut for GenNurseryProcessEdges { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.base diff --git a/src/plan/generational/global.rs b/src/plan/generational/global.rs index 86c39a07e8..55054c7388 100644 --- a/src/plan/generational/global.rs +++ b/src/plan/generational/global.rs @@ -3,6 +3,7 @@ use crate::plan::global::CreateSpecificPlanArgs; use crate::plan::ObjectQueue; use crate::plan::Plan; use crate::policy::copyspace::CopySpace; +use crate::policy::gc_work::{TraceKind, TRACE_KIND_TRANSITIVE_PIN}; use crate::policy::space::Space; use crate::scheduler::*; use crate::util::copy::CopySemantics; @@ -222,12 +223,17 @@ impl CommonGenPlan { } /// Trace objects for spaces in generational and common plans for a nursery GC. - pub fn trace_object_nursery( + pub fn trace_object_nursery( &self, queue: &mut Q, object: ObjectReference, worker: &mut GCWorker, ) -> ObjectReference { + assert!( + KIND != TRACE_KIND_TRANSITIVE_PIN, + "A copying nursery cannot pin objects" + ); + // Evacuate nursery objects if self.nursery.in_space(object) { return self.nursery.trace_object::( @@ -323,7 +329,7 @@ pub trait GenerationalPlan: Plan { pub trait GenerationalPlanExt: GenerationalPlan { /// Trace an object in nursery collection. If the object is in nursery, we should call `trace_object` /// on the space. Otherwise, we can just return the object. - fn trace_object_nursery( + fn trace_object_nursery( &self, queue: &mut Q, object: ObjectReference, diff --git a/src/plan/generational/immix/gc_work.rs b/src/plan/generational/immix/gc_work.rs index dd9a38c0b3..36c6878192 100644 --- a/src/plan/generational/immix/gc_work.rs +++ b/src/plan/generational/immix/gc_work.rs @@ -1,6 +1,7 @@ use super::global::GenImmix; use crate::plan::generational::gc_work::GenNurseryProcessEdges; use crate::policy::gc_work::TraceKind; +use crate::policy::gc_work::DEFAULT_TRACE; use crate::scheduler::gc_work::PlanProcessEdges; use crate::scheduler::gc_work::UnsupportedProcessEdges; use crate::vm::VMBinding; @@ -9,7 +10,7 @@ pub struct GenImmixNurseryGCWorkContext(std::marker::PhantomData< impl crate::scheduler::GCWorkContext for GenImmixNurseryGCWorkContext { type VM = VM; type PlanType = GenImmix; - type DefaultProcessEdges = GenNurseryProcessEdges; + type DefaultProcessEdges = GenNurseryProcessEdges; type PinningProcessEdges = UnsupportedProcessEdges; } diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs index ae33c6d19a..97b09b15fd 100644 --- a/src/plan/generational/immix/global.rs +++ b/src/plan/generational/immix/global.rs @@ -9,6 +9,7 @@ use crate::plan::global::CreateSpecificPlanArgs; use crate::plan::AllocationSemantics; use crate::plan::Plan; use crate::plan::PlanConstraints; +use crate::policy::gc_work::TraceKind; use crate::policy::immix::ImmixSpace; use crate::policy::immix::ImmixSpaceArgs; use crate::policy::immix::{TRACE_KIND_DEFRAG, TRACE_KIND_FAST}; @@ -215,13 +216,14 @@ impl GenerationalPlan for GenImmix { } impl crate::plan::generational::global::GenerationalPlanExt for GenImmix { - fn trace_object_nursery( + fn trace_object_nursery( &self, queue: &mut Q, object: ObjectReference, worker: &mut GCWorker, ) -> ObjectReference { - self.gen.trace_object_nursery(queue, object, worker) + self.gen + .trace_object_nursery::(queue, object, worker) } } diff --git a/src/plan/sticky/immix/gc_work.rs b/src/plan/sticky/immix/gc_work.rs index 2964725297..bd3d0817f9 100644 --- a/src/plan/sticky/immix/gc_work.rs +++ b/src/plan/sticky/immix/gc_work.rs @@ -1,4 +1,5 @@ use crate::policy::gc_work::TraceKind; +use crate::policy::gc_work::DEFAULT_TRACE; use crate::policy::gc_work::TRACE_KIND_TRANSITIVE_PIN; use crate::scheduler::gc_work::PlanProcessEdges; use crate::{plan::generational::gc_work::GenNurseryProcessEdges, vm::VMBinding}; @@ -6,11 +7,13 @@ use crate::{plan::generational::gc_work::GenNurseryProcessEdges, vm::VMBinding}; use super::global::StickyImmix; pub struct StickyImmixNurseryGCWorkContext(std::marker::PhantomData); + impl crate::scheduler::GCWorkContext for StickyImmixNurseryGCWorkContext { type VM = VM; type PlanType = StickyImmix; - type DefaultProcessEdges = GenNurseryProcessEdges; - type PinningProcessEdges = GenNurseryProcessEdges; + type DefaultProcessEdges = GenNurseryProcessEdges; + type PinningProcessEdges = + GenNurseryProcessEdges; } pub struct StickyImmixMatureGCWorkContext( diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index f2d2077502..0d152eb8c0 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -4,7 +4,10 @@ use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; use crate::plan::immix; use crate::plan::PlanConstraints; +use crate::policy::gc_work::TraceKind; +use crate::policy::gc_work::TRACE_KIND_TRANSITIVE_PIN; use crate::policy::immix::ImmixSpace; +use crate::policy::immix::TRACE_KIND_FAST; use crate::policy::sft::SFT; use crate::policy::space::Space; use crate::util::copy::CopyConfig; @@ -91,7 +94,7 @@ impl Plan for StickyImmix { } else { info!("Full heap GC"); use crate::plan::immix::Immix; - use crate::policy::immix::{TRACE_KIND_DEFRAG, TRACE_KIND_FAST}; + use crate::policy::immix::TRACE_KIND_DEFRAG; Immix::schedule_immix_full_heap_collection::< StickyImmix, StickyImmixMatureGCWorkContext, @@ -227,7 +230,7 @@ impl GenerationalPlan for StickyImmix { } impl crate::plan::generational::global::GenerationalPlanExt for StickyImmix { - fn trace_object_nursery( + fn trace_object_nursery( &self, queue: &mut Q, object: crate::util::ObjectReference, @@ -239,7 +242,15 @@ impl crate::plan::generational::global::GenerationalPlanExt f trace!("Immix mature object {}, skip", object); return object; } else { - let object = if crate::policy::immix::PREFER_COPY_ON_NURSERY_GC { + let object = if KIND == TRACE_KIND_TRANSITIVE_PIN || KIND == TRACE_KIND_FAST { + trace!( + "Immix nursery object {} is being traced without moving", + object + ); + self.immix + .immix_space + .trace_object_without_moving(queue, object) + } else if crate::policy::immix::PREFER_COPY_ON_NURSERY_GC { let ret = self.immix.immix_space.trace_object_with_opportunistic_copy( queue, object, @@ -255,7 +266,7 @@ impl crate::plan::generational::global::GenerationalPlanExt f if ret == object { "".to_string() } else { - format!(" -> new object {}", ret) + format!("-> new object {}", ret) } ); ret