From f26d3b76ac2904503b0971c44e956d06b0ae2e20 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Tue, 19 Sep 2023 01:58:16 +0000 Subject: [PATCH 1/2] Extract set_collection_kind and set_gc_status --- docs/userguide/src/tutorial/code/mygc_semispace/global.rs | 3 --- src/plan/generational/copying/global.rs | 3 --- src/plan/generational/immix/global.rs | 5 ----- src/plan/global.rs | 2 +- src/plan/immix/global.rs | 3 --- src/plan/markcompact/global.rs | 4 ---- src/plan/marksweep/global.rs | 3 --- src/plan/pageprotect/global.rs | 3 --- src/plan/semispace/global.rs | 3 --- src/plan/sticky/immix/global.rs | 4 ---- src/scheduler/gc_work.rs | 6 +++++- 11 files changed, 6 insertions(+), 33 deletions(-) diff --git a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs index 27d272f918..a7e6fe7485 100644 --- a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs +++ b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs @@ -1,7 +1,6 @@ // ANCHOR: imports_no_gc_work use crate::plan::global::BasePlan; //Modify use crate::plan::global::CommonPlan; // Add -use crate::plan::global::GcStatus; // Add use crate::plan::global::{CreateGeneralPlanArgs, CreateSpecificPlanArgs}; use crate::plan::mygc::mutator::ALLOCATOR_MAPPING; use crate::plan::mygc::gc_work::MyGCWorkContext; @@ -77,8 +76,6 @@ impl Plan for MyGC { // Modify // ANCHOR: schedule_collection fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); scheduler.schedule_common_work::>(self); } // ANCHOR_END: schedule_collection diff --git a/src/plan/generational/copying/global.rs b/src/plan/generational/copying/global.rs index 7f5d2f4c97..c0cb53bfde 100644 --- a/src/plan/generational/copying/global.rs +++ b/src/plan/generational/copying/global.rs @@ -8,7 +8,6 @@ use crate::plan::global::BasePlan; use crate::plan::global::CommonPlan; use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; -use crate::plan::global::GcStatus; use crate::plan::AllocationSemantics; use crate::plan::Plan; use crate::plan::PlanConstraints; @@ -73,8 +72,6 @@ impl Plan for GenCopy { fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { let is_full_heap = self.requires_full_heap_collection(); - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); if is_full_heap { scheduler.schedule_common_work::>(self); } else { diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs index 3373688157..e9e96b08e2 100644 --- a/src/plan/generational/immix/global.rs +++ b/src/plan/generational/immix/global.rs @@ -6,7 +6,6 @@ use crate::plan::global::BasePlan; use crate::plan::global::CommonPlan; use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; -use crate::plan::global::GcStatus; use crate::plan::AllocationSemantics; use crate::plan::Plan; use crate::plan::PlanConstraints; @@ -104,10 +103,6 @@ impl Plan for GenImmix { #[allow(clippy::branches_sharing_code)] fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { let is_full_heap = self.requires_full_heap_collection(); - - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); - if !is_full_heap { debug!("Nursery GC"); scheduler.schedule_common_work::>(self); diff --git a/src/plan/global.rs b/src/plan/global.rs index df887488dd..48bec7320b 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -691,7 +691,7 @@ impl BasePlan { self.vm_space.release(); } - pub fn set_collection_kind(&self, plan: &P) { + pub fn set_collection_kind(&self, plan: &dyn Plan) { self.cur_collection_attempts.store( if self.is_user_triggered_collection() { 1 diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs index 23e2b0402e..d1e4e6f8f3 100644 --- a/src/plan/immix/global.rs +++ b/src/plan/immix/global.rs @@ -4,7 +4,6 @@ use crate::plan::global::BasePlan; use crate::plan::global::CommonPlan; use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; -use crate::plan::global::GcStatus; use crate::plan::AllocationSemantics; use crate::plan::Plan; use crate::plan::PlanConstraints; @@ -72,8 +71,6 @@ impl Plan for Immix { } fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); Self::schedule_immix_full_heap_collection::< Immix, ImmixGCWorkContext, diff --git a/src/plan/markcompact/global.rs b/src/plan/markcompact/global.rs index 32da425ea3..3eefae3d23 100644 --- a/src/plan/markcompact/global.rs +++ b/src/plan/markcompact/global.rs @@ -4,7 +4,6 @@ use super::gc_work::{ UpdateReferences, }; use crate::plan::global::CommonPlan; -use crate::plan::global::GcStatus; use crate::plan::global::{BasePlan, CreateGeneralPlanArgs, CreateSpecificPlanArgs}; use crate::plan::markcompact::mutator::ALLOCATOR_MAPPING; use crate::plan::AllocationSemantics; @@ -79,9 +78,6 @@ impl Plan for MarkCompact { } fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); - // TODO use schedule_common once it can work with markcompact // self.common() // .schedule_common::, NoCopy>( diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index 34a59f5a10..8d1d4d0137 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -2,7 +2,6 @@ use crate::plan::global::BasePlan; use crate::plan::global::CommonPlan; use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; -use crate::plan::global::GcStatus; use crate::plan::marksweep::gc_work::MSGCWorkContext; use crate::plan::marksweep::mutator::ALLOCATOR_MAPPING; use crate::plan::AllocationSemantics; @@ -48,8 +47,6 @@ pub const MS_CONSTRAINTS: PlanConstraints = PlanConstraints { impl Plan for MarkSweep { fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); scheduler.schedule_common_work::>(self); } diff --git a/src/plan/pageprotect/global.rs b/src/plan/pageprotect/global.rs index 1e8021fae8..999121d126 100644 --- a/src/plan/pageprotect/global.rs +++ b/src/plan/pageprotect/global.rs @@ -2,7 +2,6 @@ use super::gc_work::PPGCWorkContext; use super::mutator::ALLOCATOR_MAPPING; use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; -use crate::plan::global::GcStatus; use crate::plan::AllocationSemantics; use crate::plan::Plan; use crate::plan::PlanConstraints; @@ -39,8 +38,6 @@ impl Plan for PageProtect { } fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); scheduler.schedule_common_work::>(self); } diff --git a/src/plan/semispace/global.rs b/src/plan/semispace/global.rs index ec74ec6c4c..e275e3aaa8 100644 --- a/src/plan/semispace/global.rs +++ b/src/plan/semispace/global.rs @@ -2,7 +2,6 @@ use super::gc_work::SSGCWorkContext; use crate::plan::global::CommonPlan; use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; -use crate::plan::global::GcStatus; use crate::plan::semispace::mutator::ALLOCATOR_MAPPING; use crate::plan::AllocationSemantics; use crate::plan::Plan; @@ -66,8 +65,6 @@ impl Plan for SemiSpace { } fn schedule_collection(&'static self, scheduler: &GCWorkScheduler) { - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); scheduler.schedule_common_work::>(self); } diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index c38b3d590f..c088884aec 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -3,7 +3,6 @@ use crate::plan::global::CommonPlan; use crate::plan::global::CreateGeneralPlanArgs; use crate::plan::global::CreateSpecificPlanArgs; use crate::plan::immix; -use crate::plan::GcStatus; use crate::plan::PlanConstraints; use crate::policy::immix::ImmixSpace; use crate::policy::sft::SFT; @@ -80,9 +79,6 @@ impl Plan for StickyImmix { } fn schedule_collection(&'static self, scheduler: &crate::scheduler::GCWorkScheduler) { - self.base().set_collection_kind::(self); - self.base().set_gc_status(GcStatus::GcPrepare); - let is_full_heap = self.requires_full_heap_collection(); self.gc_full_heap.store(is_full_heap, Ordering::SeqCst); diff --git a/src/scheduler/gc_work.rs b/src/scheduler/gc_work.rs index ebf45d5c77..ec9c0208ae 100644 --- a/src/scheduler/gc_work.rs +++ b/src/scheduler/gc_work.rs @@ -14,7 +14,11 @@ pub struct ScheduleCollection; impl GCWork for ScheduleCollection { fn do_work(&mut self, worker: &mut GCWorker, mmtk: &'static MMTK) { - mmtk.get_plan().schedule_collection(worker.scheduler()); + let plan = mmtk.get_plan(); + plan.base().set_collection_kind(plan); + plan.base().set_gc_status(GcStatus::GcPrepare); + + plan.schedule_collection(worker.scheduler()); // Tell GC trigger that GC started. // We now know what kind of GC this is (e.g. nursery vs mature in gen copy, defrag vs fast in Immix) From eed0be6fc689dd02d6e9db2ace8cb8fbeea8cae8 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Mon, 16 Oct 2023 05:29:01 +0000 Subject: [PATCH 2/2] Move gc_trigger.policy.on_gc_start --- src/scheduler/gc_work.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/scheduler/gc_work.rs b/src/scheduler/gc_work.rs index 827e590447..a957d13a3f 100644 --- a/src/scheduler/gc_work.rs +++ b/src/scheduler/gc_work.rs @@ -15,17 +15,13 @@ pub struct ScheduleCollection; impl GCWork for ScheduleCollection { fn do_work(&mut self, worker: &mut GCWorker, mmtk: &'static MMTK) { let plan = mmtk.get_plan(); + // Tell GC trigger that GC started. + plan.base().gc_trigger.policy.on_gc_start(mmtk); + plan.base().set_collection_kind(plan); plan.base().set_gc_status(GcStatus::GcPrepare); plan.schedule_collection(worker.scheduler()); - - // Tell GC trigger that GC started. - // We now know what kind of GC this is (e.g. nursery vs mature in gen copy, defrag vs fast in Immix) - // TODO: Depending on the OS scheduling, other workers can run so fast that they can finish - // everything in the `Unconstrained` and the `Prepare` buckets before we execute the next - // statement. Consider if there is a better place to call `on_gc_start`. - mmtk.get_plan().base().gc_trigger.policy.on_gc_start(mmtk); } }