From 86a94ca502594d078b35c29ea3d1d68b34f16a28 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Wed, 21 Feb 2024 18:26:29 +1300 Subject: [PATCH] Allow binding to implement GC trigger (#1083) This PR allows bindings to create a delegated GC trigger. * Make `GCTriggerPolicy` public. Add a new type `SpaceStats` which simply wraps `Space`. * Add `Collection::create_gc_trigger`. This will be called when the `gc_trigger` option is set to `Delegated`. --- .../tutorial/code/mygc_semispace/global.rs | 3 +- src/mmtk.rs | 5 ++ src/plan/generational/copying/global.rs | 3 +- src/plan/generational/global.rs | 5 +- src/plan/generational/immix/global.rs | 3 +- src/plan/global.rs | 3 +- src/plan/immix/global.rs | 3 +- src/plan/markcompact/global.rs | 3 +- src/plan/marksweep/global.rs | 3 +- src/plan/nogc/global.rs | 3 +- src/plan/pageprotect/global.rs | 3 +- src/plan/semispace/global.rs | 3 +- src/plan/sticky/immix/global.rs | 12 ++--- src/util/heap/gc_trigger.rs | 47 ++++++++++++++++--- src/util/heap/mod.rs | 2 + src/util/test_util/mock_vm.rs | 7 +++ src/vm/collection.rs | 7 +++ 17 files changed, 90 insertions(+), 25 deletions(-) diff --git a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs index b4f2dcbab5..bbbf6fe6f5 100644 --- a/docs/userguide/src/tutorial/code/mygc_semispace/global.rs +++ b/docs/userguide/src/tutorial/code/mygc_semispace/global.rs @@ -13,6 +13,7 @@ use crate::scheduler::*; // Modify use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; use crate::util::heap::VMRequest; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::opaque_pointer::*; use crate::vm::VMBinding; @@ -78,7 +79,7 @@ impl Plan for MyGC { // ANCHOR_END: schedule_collection // ANCHOR: collection_required() - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } // ANCHOR_END: collection_required() diff --git a/src/mmtk.rs b/src/mmtk.rs index 067121258c..3ec1855ec0 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -299,6 +299,11 @@ impl MMTK { self.state.is_emergency_collection() } + /// Return true if the current GC is trigger manually by the user/binding. + pub fn is_user_triggered_collection(&self) -> bool { + self.state.is_user_triggered_collection() + } + /// The application code has requested a collection. This is just a GC hint, and /// we may ignore it. /// diff --git a/src/plan/generational/copying/global.rs b/src/plan/generational/copying/global.rs index f69a2e34f2..dd517fd607 100644 --- a/src/plan/generational/copying/global.rs +++ b/src/plan/generational/copying/global.rs @@ -16,6 +16,7 @@ use crate::policy::space::Space; use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::Address; use crate::util::ObjectReference; @@ -64,7 +65,7 @@ impl Plan for GenCopy { } } - fn collection_required(&self, space_full: bool, space: Option<&dyn Space>) -> bool + fn collection_required(&self, space_full: bool, space: Option>) -> bool where Self: Sized, { diff --git a/src/plan/generational/global.rs b/src/plan/generational/global.rs index e12430f576..6f92831dab 100644 --- a/src/plan/generational/global.rs +++ b/src/plan/generational/global.rs @@ -6,6 +6,7 @@ use crate::policy::copyspace::CopySpace; use crate::policy::space::Space; use crate::scheduler::*; use crate::util::copy::CopySemantics; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::statistics::counter::EventCounter; use crate::util::Address; @@ -98,7 +99,7 @@ impl CommonGenPlan { &self, plan: &P, space_full: bool, - space: Option<&dyn Space>, + space: Option>, ) -> bool { let cur_nursery = self.nursery.reserved_pages(); let max_nursery = self.common.base.options.get_max_nursery_pages(); @@ -120,7 +121,7 @@ impl CommonGenPlan { // - if space is none, it is not. Return false immediately. // - if space is some, we further check its descriptor. let is_triggered_by_nursery = space.map_or(false, |s| { - s.common().descriptor == self.nursery.common().descriptor + s.0.common().descriptor == self.nursery.common().descriptor }); // If space is full and the GC is not triggered by nursery, next GC will be full heap GC. if space_full && !is_triggered_by_nursery { diff --git a/src/plan/generational/immix/global.rs b/src/plan/generational/immix/global.rs index ebc0b2d12f..ae33c6d19a 100644 --- a/src/plan/generational/immix/global.rs +++ b/src/plan/generational/immix/global.rs @@ -17,6 +17,7 @@ use crate::scheduler::GCWorkScheduler; use crate::scheduler::GCWorker; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::Address; use crate::util::ObjectReference; @@ -90,7 +91,7 @@ impl Plan for GenImmix { ) } - fn collection_required(&self, space_full: bool, space: Option<&dyn Space>) -> bool + fn collection_required(&self, space_full: bool, space: Option>) -> bool where Self: Sized, { diff --git a/src/plan/global.rs b/src/plan/global.rs index 66560fe0c9..5b28c4b720 100644 --- a/src/plan/global.rs +++ b/src/plan/global.rs @@ -14,6 +14,7 @@ use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::{CopyConfig, GCWorkerCopyContext}; use crate::util::heap::gc_trigger::GCTrigger; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::layout::Mmapper; use crate::util::heap::layout::VMMap; use crate::util::heap::HeapMeta; @@ -213,7 +214,7 @@ pub trait Plan: 'static + HasSpaces + Sync + Downcast { /// # Arguments /// * `space_full`: the allocation to a specific space failed, must recover pages within 'space'. /// * `space`: an option to indicate if there is a space that has failed in an allocation. - fn collection_required(&self, space_full: bool, space: Option<&dyn Space>) -> bool; + fn collection_required(&self, space_full: bool, space: Option>) -> bool; // Note: The following methods are about page accounting. The default implementation should // work fine for non-copying plans. For copying plans, the plan should override any of these methods diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs index 9b2e96bf2d..0741ba3a99 100644 --- a/src/plan/immix/global.rs +++ b/src/plan/immix/global.rs @@ -13,6 +13,7 @@ use crate::policy::space::Space; use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::vm::VMBinding; @@ -45,7 +46,7 @@ pub const IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints { }; impl Plan for Immix { - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } diff --git a/src/plan/markcompact/global.rs b/src/plan/markcompact/global.rs index 814c26c23b..e9a2ec037d 100644 --- a/src/plan/markcompact/global.rs +++ b/src/plan/markcompact/global.rs @@ -15,6 +15,7 @@ use crate::scheduler::gc_work::*; use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::CopySemantics; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::metadata::side_metadata::SideMetadataContext; #[cfg(not(feature = "vo_bit"))] @@ -159,7 +160,7 @@ impl Plan for MarkCompact { .add(crate::util::sanity::sanity_checker::ScheduleSanityGC::::new(self)); } - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index c1356d2677..41dde00dab 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -10,6 +10,7 @@ use crate::plan::PlanConstraints; use crate::policy::space::Space; use crate::scheduler::GCWorkScheduler; use crate::util::alloc::allocators::AllocatorSelector; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::VMWorkerThread; @@ -64,7 +65,7 @@ impl Plan for MarkSweep { self.common.release(tls, true); } - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } diff --git a/src/plan/nogc/global.rs b/src/plan/nogc/global.rs index df6d759636..b2d02f58d3 100644 --- a/src/plan/nogc/global.rs +++ b/src/plan/nogc/global.rs @@ -9,6 +9,7 @@ use crate::policy::immortalspace::ImmortalSpace; use crate::policy::space::Space; use crate::scheduler::GCWorkScheduler; use crate::util::alloc::allocators::AllocatorSelector; +use crate::util::heap::gc_trigger::SpaceStats; #[allow(unused_imports)] use crate::util::heap::VMRequest; use crate::util::metadata::side_metadata::SideMetadataContext; @@ -45,7 +46,7 @@ impl Plan for NoGC { &NOGC_CONSTRAINTS } - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } diff --git a/src/plan/pageprotect/global.rs b/src/plan/pageprotect/global.rs index f718a9a14a..9e9bcb1fdf 100644 --- a/src/plan/pageprotect/global.rs +++ b/src/plan/pageprotect/global.rs @@ -8,6 +8,7 @@ use crate::plan::PlanConstraints; use crate::policy::space::Space; use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::{plan::global::BasePlan, vm::VMBinding}; @@ -57,7 +58,7 @@ impl Plan for PageProtect { self.space.release(true); } - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } diff --git a/src/plan/semispace/global.rs b/src/plan/semispace/global.rs index b4a44000ce..a55cbc0976 100644 --- a/src/plan/semispace/global.rs +++ b/src/plan/semispace/global.rs @@ -11,6 +11,7 @@ use crate::policy::space::Space; use crate::scheduler::*; use crate::util::alloc::allocators::AllocatorSelector; use crate::util::copy::*; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::heap::VMRequest; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::opaque_pointer::VMWorkerThread; @@ -95,7 +96,7 @@ impl Plan for SemiSpace { self.fromspace().release(); } - fn collection_required(&self, space_full: bool, _space: Option<&dyn Space>) -> bool { + fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index 44e6ad6153..f913e2e4b8 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -10,6 +10,7 @@ use crate::policy::space::Space; use crate::util::copy::CopyConfig; use crate::util::copy::CopySelector; use crate::util::copy::CopySemantics; +use crate::util::heap::gc_trigger::SpaceStats; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::statistics::counter::EventCounter; use crate::vm::ObjectModel; @@ -138,14 +139,13 @@ impl Plan for StickyImmix { .store(next_gc_full_heap, Ordering::Relaxed); } - fn collection_required( - &self, - space_full: bool, - space: Option<&dyn crate::policy::space::Space>, - ) -> bool { + fn collection_required(&self, space_full: bool, space: Option>) -> bool { let nursery_full = self.immix.immix_space.get_pages_allocated() > self.options().get_max_nursery_pages(); - if space_full && space.is_some() && space.unwrap().name() != self.immix.immix_space.name() { + if space_full + && space.is_some() + && space.as_ref().unwrap().0.name() != self.immix.immix_space.name() + { self.next_gc_full_heap.store(true, Ordering::SeqCst); } self.immix.collection_required(space_full, space) || nursery_full diff --git a/src/util/heap/gc_trigger.rs b/src/util/heap/gc_trigger.rs index b4c1f2f70c..e10a7a2db6 100644 --- a/src/util/heap/gc_trigger.rs +++ b/src/util/heap/gc_trigger.rs @@ -43,7 +43,9 @@ impl GCTrigger { conversions::bytes_to_pages_up(min), conversions::bytes_to_pages_up(max), )), - GCTriggerSelector::Delegated => unimplemented!(), + GCTriggerSelector::Delegated => { + >::create_gc_trigger() + } }, options, gc_requester, @@ -65,7 +67,10 @@ impl GCTrigger { /// * `space`: The space that triggered the poll. This could `None` if the poll is not triggered by a space. pub fn poll(&self, space_full: bool, space: Option<&dyn Space>) -> bool { let plan = unsafe { self.plan.assume_init() }; - if self.policy.is_gc_required(space_full, space, plan) { + if self + .policy + .is_gc_required(space_full, space.map(|s| SpaceStats::new(s)), plan) + { info!( "[POLL] {}{} ({}/{} pages)", if let Some(space) = space { @@ -101,6 +106,26 @@ impl GCTrigger { } } +/// Provides statistics about the space. This is exposed to bindings, as it is used +/// in both [`crate::plan::Plan`] and [`GCTriggerPolicy`]. +// This type exists so we do not need to expose the `Space` trait to the bindings. +pub struct SpaceStats<'a, VM: VMBinding>(pub(crate) &'a dyn Space); + +impl<'a, VM: VMBinding> SpaceStats<'a, VM> { + /// Create new SpaceStats. + fn new(space: &'a dyn Space) -> Self { + Self(space) + } + + /// Get the number of reserved pages for the space. + pub fn reserved_pages(&self) -> usize { + self.0.reserved_pages() + } + + // We may expose more methods to bindings if they need more information for implementing GC triggers. + // But we should never expose `Space` itself. +} + /// This trait describes a GC trigger policy. A triggering policy have hooks to be informed about /// GC start/end so they can collect some statistics about GC and allocation. The policy needs to /// decide the (current) heap limit and decide whether a GC should be performed. @@ -113,17 +138,25 @@ pub trait GCTriggerPolicy: Sync + Send { /// Inform the triggering policy that a GC starts. fn on_gc_start(&self, _mmtk: &'static MMTK) {} /// Inform the triggering policy that a GC is about to start the release work. This is called - /// in the global [`crate::scheduler::gc_work::Release`] work packet. This means we assume a plan + /// in the global Release work packet. This means we assume a plan /// do not schedule any work that reclaims memory before the global `Release` work. The current plans /// satisfy this assumption: they schedule other release work in `plan.release()`. fn on_gc_release(&self, _mmtk: &'static MMTK) {} /// Inform the triggering policy that a GC ends. fn on_gc_end(&self, _mmtk: &'static MMTK) {} - /// Is a GC required now? + /// Is a GC required now? The GC trigger may implement its own heuristics to decide when + /// a GC should be performed. However, we recommend the implementation to do its own checks + /// first, and always call `plan.collection_required(space_full, space)` at the end as a fallback to see if the plan needs + /// to do a GC. + /// + /// Arguments: + /// * `space_full`: Is any space full? + /// * `space`: The space that is full. The GC trigger may access some stats of the space. + /// * `plan`: The reference to the plan in use. fn is_gc_required( &self, space_full: bool, - space: Option<&dyn Space>, + space: Option>, plan: &dyn Plan, ) -> bool; /// Is current heap full? @@ -144,7 +177,7 @@ impl GCTriggerPolicy for FixedHeapSizeTrigger { fn is_gc_required( &self, space_full: bool, - space: Option<&dyn Space>, + space: Option>, plan: &dyn Plan, ) -> bool { // Let the plan decide @@ -341,7 +374,7 @@ impl GCTriggerPolicy for MemBalancerTrigger { fn is_gc_required( &self, space_full: bool, - space: Option<&dyn Space>, + space: Option>, plan: &dyn Plan, ) -> bool { // Let the plan decide diff --git a/src/util/heap/mod.rs b/src/util/heap/mod.rs index e980b5adff..ae974740e8 100644 --- a/src/util/heap/mod.rs +++ b/src/util/heap/mod.rs @@ -15,6 +15,8 @@ mod vmrequest; pub(crate) use self::accounting::PageAccounting; pub(crate) use self::blockpageresource::BlockPageResource; pub(crate) use self::freelistpageresource::FreeListPageResource; +pub use self::gc_trigger::GCTriggerPolicy; +pub use self::gc_trigger::SpaceStats; pub(crate) use self::heap_meta::HeapMeta; pub use self::layout::vm_layout; pub(crate) use self::monotonepageresource::MonotonePageResource; diff --git a/src/util/test_util/mock_vm.rs b/src/util/test_util/mock_vm.rs index 078ba49692..4ac620ac80 100644 --- a/src/util/test_util/mock_vm.rs +++ b/src/util/test_util/mock_vm.rs @@ -8,6 +8,7 @@ use crate::scheduler::gc_work::SFTProcessEdges; use crate::scheduler::*; use crate::util::alloc::AllocationError; use crate::util::copy::*; +use crate::util::heap::gc_trigger::GCTriggerPolicy; use crate::util::opaque_pointer::*; use crate::util::{Address, ObjectReference}; use crate::vm::object_model::specs::*; @@ -205,6 +206,7 @@ pub struct MockVM { pub post_forwarding: MockMethod, pub vm_live_bytes: MockMethod<(), usize>, pub is_collection_enabled: MockMethod<(), bool>, + pub create_gc_trigger: MockMethod<(), Box>>, // object model pub copy_object: MockMethod< ( @@ -282,6 +284,7 @@ impl Default for MockVM { post_forwarding: MockMethod::new_default(), vm_live_bytes: MockMethod::new_default(), is_collection_enabled: MockMethod::new_fixed(Box::new(|_| true)), + create_gc_trigger: MockMethod::new_unimplemented(), copy_object: MockMethod::new_unimplemented(), copy_object_to: MockMethod::new_unimplemented(), @@ -459,6 +462,10 @@ impl crate::vm::Collection for MockVM { fn vm_live_bytes() -> usize { mock!(vm_live_bytes()) } + + fn create_gc_trigger() -> Box> { + mock!(create_gc_trigger()) + } } impl crate::vm::ObjectModel for MockVM { diff --git a/src/vm/collection.rs b/src/vm/collection.rs index e7f8241ae4..a5e0a63acd 100644 --- a/src/vm/collection.rs +++ b/src/vm/collection.rs @@ -1,4 +1,5 @@ use crate::util::alloc::AllocationError; +use crate::util::heap::gc_trigger::GCTriggerPolicy; use crate::util::opaque_pointer::*; use crate::vm::VMBinding; use crate::{scheduler::*, Mutator}; @@ -158,4 +159,10 @@ pub trait Collection { // initialization is done, such as initializing class metadata for scanning objects. true } + + /// Ask the binding to create a [`GCTriggerPolicy`] if the option `gc_trigger` is set to + /// `crate::util::options::GCTriggerSelector::Delegated`. + fn create_gc_trigger() -> Box> { + unimplemented!() + } }