Collect live bytes per space, and report by space (#1238)

The current `count_live_bytes_in_gc` feature adds the size of all live
objects and compare with the used pages reported by the plan. There are
two issues with the feature:
1. VM space is not included in the used pages reported by the plan, but
the live objects include objects in the VM space. So the reported
fragmentation/utilization is wrong when the VM space is in use.
2. Spaces/policies have very different fragmentation ratio. Reporting
the fragmentation for the entire heap is not useful.

This PR refactors the current `count_live_bytes_in_gc` feature so we
collect live bytes per space, and report by space.

Author: qinsoon

Cargo.toml

@@ -172,9 +172,6 @@ work_packet_stats = []
 # Count the malloc'd memory into the heap size
 malloc_counted_size = []
 
-# Count the size of all live objects in GC
-count_live_bytes_in_gc = []
-
 # Workaround a problem where bpftrace scripts (see tools/tracing/timeline/capture.bt) cannot
 # capture the type names of work packets.
 bpftrace_workaround = []

docs/userguide/src/migration/prefix.md

@@ -32,6 +32,28 @@ Notes for the mmtk-core developers:
 
 ## 0.30.0
 
+### `live_bytes_in_last_gc` becomes a runtime option, and returns a map for live bytes in each space
+
+```admonish tldr
+`count_live_bytes_in_gc` is now a runtime option instead of a features (build-time), and we collect
+live bytes statistics per space. Correspondingly, `memory_manager::live_bytes_in_last_gc` now returns a map for
+live bytes in each space.
+```
+
+API changes:
+
+-   module `util::options`
+    +   `Options` includes `count_live_bytes_in_gc`, which defaults to `false`. This can be turned on at run-time.
+    +   The old `count_live_bytes_in_gc` feature is removed.
+-   module `memory_manager`
+    +   `live_bytes_in_last_gc` now returns a `HashMap<&'static str, LiveBytesStats>`. The keys are
+        strings for space names, and the values are statistics for live bytes in the space.
+
+See also:
+
+-   PR: <https://github.com/mmtk/mmtk-core/pull/1238>
+
+
 ### mmap-related functions require annotation
 
 ```admonish tldr

src/global_state.rs

@@ -1,9 +1,9 @@
+use atomic_refcell::AtomicRefCell;
+use std::collections::HashMap;
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::Mutex;
 use std::time::Instant;
 
-use atomic_refcell::AtomicRefCell;
-
 /// This stores some global states for an MMTK instance.
 /// Some MMTK components like plans and allocators may keep an reference to the struct, and can access it.
 // This used to be a part of the `BasePlan`. In that case, any component that accesses
@@ -45,9 +45,8 @@ pub struct GlobalState {
     /// A counteer that keeps tracks of the number of bytes allocated by malloc
     #[cfg(feature = "malloc_counted_size")]
     pub(crate) malloc_bytes: AtomicUsize,
-    /// This stores the size in bytes for all the live objects in last GC. This counter is only updated in the GC release phase.
-    #[cfg(feature = "count_live_bytes_in_gc")]
-    pub(crate) live_bytes_in_last_gc: AtomicUsize,
+    /// This stores the live bytes and the used bytes (by pages) for each space in last GC. This counter is only updated in the GC release phase.
+    pub(crate) live_bytes_in_last_gc: AtomicRefCell<HashMap<&'static str, LiveBytesStats>>,
 }
 
 impl GlobalState {
@@ -183,16 +182,6 @@ impl GlobalState {
     pub(crate) fn decrease_malloc_bytes_by(&self, size: usize) {
         self.malloc_bytes.fetch_sub(size, Ordering::SeqCst);
     }
-
-    #[cfg(feature = "count_live_bytes_in_gc")]
-    pub fn get_live_bytes_in_last_gc(&self) -> usize {
-        self.live_bytes_in_last_gc.load(Ordering::SeqCst)
-    }
-
-    #[cfg(feature = "count_live_bytes_in_gc")]
-    pub fn set_live_bytes_in_last_gc(&self, size: usize) {
-        self.live_bytes_in_last_gc.store(size, Ordering::SeqCst);
-    }
 }
 
 impl Default for GlobalState {
@@ -213,8 +202,7 @@ impl Default for GlobalState {
             allocation_bytes: AtomicUsize::new(0),
             #[cfg(feature = "malloc_counted_size")]
             malloc_bytes: AtomicUsize::new(0),
-            #[cfg(feature = "count_live_bytes_in_gc")]
-            live_bytes_in_last_gc: AtomicUsize::new(0),
+            live_bytes_in_last_gc: AtomicRefCell::new(HashMap::new()),
         }
     }
 }
@@ -225,3 +213,15 @@ pub enum GcStatus {
     GcPrepare,
     GcProper,
 }
+
+/// Statistics for the live bytes in the last GC. The statistics is per space.
+#[derive(Copy, Clone, Debug)]
+pub struct LiveBytesStats {
+    /// Total accumulated bytes of live objects in the space.
+    pub live_bytes: usize,
+    /// Total pages used by the space.
+    pub used_pages: usize,
+    /// Total bytes used by the space, computed from `used_pages`.
+    /// The ratio of live_bytes and used_bytes reflects the utilization of the memory in the space.
+    pub used_bytes: usize,
+}

src/lib.rs

@@ -46,6 +46,7 @@ pub(crate) use mmtk::MMAPPER;
 pub use mmtk::MMTK;
 
 mod global_state;
+pub use crate::global_state::LiveBytesStats;
 
 mod policy;
 

src/memory_manager.rs

@@ -26,6 +26,8 @@ use crate::vm::slot::MemorySlice;
 use crate::vm::ReferenceGlue;
 use crate::vm::VMBinding;
 
+use std::collections::HashMap;
+
 /// Initialize an MMTk instance. A VM should call this method after creating an [`crate::MMTK`]
 /// instance but before using any of the methods provided in MMTk (except `process()` and `process_bulk()`).
 ///
@@ -531,16 +533,18 @@ pub fn free_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
     mmtk.get_plan().get_free_pages() << LOG_BYTES_IN_PAGE
 }
 
-/// Return the size of all the live objects in bytes in the last GC. MMTk usually accounts for memory in pages.
+/// Return a hash map for live bytes statistics in the last GC for each space.
+///
+/// MMTk usually accounts for memory in pages by each space.
 /// This is a special method that we count the size of every live object in a GC, and sum up the total bytes.
-/// We provide this method so users can compare with `used_bytes` (which does page accounting), and know if
-/// the heap is fragmented.
+/// We provide this method so users can use [`crate::LiveBytesStats`] to know if
+/// the space is fragmented.
 /// The value returned by this method is only updated when we finish tracing in a GC. A recommended timing
 /// to call this method is at the end of a GC (e.g. when the runtime is about to resume threads).
-#[cfg(feature = "count_live_bytes_in_gc")]
-pub fn live_bytes_in_last_gc<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
-    use std::sync::atomic::Ordering;
-    mmtk.state.live_bytes_in_last_gc.load(Ordering::SeqCst)
+pub fn live_bytes_in_last_gc<VM: VMBinding>(
+    mmtk: &MMTK<VM>,
+) -> HashMap<&'static str, crate::LiveBytesStats> {
+    mmtk.state.live_bytes_in_last_gc.borrow().clone()
 }
 
 /// Return the starting address of the heap. *Note that currently MMTk uses

src/mmtk.rs

@@ -12,6 +12,7 @@ use crate::util::address::ObjectReference;
 use crate::util::analysis::AnalysisManager;
 use crate::util::finalizable_processor::FinalizableProcessor;
 use crate::util::heap::gc_trigger::GCTrigger;
+use crate::util::heap::layout::heap_parameters::MAX_SPACES;
 use crate::util::heap::layout::vm_layout::VMLayout;
 use crate::util::heap::layout::{self, Mmapper, VMMap};
 use crate::util::heap::HeapMeta;
@@ -26,6 +27,7 @@ use crate::util::statistics::stats::Stats;
 use crate::vm::ReferenceGlue;
 use crate::vm::VMBinding;
 use std::cell::UnsafeCell;
+use std::collections::HashMap;
 use std::default::Default;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
@@ -526,4 +528,34 @@ impl<VM: VMBinding> MMTK<VM> {
             space.enumerate_objects(&mut enumerator);
         })
     }
+
+    /// Aggregate a hash map of live bytes per space with the space stats to produce
+    /// a map of live bytes stats for the spaces.
+    pub(crate) fn aggregate_live_bytes_in_last_gc(
+        &self,
+        live_bytes_per_space: [usize; MAX_SPACES],
+    ) -> HashMap<&'static str, crate::LiveBytesStats> {
+        use crate::policy::space::Space;
+        let mut ret = HashMap::new();
+        self.get_plan().for_each_space(&mut |space: &dyn Space<VM>| {
+            let space_name = space.get_name();
+            let space_idx = space.get_descriptor().get_index();
+            let used_pages = space.reserved_pages();
+            if used_pages != 0 {
+                let used_bytes = crate::util::conversions::pages_to_bytes(used_pages);
+                let live_bytes = live_bytes_per_space[space_idx];
+                debug_assert!(
+                    live_bytes <= used_bytes,
+                    "Live bytes of objects in {} ({} bytes) is larger than used pages ({} bytes), something is wrong.",
+                    space_name, live_bytes, used_bytes
+                );
+                ret.insert(space_name, crate::LiveBytesStats {
+                    live_bytes,
+                    used_pages,
+                    used_bytes,
+                });
+            }
+        });
+        ret
+    }
 }

src/plan/markcompact/gc_work.rs

@@ -51,7 +51,6 @@ impl<VM: VMBinding> GCWork<VM> for UpdateReferences<VM> {
         mmtk.slot_logger.reset();
 
         // We do two passes of transitive closures. We clear the live bytes from the first pass.
-        #[cfg(feature = "count_live_bytes_in_gc")]
         mmtk.scheduler
             .worker_group
             .get_and_clear_worker_live_bytes();

src/policy/copyspace.rs

@@ -25,7 +25,7 @@ pub struct CopySpace<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> SFT for CopySpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.get_name()
     }
 

src/policy/immix/immixspace.rs

@@ -86,7 +86,7 @@ pub struct ImmixSpaceArgs {
 unsafe impl<VM: VMBinding> Sync for ImmixSpace<VM> {}
 
 impl<VM: VMBinding> SFT for ImmixSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.get_name()
     }
 

src/policy/immortalspace.rs

@@ -27,7 +27,7 @@ pub struct ImmortalSpace<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> SFT for ImmortalSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.get_name()
     }
     fn is_live(&self, _object: ObjectReference) -> bool {

src/policy/largeobjectspace.rs

@@ -32,7 +32,7 @@ pub struct LargeObjectSpace<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> SFT for LargeObjectSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.get_name()
     }
     fn is_live(&self, object: ObjectReference) -> bool {

src/policy/lockfreeimmortalspace.rs

@@ -46,7 +46,7 @@ pub struct LockFreeImmortalSpace<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> SFT for LockFreeImmortalSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.get_name()
     }
     fn is_live(&self, _object: ObjectReference) -> bool {

src/policy/markcompactspace.rs

@@ -33,7 +33,7 @@ pub const GC_EXTRA_HEADER_WORD: usize = 1;
 const GC_EXTRA_HEADER_BYTES: usize = GC_EXTRA_HEADER_WORD << LOG_BYTES_IN_WORD;
 
 impl<VM: VMBinding> SFT for MarkCompactSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.get_name()
     }
 

src/policy/marksweepspace/malloc_ms/global.rs

@@ -64,7 +64,7 @@ pub struct MallocSpace<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> SFT for MallocSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.get_name()
     }
 

src/policy/marksweepspace/native_ms/global.rs

@@ -157,7 +157,7 @@ impl AbandonedBlockLists {
 }
 
 impl<VM: VMBinding> SFT for MarkSweepSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.common.name
     }
 

src/policy/sft.rs

@@ -24,7 +24,7 @@ use std::marker::PhantomData;
 /// table of SFT rather than Space.
 pub trait SFT {
     /// The space name
-    fn name(&self) -> &str;
+    fn name(&self) -> &'static str;
 
     /// Get forwarding pointer if the object is forwarded.
     fn get_forwarded_object(&self, _object: ObjectReference) -> Option<ObjectReference> {
@@ -120,7 +120,7 @@ pub const EMPTY_SFT_NAME: &str = "empty";
 pub const EMPTY_SPACE_SFT: EmptySpaceSFT = EmptySpaceSFT {};
 
 impl SFT for EmptySpaceSFT {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         EMPTY_SFT_NAME
     }
     fn is_live(&self, object: ObjectReference) -> bool {

src/policy/space.rs

@@ -328,6 +328,10 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
         self.common().name
     }
 
+    fn get_descriptor(&self) -> SpaceDescriptor {
+        self.common().descriptor
+    }
+
     fn common(&self) -> &CommonSpace<VM>;
     fn get_gc_trigger(&self) -> &GCTrigger<VM> {
         self.common().gc_trigger.as_ref()

src/policy/vmspace.rs

@@ -29,7 +29,7 @@ pub struct VMSpace<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> SFT for VMSpace<VM> {
-    fn name(&self) -> &str {
+    fn name(&self) -> &'static str {
         self.common.name
     }
     fn is_live(&self, _object: ObjectReference) -> bool {

src/scheduler/gc_work.rs

@@ -154,14 +154,12 @@ impl<C: GCWorkContext + 'static> GCWork<C::VM> for Release<C> {
             debug_assert!(result.is_ok());
         }
 
-        #[cfg(feature = "count_live_bytes_in_gc")]
-        {
-            let live_bytes = mmtk
-                .scheduler
-                .worker_group
-                .get_and_clear_worker_live_bytes();
-            mmtk.state.set_live_bytes_in_last_gc(live_bytes);
-        }
+        let live_bytes = mmtk
+            .scheduler
+            .worker_group
+            .get_and_clear_worker_live_bytes();
+        *mmtk.state.live_bytes_in_last_gc.borrow_mut() =
+            mmtk.aggregate_live_bytes_in_last_gc(live_bytes);
     }
 }
 
@@ -820,7 +818,7 @@ pub trait ScanObjectsWork<VM: VMBinding>: GCWork<VM> + Sized {
         &self,
         buffer: &[ObjectReference],
         worker: &mut GCWorker<<Self::E as ProcessEdgesWork>::VM>,
-        _mmtk: &'static MMTK<<Self::E as ProcessEdgesWork>::VM>,
+        mmtk: &'static MMTK<<Self::E as ProcessEdgesWork>::VM>,
     ) {
         let tls = worker.tls;
 
@@ -830,14 +828,21 @@ pub trait ScanObjectsWork<VM: VMBinding>: GCWork<VM> + Sized {
         let mut scan_later = vec![];
         {
             let mut closure = ObjectsClosure::<Self::E>::new(worker, self.get_bucket());
-            for object in objects_to_scan.iter().copied() {
-                // For any object we need to scan, we count its liv bytes
-                #[cfg(feature = "count_live_bytes_in_gc")]
-                closure
-                    .worker
-                    .shared
-                    .increase_live_bytes(VM::VMObjectModel::get_current_size(object));
 
+            // For any object we need to scan, we count its live bytes.
+            // Check the option outside the loop for better performance.
+            if crate::util::rust_util::unlikely(*mmtk.get_options().count_live_bytes_in_gc) {
+                // Borrow before the loop.
+                let mut live_bytes_stats = closure.worker.shared.live_bytes_per_space.borrow_mut();
+                for object in objects_to_scan.iter().copied() {
+                    crate::scheduler::worker::GCWorkerShared::<VM>::increase_live_bytes(
+                        &mut live_bytes_stats,
+                        object,
+                    );
+                }
+            }
+
+            for object in objects_to_scan.iter().copied() {
                 if <VM as VMBinding>::VMScanning::support_slot_enqueuing(tls, object) {
                     trace!("Scan object (slot) {}", object);
                     // If an object supports slot-enqueuing, we enqueue its slots.