Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/userguide/src/tutorial/code/mygc_semispace/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -78,7 +79,7 @@ impl<VM: VMBinding> Plan for MyGC<VM> {
// ANCHOR_END: schedule_collection

// ANCHOR: collection_required()
fn collection_required(&self, space_full: bool, _space: Option<&dyn Space<Self::VM>>) -> bool {
fn collection_required(&self, space_full: bool, _space: Option<SpaceStats<Self::VM>>) -> bool {
self.base().collection_required(self, space_full)
}
// ANCHOR_END: collection_required()
Expand Down
5 changes: 5 additions & 0 deletions src/mmtk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,11 @@ impl<VM: VMBinding> MMTK<VM> {
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.
///
Expand Down
3 changes: 2 additions & 1 deletion src/plan/generational/copying/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,7 +65,7 @@ impl<VM: VMBinding> Plan for GenCopy<VM> {
}
}

fn collection_required(&self, space_full: bool, space: Option<&dyn Space<Self::VM>>) -> bool
fn collection_required(&self, space_full: bool, space: Option<SpaceStats<Self::VM>>) -> bool
where
Self: Sized,
{
Expand Down
5 changes: 3 additions & 2 deletions src/plan/generational/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -98,7 +99,7 @@ impl<VM: VMBinding> CommonGenPlan<VM> {
&self,
plan: &P,
space_full: bool,
space: Option<&dyn Space<VM>>,
space: Option<SpaceStats<VM>>,
) -> bool {
let cur_nursery = self.nursery.reserved_pages();
let max_nursery = self.common.base.options.get_max_nursery_pages();
Expand All @@ -120,7 +121,7 @@ impl<VM: VMBinding> CommonGenPlan<VM> {
// - 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 {
Expand Down
3 changes: 2 additions & 1 deletion src/plan/generational/immix/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -90,7 +91,7 @@ impl<VM: VMBinding> Plan for GenImmix<VM> {
)
}

fn collection_required(&self, space_full: bool, space: Option<&dyn Space<Self::VM>>) -> bool
fn collection_required(&self, space_full: bool, space: Option<SpaceStats<Self::VM>>) -> bool
where
Self: Sized,
{
Expand Down
3 changes: 2 additions & 1 deletion src/plan/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Self::VM>>) -> bool;
fn collection_required(&self, space_full: bool, space: Option<SpaceStats<Self::VM>>) -> 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
Expand Down
3 changes: 2 additions & 1 deletion src/plan/immix/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -45,7 +46,7 @@ pub const IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints {
};

impl<VM: VMBinding> Plan for Immix<VM> {
fn collection_required(&self, space_full: bool, _space: Option<&dyn Space<Self::VM>>) -> bool {
fn collection_required(&self, space_full: bool, _space: Option<SpaceStats<Self::VM>>) -> bool {
self.base().collection_required(self, space_full)
}

Expand Down
3 changes: 2 additions & 1 deletion src/plan/markcompact/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down Expand Up @@ -159,7 +160,7 @@ impl<VM: VMBinding> Plan for MarkCompact<VM> {
.add(crate::util::sanity::sanity_checker::ScheduleSanityGC::<Self>::new(self));
}

fn collection_required(&self, space_full: bool, _space: Option<&dyn Space<Self::VM>>) -> bool {
fn collection_required(&self, space_full: bool, _space: Option<SpaceStats<Self::VM>>) -> bool {
self.base().collection_required(self, space_full)
}

Expand Down
3 changes: 2 additions & 1 deletion src/plan/marksweep/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,7 +65,7 @@ impl<VM: VMBinding> Plan for MarkSweep<VM> {
self.common.release(tls, true);
}

fn collection_required(&self, space_full: bool, _space: Option<&dyn Space<Self::VM>>) -> bool {
fn collection_required(&self, space_full: bool, _space: Option<SpaceStats<Self::VM>>) -> bool {
self.base().collection_required(self, space_full)
}

Expand Down
3 changes: 2 additions & 1 deletion src/plan/nogc/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -45,7 +46,7 @@ impl<VM: VMBinding> Plan for NoGC<VM> {
&NOGC_CONSTRAINTS
}

fn collection_required(&self, space_full: bool, _space: Option<&dyn Space<Self::VM>>) -> bool {
fn collection_required(&self, space_full: bool, _space: Option<SpaceStats<Self::VM>>) -> bool {
self.base().collection_required(self, space_full)
}

Expand Down
3 changes: 2 additions & 1 deletion src/plan/pageprotect/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -57,7 +58,7 @@ impl<VM: VMBinding> Plan for PageProtect<VM> {
self.space.release(true);
}

fn collection_required(&self, space_full: bool, _space: Option<&dyn Space<Self::VM>>) -> bool {
fn collection_required(&self, space_full: bool, _space: Option<SpaceStats<Self::VM>>) -> bool {
self.base().collection_required(self, space_full)
}

Expand Down
3 changes: 2 additions & 1 deletion src/plan/semispace/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -95,7 +96,7 @@ impl<VM: VMBinding> Plan for SemiSpace<VM> {
self.fromspace().release();
}

fn collection_required(&self, space_full: bool, _space: Option<&dyn Space<Self::VM>>) -> bool {
fn collection_required(&self, space_full: bool, _space: Option<SpaceStats<Self::VM>>) -> bool {
self.base().collection_required(self, space_full)
}

Expand Down
12 changes: 6 additions & 6 deletions src/plan/sticky/immix/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -138,14 +139,13 @@ impl<VM: VMBinding> Plan for StickyImmix<VM> {
.store(next_gc_full_heap, Ordering::Relaxed);
}

fn collection_required(
&self,
space_full: bool,
space: Option<&dyn crate::policy::space::Space<Self::VM>>,
) -> bool {
fn collection_required(&self, space_full: bool, space: Option<SpaceStats<Self::VM>>) -> 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
Expand Down
47 changes: 40 additions & 7 deletions src/util/heap/gc_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ impl<VM: VMBinding> GCTrigger<VM> {
conversions::bytes_to_pages_up(min),
conversions::bytes_to_pages_up(max),
)),
GCTriggerSelector::Delegated => unimplemented!(),
GCTriggerSelector::Delegated => {
<VM::VMCollection as crate::vm::Collection<VM>>::create_gc_trigger()
}
},
options,
gc_requester,
Expand All @@ -65,7 +67,10 @@ impl<VM: VMBinding> GCTrigger<VM> {
/// * `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<VM>>) -> 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR: Why is Plan safe to be exposed to the binding, whereas Space is not? (See L126 below in this file)

Copy link
Member

@wenyuzhao wenyuzhao Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's still possible to access spaces from a plan reference?

pub trait Plan: 'static + HasSpaces + Sync + Downcast {
/// Get the plan constraints for the plan.
/// This returns a non-constant value. A constant value can be found in each plan's module if needed.
fn constraints(&self) -> &'static PlanConstraints;
/// Create a copy config for this plan. A copying GC plan MUST override this method,
/// and provide a valid config.
fn create_copy_config(&'static self) -> CopyConfig<Self::VM> {
// Use the empty default copy config for non copying GC.
CopyConfig::default()
}
/// Get a immutable reference to the base plan. `BasePlan` is included by all the MMTk GC plans.
fn base(&self) -> &BasePlan<Self::VM>;

pub struct BasePlan<VM: VMBinding> {
pub(crate) global_state: Arc<GlobalState>,
pub options: Arc<Options>,
pub gc_trigger: Arc<GCTrigger<VM>>,
// Spaces in base plan
#[cfg(feature = "code_space")]
#[space]
pub code_space: ImmortalSpace<VM>,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR: Why is Plan safe to be exposed to the binding, whereas Space is not? (See L126 below in this file)

Space is not public before this PR, and I don't think we have a good reason to expose it. So I decided to wrap Space into a public type SpaceStats.

For Plan, I don't think we want to expose Plan either (at least some methods in Plan should not be public). Maybe we should check the visibility of Plan's methods, and have another PR to tidy it up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's still possible to access spaces from a plan reference?

pub trait Plan: 'static + HasSpaces + Sync + Downcast {
/// Get the plan constraints for the plan.
/// This returns a non-constant value. A constant value can be found in each plan's module if needed.
fn constraints(&self) -> &'static PlanConstraints;
/// Create a copy config for this plan. A copying GC plan MUST override this method,
/// and provide a valid config.
fn create_copy_config(&'static self) -> CopyConfig<Self::VM> {
// Use the empty default copy config for non copying GC.
CopyConfig::default()
}
/// Get a immutable reference to the base plan. `BasePlan` is included by all the MMTk GC plans.
fn base(&self) -> &BasePlan<Self::VM>;

pub struct BasePlan<VM: VMBinding> {
pub(crate) global_state: Arc<GlobalState>,
pub options: Arc<Options>,
pub gc_trigger: Arc<GCTrigger<VM>>,
// Spaces in base plan
#[cfg(feature = "code_space")]
#[space]
pub code_space: ImmortalSpace<VM>,

BasePlan is not public, though it appears in a public trait. It is the same case for Space in Plan::collection_required before this PR. I am not sure what is Rust's rule on this. There is a lint private_in_public, but it seems Rust does not warn for these cases.

{
info!(
"[POLL] {}{} ({}/{} pages)",
if let Some(space) = space {
Expand Down Expand Up @@ -101,6 +106,26 @@ impl<VM: VMBinding> GCTrigger<VM> {
}
}

/// 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<VM>);

impl<'a, VM: VMBinding> SpaceStats<'a, VM> {
/// Create new SpaceStats.
fn new(space: &'a dyn Space<VM>) -> 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.
Expand All @@ -113,17 +138,25 @@ pub trait GCTriggerPolicy<VM: VMBinding>: Sync + Send {
/// Inform the triggering policy that a GC starts.
fn on_gc_start(&self, _mmtk: &'static MMTK<VM>) {}
/// 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<VM>) {}
/// Inform the triggering policy that a GC ends.
fn on_gc_end(&self, _mmtk: &'static MMTK<VM>) {}
/// 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<VM>>,
space: Option<SpaceStats<VM>>,
plan: &dyn Plan<VM = VM>,
) -> bool;
/// Is current heap full?
Expand All @@ -144,7 +177,7 @@ impl<VM: VMBinding> GCTriggerPolicy<VM> for FixedHeapSizeTrigger {
fn is_gc_required(
&self,
space_full: bool,
space: Option<&dyn Space<VM>>,
space: Option<SpaceStats<VM>>,
plan: &dyn Plan<VM = VM>,
) -> bool {
// Let the plan decide
Expand Down Expand Up @@ -341,7 +374,7 @@ impl<VM: VMBinding> GCTriggerPolicy<VM> for MemBalancerTrigger {
fn is_gc_required(
&self,
space_full: bool,
space: Option<&dyn Space<VM>>,
space: Option<SpaceStats<VM>>,
plan: &dyn Plan<VM = VM>,
) -> bool {
// Let the plan decide
Expand Down
2 changes: 2 additions & 0 deletions src/util/heap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions src/util/test_util/mock_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -205,6 +206,7 @@ pub struct MockVM {
pub post_forwarding: MockMethod<VMWorkerThread, ()>,
pub vm_live_bytes: MockMethod<(), usize>,
pub is_collection_enabled: MockMethod<(), bool>,
pub create_gc_trigger: MockMethod<(), Box<dyn GCTriggerPolicy<MockVM>>>,
// object model
pub copy_object: MockMethod<
(
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -459,6 +462,10 @@ impl crate::vm::Collection<MockVM> for MockVM {
fn vm_live_bytes() -> usize {
mock!(vm_live_bytes())
}

fn create_gc_trigger() -> Box<dyn GCTriggerPolicy<MockVM>> {
mock!(create_gc_trigger())
}
}

impl crate::vm::ObjectModel<MockVM> for MockVM {
Expand Down
7 changes: 7 additions & 0 deletions src/vm/collection.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -158,4 +159,10 @@ pub trait Collection<VM: VMBinding> {
// 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<dyn GCTriggerPolicy<VM>> {
unimplemented!()
}
}