diff --git a/docs/userguide/src/migration/prefix.md b/docs/userguide/src/migration/prefix.md index c32108bba8..abb987469e 100644 --- a/docs/userguide/src/migration/prefix.md +++ b/docs/userguide/src/migration/prefix.md @@ -32,6 +32,29 @@ Notes for the mmtk-core developers: ## 0.26.0 +### Remove GC in `harness_begin` + +```admonish tldr +`harness_begin` no longer triggers a GC before collecting statistics. The user application +is responsible to trigger a GC before calling `harness_begin`. +``` + +API changes: +* module `memory_manager` + * `handle_user_collection_request` + * It now takes an argument `exhaustive` to indicate if the user triggered GC is + exhaustive (full heap) or not. +* type `Options` + * The runtime option `full_heap_sytem_gc` is removed. + +Not API change, but worth noting: + +* module `mmtk::memory_manager` + * `harness_begin` + * The function used to trigger a forced full heap GC before starting collecting statistics. + Now it no longer triggers the GC. + * The user applications and benchmarks are responsible to trigger a GC before calling `harness_begin`. + ### Rename "edge" to "slot" ```admonish tldr @@ -75,7 +98,6 @@ See also: - PR: - Example: - ## 0.25.0 ### `ObjectReference` is no longer nullable diff --git a/src/global_state.rs b/src/global_state.rs index f355e697f2..0d306051a6 100644 --- a/src/global_state.rs +++ b/src/global_state.rs @@ -25,6 +25,8 @@ pub struct GlobalState { pub(crate) emergency_collection: AtomicBool, /// Is the current GC triggered by the user? pub(crate) user_triggered_collection: AtomicBool, + /// Is the current user triggered GC an exhaustive GC (e.g. full heap)? + pub(crate) user_triggered_exhaustive_gc: AtomicBool, /// Is the current GC triggered internally by MMTK? This is unused for now. We may have internally triggered GC /// for a concurrent plan. pub(crate) internal_triggered_collection: AtomicBool, @@ -123,7 +125,9 @@ impl GlobalState { self.internal_triggered_collection .store(false, Ordering::SeqCst); self.user_triggered_collection - .store(false, Ordering::Relaxed); + .store(false, Ordering::SeqCst); + self.user_triggered_exhaustive_gc + .store(false, Ordering::SeqCst); } /// Are the stacks scanned? @@ -204,6 +208,7 @@ impl Default for GlobalState { stacks_prepared: AtomicBool::new(false), emergency_collection: AtomicBool::new(false), user_triggered_collection: AtomicBool::new(false), + user_triggered_exhaustive_gc: AtomicBool::new(false), internal_triggered_collection: AtomicBool::new(false), last_internal_triggered_collection: AtomicBool::new(false), allocation_success: AtomicBool::new(false), diff --git a/src/memory_manager.rs b/src/memory_manager.rs index 6f41a3951b..b747d7fd35 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -571,8 +571,13 @@ pub fn total_bytes(mmtk: &MMTK) -> usize { /// Arguments: /// * `mmtk`: A reference to an MMTk instance. /// * `tls`: The thread that triggers this collection request. -pub fn handle_user_collection_request(mmtk: &MMTK, tls: VMMutatorThread) { - mmtk.handle_user_collection_request(tls, false, false); +/// * `exhaustive`: The GC should be exhaustive (e.g. full heap GC, compaction GCs, etc.) +pub fn handle_user_collection_request( + mmtk: &MMTK, + tls: VMMutatorThread, + exhaustive: bool, +) { + mmtk.handle_user_collection_request(tls, exhaustive); } /// Is the object alive? @@ -710,8 +715,9 @@ pub fn add_phantom_candidate(mmtk: &MMTK, reff: ObjectReferen mmtk.reference_processors.add_phantom_candidate(reff); } -/// Generic hook to allow benchmarks to be harnessed. We do a full heap -/// GC, and then start recording statistics for MMTk. +/// Generic hook to allow benchmarks to be harnessed. It is recommended that +/// the application/benchmark should trigger a GC through [`crate::memory_manager::handle_user_collection_request`] +/// before calling `harness_begin`. /// /// Arguments: /// * `mmtk`: A reference to an MMTk instance. diff --git a/src/mmtk.rs b/src/mmtk.rs index eea56e7e45..d3b68ed623 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -318,12 +318,10 @@ impl MMTK { self.scheduler.respawn_gc_threads_after_forking(tls); } - /// Generic hook to allow benchmarks to be harnessed. MMTk will trigger a GC - /// to clear any residual garbage and start collecting statistics for the benchmark. + /// Generic hook to allow benchmarks to be harnessed. /// This is usually called by the benchmark harness as its last step before the actual benchmark. - pub fn harness_begin(&self, tls: VMMutatorThread) { + pub fn harness_begin(&self, _tls: VMMutatorThread) { probe!(mmtk, harness_begin); - self.handle_user_collection_request(tls, true, true); self.inside_harness.store(true, Ordering::SeqCst); self.stats.start_all(); self.scheduler.enable_stat(); @@ -403,35 +401,26 @@ impl MMTK { } /// The application code has requested a collection. This is just a GC hint, and - /// we may ignore it. + /// we may ignore it (See `ignore_system_gc` and `full_heap_system_gc` in [`crate::util::options::Options`]). /// /// # Arguments /// * `tls`: The mutator thread that requests the GC - /// * `force`: The request cannot be ignored (except for NoGC) - /// * `exhaustive`: The requested GC should be exhaustive. This is also a hint. - pub fn handle_user_collection_request( - &self, - tls: VMMutatorThread, - force: bool, - exhaustive: bool, - ) { + /// * `exhaustive`: The GC should be exhaustive (e.g. full heap GC, compaction GCs, etc.) + pub fn handle_user_collection_request(&self, tls: VMMutatorThread, exhaustive: bool) { use crate::vm::Collection; if !self.get_plan().constraints().collects_garbage { warn!("User attempted a collection request, but the plan can not do GC. The request is ignored."); return; } - if force || !*self.options.ignore_system_gc && VM::VMCollection::is_collection_enabled() { + if !*self.options.ignore_system_gc && VM::VMCollection::is_collection_enabled() { info!("User triggering collection"); - if exhaustive { - if let Some(gen) = self.get_plan().generational() { - gen.force_full_heap_collection(); - } - } - self.state .user_triggered_collection - .store(true, Ordering::Relaxed); + .store(true, Ordering::SeqCst); + self.state + .user_triggered_exhaustive_gc + .store(exhaustive, Ordering::SeqCst); self.gc_requester.request(); VM::VMCollection::block_for_gc(tls); } diff --git a/src/plan/generational/global.rs b/src/plan/generational/global.rs index 55054c7388..87d3645ba2 100644 --- a/src/plan/generational/global.rs +++ b/src/plan/generational/global.rs @@ -152,7 +152,12 @@ impl CommonGenPlan { .global_state .user_triggered_collection .load(Ordering::SeqCst) - && *self.common.base.options.full_heap_system_gc + && self + .common + .base + .global_state + .user_triggered_exhaustive_gc + .load(Ordering::SeqCst) { trace!("full heap: user triggered"); // User triggered collection, and we force full heap for user triggered collection diff --git a/src/plan/immix/global.rs b/src/plan/immix/global.rs index 6292b01aa2..17450eac94 100644 --- a/src/plan/immix/global.rs +++ b/src/plan/immix/global.rs @@ -180,7 +180,10 @@ impl Immix { .cur_collection_attempts .load(Ordering::SeqCst), plan.base().global_state.is_user_triggered_collection(), - *plan.base().options.full_heap_system_gc, + plan.base() + .global_state + .user_triggered_exhaustive_gc + .load(Ordering::SeqCst), ); if in_defrag { diff --git a/src/plan/sticky/immix/global.rs b/src/plan/sticky/immix/global.rs index 4a0d9ab14c..9628f908eb 100644 --- a/src/plan/sticky/immix/global.rs +++ b/src/plan/sticky/immix/global.rs @@ -354,7 +354,13 @@ impl StickyImmix { .global_state .user_triggered_collection .load(Ordering::SeqCst) - && *self.immix.common.base.options.full_heap_system_gc + && self + .immix + .common + .base + .global_state + .user_triggered_exhaustive_gc + .load(Ordering::SeqCst) { // User triggered collection, and we force full heap for user triggered collection true diff --git a/src/util/options.rs b/src/util/options.rs index 3d1e70a965..30a6644f0f 100644 --- a/src/util/options.rs +++ b/src/util/options.rs @@ -806,8 +806,6 @@ options! { /// to 10% of the heap size while using the default value for max nursery. nursery: NurserySize [env_var: true, command_line: true] [|v: &NurserySize| v.validate()] = NurserySize::ProportionalBounded { min: DEFAULT_PROPORTIONAL_MIN_NURSERY, max: DEFAULT_PROPORTIONAL_MAX_NURSERY }, - /// Should a major GC be performed when a system GC is required? - full_heap_system_gc: bool [env_var: true, command_line: true] [always_valid] = false, /// Should finalization be disabled? no_finalizer: bool [env_var: true, command_line: true] [always_valid] = false, /// Should reference type processing be disabled?