WIP: Make clear distinction between "GC" and "pauses"

src/global_state.rs

@@ -17,9 +17,9 @@ pub struct GlobalState {
     /// Whether MMTk is now ready for collection. This is set to true when initialize_collection() is called.
     pub(crate) initialized: AtomicBool,
     /// The current GC status.
-    pub(crate) gc_status: Mutex<GcStatus>,
+    pub(crate) pause_state: Mutex<PauseState>,
     /// When did the last GC start? Only accessed by the last parked worker.
-    pub(crate) gc_start_time: AtomicRefCell<Option<Instant>>,
+    pub(crate) pause_start_time: AtomicRefCell<Option<Instant>>,
     /// Is the current GC an emergency collection? Emergency means we may run out of memory soon, and we should
     /// attempt to collect as much as we can.
     pub(crate) emergency_collection: AtomicBool,
@@ -201,8 +201,8 @@ impl Default for GlobalState {
     fn default() -> Self {
         Self {
             initialized: AtomicBool::new(false),
-            gc_status: Mutex::new(GcStatus::NotInGC),
-            gc_start_time: AtomicRefCell::new(None),
+            pause_state: Mutex::new(PauseState::NotInPause),
+            pause_start_time: AtomicRefCell::new(None),
             stacks_prepared: AtomicBool::new(false),
             emergency_collection: AtomicBool::new(false),
             user_triggered_collection: AtomicBool::new(false),
@@ -222,11 +222,15 @@ impl Default for GlobalState {
     }
 }
 
-#[derive(PartialEq)]
-pub enum GcStatus {
-    NotInGC,
-    GcPrepare,
-    GcProper,
+/// The state of stop-the-world (STW) pauses.
+#[derive(PartialEq, Eq, Clone, Copy, Debug)]
+pub enum PauseState {
+    /// STW pause is not triggered.  All mutators can run normally.
+    NotInPause,
+    /// STW pause is triggered, but some mutators may still be running.
+    PauseTriggered,
+    /// All mutators have come to a stop.
+    MutatorsStopped,
 }
 
 /// Statistics for the live bytes in the last GC. The statistics is per space.

src/mmtk.rs

@@ -1,5 +1,5 @@
 //! MMTk instance.
-use crate::global_state::{GcStatus, GlobalState};
+use crate::global_state::{GlobalState, PauseState};
 use crate::plan::gc_requester::GCRequester;
 use crate::plan::CreateGeneralPlanArgs;
 use crate::plan::Plan;
@@ -356,30 +356,27 @@ impl<VM: VMBinding> MMTK<VM> {
         self.inside_sanity.load(Ordering::Relaxed)
     }
 
-    pub(crate) fn set_gc_status(&self, s: GcStatus) {
-        let mut gc_status = self.state.gc_status.lock().unwrap();
-        if *gc_status == GcStatus::NotInGC {
-            self.state.stacks_prepared.store(false, Ordering::SeqCst);
-            // FIXME stats
-            self.stats.start_gc();
-        }
-        *gc_status = s;
-        if *gc_status == GcStatus::NotInGC {
-            // FIXME stats
-            if self.stats.get_gathering_stats() {
-                self.stats.end_gc();
-            }
-        }
-    }
+    /// This function is called when changing the pause state.  The GC statistics module will be
+    /// notified when entering or leaving STW pause.
+    pub(crate) fn pause_state_transition(&self, new_state: PauseState) {
+        let old_state = {
+            let mut pause_state = self.state.pause_state.lock().unwrap();
+            std::mem::replace(&mut *pause_state, new_state)
+        };
 
-    /// Return true if a collection is in progress.
-    pub fn gc_in_progress(&self) -> bool {
-        *self.state.gc_status.lock().unwrap() != GcStatus::NotInGC
-    }
+        assert_ne!(
+            old_state, new_state,
+            "Setting to the same pause state.  old_state: {old_state:?}, new_state: {new_state:?}"
+        );
 
-    /// Return true if a collection is in progress and past the preparatory stage.
-    pub fn gc_in_progress_proper(&self) -> bool {
-        *self.state.gc_status.lock().unwrap() == GcStatus::GcProper
+        if old_state == PauseState::NotInPause {
+            // FIXME: This seems out of place.  We should move it to ScheduleCollection or another
+            // work packet related to stack root scanning.
+            self.state.stacks_prepared.store(false, Ordering::SeqCst);
+            self.stats.start_pause();
+        } else if new_state == PauseState::NotInPause {
+            self.stats.end_pause();
+        }
     }
 
     /// Return true if the current GC is an emergency GC.

src/scheduler/gc_work.rs

@@ -1,6 +1,6 @@
 use super::work_bucket::WorkBucketStage;
 use super::*;
-use crate::global_state::GcStatus;
+use crate::global_state::PauseState;
 use crate::plan::ObjectsClosure;
 use crate::plan::VectorObjectQueue;
 use crate::util::*;
@@ -26,7 +26,7 @@ impl<VM: VMBinding> GCWork<VM> for ScheduleCollection {
             mmtk.get_plan().notify_emergency_collection();
         }
         // Set to GcPrepare
-        mmtk.set_gc_status(GcStatus::GcPrepare);
+        mmtk.pause_state_transition(PauseState::PauseTriggered);
 
         // Let the plan to schedule collection work
         mmtk.get_plan().schedule_collection(worker.scheduler());
@@ -444,7 +444,7 @@ impl<C: GCWorkContext> GCWork<C::VM> for ScanMutatorRoots<C> {
             <C::VM as VMBinding>::VMScanning::notify_initial_thread_scan_complete(
                 false, worker.tls,
             );
-            mmtk.set_gc_status(GcStatus::GcProper);
+            mmtk.pause_state_transition(PauseState::MutatorsStopped);
         }
     }
 }

src/scheduler/scheduler.rs

@@ -7,7 +7,7 @@ use super::worker::{GCWorker, ThreadId, WorkerGroup};
 use super::worker_goals::{WorkerGoal, WorkerGoals};
 use super::worker_monitor::{LastParkedResult, WorkerMonitor};
 use super::*;
-use crate::global_state::GcStatus;
+use crate::global_state::PauseState;
 use crate::mmtk::MMTK;
 use crate::util::opaque_pointer::*;
 use crate::util::options::AffinityKind;
@@ -506,7 +506,7 @@ impl<VM: VMBinding> GCWorkScheduler<VM> {
                 probe!(mmtk, gc_start);
 
                 {
-                    let mut gc_start_time = worker.mmtk.state.gc_start_time.borrow_mut();
+                    let mut gc_start_time = worker.mmtk.state.pause_start_time.borrow_mut();
                     assert!(gc_start_time.is_none(), "GC already started?");
                     *gc_start_time = Some(Instant::now());
                 }
@@ -569,7 +569,7 @@ impl<VM: VMBinding> GCWorkScheduler<VM> {
 
         // Compute the elapsed time of the GC.
         let start_time = {
-            let mut gc_start_time = worker.mmtk.state.gc_start_time.borrow_mut();
+            let mut gc_start_time = worker.mmtk.state.pause_start_time.borrow_mut();
             gc_start_time.take().expect("GC not started yet?")
         };
         let elapsed = start_time.elapsed();
@@ -619,7 +619,7 @@ impl<VM: VMBinding> GCWorkScheduler<VM> {
         self.debug_assert_all_stw_buckets_closed();
 
         // Set to NotInGC after everything, and right before resuming mutators.
-        mmtk.set_gc_status(GcStatus::NotInGC);
+        mmtk.pause_state_transition(PauseState::NotInPause);
         <VM as VMBinding>::VMCollection::resume_mutators(worker.tls);
 
         concurrent_work_scheduled

src/util/statistics/stats.rs

@@ -41,10 +41,12 @@ impl SharedStats {
 
 /// GC statistics
 ///
-/// The struct holds basic GC statistics, like the GC count,
+/// The struct holds basic GC statistics, like the pause count,
 /// and an array of counters.
 pub struct Stats {
-    gc_count: AtomicUsize,
+    /// The number of stop-the-world (STW) pauses since statistics begins.
+    pause_count: AtomicUsize,
+    /// A reference to the time counter.  Can be used to print the total time.
     total_time: Arc<Mutex<Timer>>,
     // crate `pfm` uses libpfm4 under the hood for parsing perf event names
     // Initialization of libpfm4 is required before we can use `PerfEvent` types
@@ -92,7 +94,7 @@ impl Stats {
             ))));
         }
         Stats {
-            gc_count: AtomicUsize::new(0),
+            pause_count: AtomicUsize::new(0),
             total_time: t,
             #[cfg(feature = "perf_counter")]
             perfmon,
@@ -147,8 +149,8 @@ impl Stats {
         counter
     }
 
-    pub fn start_gc(&self) {
-        self.gc_count.fetch_add(1, Ordering::SeqCst);
+    pub fn start_pause(&self) {
+        self.pause_count.fetch_add(1, Ordering::SeqCst);
         if !self.get_gathering_stats() {
             return;
         }
@@ -159,7 +161,7 @@ impl Stats {
         self.shared.increment_phase();
     }
 
-    pub fn end_gc(&self) {
+    pub fn end_pause(&self) {
         if !self.get_gathering_stats() {
             return;
         }
@@ -200,7 +202,7 @@ impl Stats {
     }
 
     pub fn print_column_names(&self, scheduler_stat: &HashMap<String, String>) {
-        print!("GC\t");
+        print!("pauses\t");
         let counter = self.counters.lock().unwrap();
         for iter in &(*counter) {
             let c = iter.lock().unwrap();