Side metadata performance regression (#264)

* reduce WCR of metadata space for 32-bits
* fix bzero and unmap for 32-bits policy-specific
* add the side_gc_header feature

Author: Javad Amiri

Cargo.toml

@@ -65,6 +65,9 @@ nogc_no_zeroing = ["nogc_lock_free"]
 # Q: Why do we need this as a compile time flat? We can always set the number of GC threads through options.
 single_worker = []
 
+# To support VMs that do not provide in-header GC byte
+side_gc_header = []
+
 # Do not modify the following line - ci-common.sh matches it
 # -- Mutally exclusive features --
 # Only one feature from each group can be provided. Otherwise build will fail.

docs/tutorial/code/mygc_semispace/global.rs

@@ -9,6 +9,7 @@ use crate::plan::mygc::mutator::create_mygc_mutator;
 use crate::plan::mygc::mutator::ALLOCATOR_MAPPING;
 use crate::plan::AllocationSemantics;
 use crate::plan::Plan;
+use crate::plan::PlanConstraints;
 use crate::policy::copyspace::CopySpace; // Add
 use crate::policy::space::Space;
 use crate::scheduler::gc_work::*; // Add
@@ -22,10 +23,9 @@ use crate::util::heap::VMRequest;
 use crate::util::options::UnsafeOptionsWrapper;
 use crate::util::OpaquePointer;
 use crate::vm::VMBinding;
-use crate::plan::PlanConstraints;
-use std::sync::atomic::{AtomicBool, Ordering}; // Add 
-use std::sync::Arc;
 use enum_map::EnumMap;
+use std::sync::atomic::{AtomicBool, Ordering}; // Add
+use std::sync::Arc;
 // Remove #[allow(unused_imports)].
 // Remove handle_user_collection_request.
 // ANCHOR_END: imports
@@ -37,10 +37,10 @@ pub const ALLOC_MyGC: AllocationSemantics = AllocationSemantics::Default; // Add
 // Modify
 // ANCHOR: plan_def
 pub struct MyGC<VM: VMBinding> {
-    pub hi: AtomicBool, 
+    pub hi: AtomicBool,
     pub copyspace0: CopySpace<VM>,
     pub copyspace1: CopySpace<VM>,
-    pub common: CommonPlan<VM>, 
+    pub common: CommonPlan<VM>,
 }
 // ANCHOR_END: plan_def
 
@@ -78,7 +78,7 @@ impl<VM: VMBinding> Plan for MyGC<VM> {
     // Modify
     // ANCHOR: gc_init
     fn gc_init(
-        &mut self, 
+        &mut self,
         heap_size: usize,
         vm_map: &'static VMMap,
         scheduler: &Arc<MMTkScheduler<VM>>,
@@ -91,13 +91,15 @@ impl<VM: VMBinding> Plan for MyGC<VM> {
 
     // Modify
     // ANCHOR: schedule_collection
-    fn schedule_collection(&'static self, scheduler:&MMTkScheduler<VM>) {
+    fn schedule_collection(&'static self, scheduler: &MMTkScheduler<VM>) {
         self.base().set_collection_kind();
         self.base().set_gc_status(GcStatus::GcPrepare);
         scheduler.work_buckets[WorkBucketStage::Unconstrained]
             .add(StopMutators::<MyGCProcessEdges<VM>>::new());
-        scheduler.work_buckets[WorkBucketStage::Prepare].add(Prepare::<Self, MyGCCopyContext<VM>>::new(self));
-        scheduler.work_buckets[WorkBucketStage::Release].add(Release::<Self, MyGCCopyContext<VM>>::new(self));
+        scheduler.work_buckets[WorkBucketStage::Prepare]
+            .add(Prepare::<Self, MyGCCopyContext<VM>>::new(self));
+        scheduler.work_buckets[WorkBucketStage::Release]
+            .add(Release::<Self, MyGCCopyContext<VM>>::new(self));
         scheduler.set_finalizer(Some(EndOfGC));
     }
     // ANCHOR_END: schedule_collection
@@ -114,7 +116,7 @@ impl<VM: VMBinding> Plan for MyGC<VM> {
         self.hi
             .store(!self.hi.load(Ordering::SeqCst), Ordering::SeqCst);
         // Flips 'hi' to flip space definitions
-        let hi = self.hi.load(Ordering::SeqCst); 
+        let hi = self.hi.load(Ordering::SeqCst);
         self.copyspace0.prepare(hi);
         self.copyspace1.prepare(!hi);
     }
@@ -134,7 +136,7 @@ impl<VM: VMBinding> Plan for MyGC<VM> {
         self.tospace().reserved_pages()
     }
     // ANCHOR_END: plan_get_collection_reserve
- 
+
     // Modify
     // ANCHOR: plan_get_pages_used
     fn get_pages_used(&self) -> usize {
@@ -191,11 +193,11 @@ impl<VM: VMBinding> MyGC<VM> {
                 mmapper,
                 &mut heap,
             ),
-            common: CommonPlan::new(vm_map, mmapper, options, heap, &MYGC_CONSTRAINTS),
+            common: CommonPlan::new(vm_map, mmapper, options, heap, &MYGC_CONSTRAINTS, &[]),
         }
     }
     // ANCHOR_END: plan_new
-    
+
     // ANCHOR: plan_space_access
     pub fn tospace(&self) -> &CopySpace<VM> {
         if self.hi.load(Ordering::SeqCst) {

docs/tutorial/src/mygc/ss/exercise_solution.md

@@ -65,7 +65,7 @@ In `global.rs`:
                  mmapper,
                  &mut heap,
              ),
-             common: CommonPlan::new(vm_map, mmapper, options, heap, &TRIPLESPACE_CONSTRAINTS),
+             common: CommonPlan::new(vm_map, mmapper, options, heap, &TRIPLESPACE_CONSTRAINTS, &[]),
          }
      }
       ```

src/plan/gencopy/global.rs

@@ -3,7 +3,6 @@ use super::{
     gc_work::{GenCopyCopyContext, GenCopyMatureProcessEdges, GenCopyNurseryProcessEdges},
     LOGGING_META,
 };
-use crate::plan::global::BasePlan;
 use crate::plan::global::CommonPlan;
 use crate::plan::global::GcStatus;
 use crate::plan::AllocationSemantics;
@@ -23,11 +22,10 @@ use crate::util::heap::VMRequest;
 use crate::util::options::UnsafeOptionsWrapper;
 #[cfg(feature = "sanity")]
 use crate::util::sanity::sanity_checker::*;
-use crate::util::side_metadata::meta_bytes_per_chunk;
 use crate::util::OpaquePointer;
-use crate::vm::ObjectModel;
 use crate::vm::*;
 use crate::{mmtk::MMTK, plan::barriers::BarrierSelector};
+use crate::{plan::global::BasePlan, util::side_metadata::SideMetadataSpec};
 use enum_map::EnumMap;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
@@ -174,16 +172,8 @@ impl<VM: VMBinding> Plan for GenCopy<VM> {
         self.in_nursery.load(Ordering::SeqCst)
     }
 
-    fn global_side_metadata_per_chunk(&self) -> usize {
-        let mut side_metadata_per_chunk = if !VM::VMObjectModel::HAS_GC_BYTE {
-            meta_bytes_per_chunk(3, 1)
-        } else {
-            0
-        };
-        if super::ACTIVE_BARRIER == BarrierSelector::ObjectBarrier {
-            side_metadata_per_chunk += LOGGING_META.meta_bytes_per_chunk();
-        }
-        side_metadata_per_chunk
+    fn global_side_metadata_specs(&self) -> &[SideMetadataSpec] {
+        &self.common().global_metadata_specs
     }
 }
 
@@ -195,6 +185,11 @@ impl<VM: VMBinding> GenCopy<VM> {
         scheduler: &'static MMTkScheduler<VM>,
     ) -> Self {
         let mut heap = HeapMeta::new(HEAP_START, HEAP_END);
+        let global_metadata_specs = if super::ACTIVE_BARRIER == BarrierSelector::ObjectBarrier {
+            vec![LOGGING_META]
+        } else {
+            vec![]
+        };
 
         GenCopy {
             nursery: CopySpace::new(
@@ -225,7 +220,14 @@ impl<VM: VMBinding> GenCopy<VM> {
                 mmapper,
                 &mut heap,
             ),
-            common: CommonPlan::new(vm_map, mmapper, options, heap, &GENCOPY_CONSTRAINTS),
+            common: CommonPlan::new(
+                vm_map,
+                mmapper,
+                options,
+                heap,
+                &GENCOPY_CONSTRAINTS,
+                &&global_metadata_specs,
+            ),
             in_nursery: AtomicBool::default(),
             scheduler,
         }

src/plan/gencopy/mod.rs

@@ -24,11 +24,17 @@ pub const NO_SLOW: bool = false;
 
 pub use self::global::GENCOPY_CONSTRAINTS;
 
+#[cfg(feature = "side_gc_header")]
+use crate::util::gc_byte;
 use crate::util::side_metadata::*;
 
 const LOGGING_META: SideMetadataSpec = SideMetadataSpec {
     scope: SideMetadataScope::Global,
-    offset: 0,
+    #[cfg(not(feature = "side_gc_header"))]
+    offset: GLOBAL_SIDE_METADATA_BASE_ADDRESS.as_usize(),
+    #[cfg(feature = "side_gc_header")]
+    offset: GLOBAL_SIDE_METADATA_BASE_ADDRESS.as_usize()
+        + metadata_address_range_size(gc_byte::SIDE_GC_BYTE_SPEC),
     log_num_of_bits: 0,
     log_min_obj_size: 3,
 };

src/plan/global.rs

@@ -2,7 +2,6 @@
 
 use super::controller_collector_context::ControllerCollectorContext;
 use super::PlanConstraints;
-use crate::mmtk::MMTK;
 use crate::plan::transitive_closure::TransitiveClosure;
 use crate::plan::Mutator;
 use crate::policy::immortalspace::ImmortalSpace;
@@ -14,6 +13,7 @@ use crate::util::alloc::allocators::AllocatorSelector;
 #[cfg(feature = "analysis")]
 use crate::util::analysis::AnalysisManager;
 use crate::util::conversions::bytes_to_pages;
+use crate::util::gc_byte;
 use crate::util::heap::layout::heap_layout::Mmapper;
 use crate::util::heap::layout::heap_layout::VMMap;
 use crate::util::heap::layout::map::Map;
@@ -25,6 +25,7 @@ use crate::util::statistics::stats::Stats;
 use crate::util::OpaquePointer;
 use crate::util::{Address, ObjectReference};
 use crate::vm::*;
+use crate::{mmtk::MMTK, util::side_metadata::SideMetadataSpec};
 use downcast_rs::Downcast;
 use enum_map::EnumMap;
 use std::cell::UnsafeCell;
@@ -341,8 +342,8 @@ pub trait Plan: 'static + Sync + Send + Downcast {
         }
     }
 
-    fn global_side_metadata_per_chunk(&self) -> usize {
-        0
+    fn global_side_metadata_specs(&self) -> &[SideMetadataSpec] {
+        &[]
     }
 }
 
@@ -712,6 +713,7 @@ CommonPlan is for representing state and features used by _many_ plans, but that
 pub struct CommonPlan<VM: VMBinding> {
     pub unsync: UnsafeCell<CommonUnsync<VM>>,
     pub base: BasePlan<VM>,
+    pub global_metadata_specs: Vec<SideMetadataSpec>,
 }
 
 pub struct CommonUnsync<VM: VMBinding> {
@@ -726,7 +728,14 @@ impl<VM: VMBinding> CommonPlan<VM> {
         options: Arc<UnsafeOptionsWrapper>,
         mut heap: HeapMeta,
         constraints: &'static PlanConstraints,
+        global_side_metadata_specs: &[SideMetadataSpec],
     ) -> CommonPlan<VM> {
+        let mut specs = if cfg!(feature = "side_gc_header") {
+            vec![gc_byte::SIDE_GC_BYTE_SPEC]
+        } else {
+            vec![]
+        };
+        specs.extend_from_slice(global_side_metadata_specs);
         CommonPlan {
             unsync: UnsafeCell::new(CommonUnsync {
                 immortal: ImmortalSpace::new(
@@ -749,6 +758,7 @@ impl<VM: VMBinding> CommonPlan<VM> {
                 ),
             }),
             base: BasePlan::new(vm_map, mmapper, options, heap, constraints),
+            global_metadata_specs: specs,
         }
     }
 

src/plan/marksweep/global.rs

@@ -127,7 +127,7 @@ impl<VM: VMBinding> MarkSweep<VM> {
     ) -> Self {
         let heap = HeapMeta::new(HEAP_START, HEAP_END);
         MarkSweep {
-            common: CommonPlan::new(vm_map, mmapper, options, heap, &MS_CONSTRAINTS),
+            common: CommonPlan::new(vm_map, mmapper, options, heap, &MS_CONSTRAINTS, &[]),
             ms: MallocSpace::new(),
         }
     }

src/plan/semispace/global.rs

@@ -1,6 +1,4 @@
 use super::gc_work::{SSCopyContext, SSProcessEdges};
-use crate::mmtk::MMTK;
-use crate::plan::global::BasePlan;
 use crate::plan::global::CommonPlan;
 use crate::plan::global::GcStatus;
 use crate::plan::semispace::mutator::ALLOCATOR_MAPPING;
@@ -22,10 +20,9 @@ use crate::util::heap::VMRequest;
 use crate::util::options::UnsafeOptionsWrapper;
 #[cfg(feature = "sanity")]
 use crate::util::sanity::sanity_checker::*;
-use crate::util::side_metadata::meta_bytes_per_chunk;
 use crate::util::OpaquePointer;
-use crate::vm::ObjectModel;
-use crate::vm::VMBinding;
+use crate::{mmtk::MMTK, util::side_metadata::SideMetadataSpec};
+use crate::{plan::global::BasePlan, vm::VMBinding};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 
@@ -141,12 +138,8 @@ impl<VM: VMBinding> Plan for SemiSpace<VM> {
         &self.common
     }
 
-    fn global_side_metadata_per_chunk(&self) -> usize {
-        if !VM::VMObjectModel::HAS_GC_BYTE {
-            meta_bytes_per_chunk(3, 1)
-        } else {
-            0
-        }
+    fn global_side_metadata_specs(&self) -> &[SideMetadataSpec] {
+        &self.common().global_metadata_specs
     }
 }
 
@@ -179,7 +172,7 @@ impl<VM: VMBinding> SemiSpace<VM> {
                 mmapper,
                 &mut heap,
             ),
-            common: CommonPlan::new(vm_map, mmapper, options, heap, &SS_CONSTRAINTS),
+            common: CommonPlan::new(vm_map, mmapper, options, heap, &SS_CONSTRAINTS, &[]),
         }
     }
 

src/policy/lockfreeimmortalspace.rs

@@ -92,12 +92,14 @@ impl<VM: VMBinding> Space<VM> for LockFreeImmortalSpace<VM> {
         self.limit = AVAILABLE_START + total_bytes;
         // Eagerly memory map the entire heap (also zero all the memory)
         crate::util::memory::dzmmap(AVAILABLE_START, total_bytes).unwrap();
-        if !try_map_metadata_space(
+        if try_map_metadata_space(
             AVAILABLE_START,
             total_bytes,
-            VM::VMActivePlan::global().global_side_metadata_per_chunk(),
-            self.local_side_metadata_per_chunk(),
-        ) {
+            VM::VMActivePlan::global().global_side_metadata_specs(),
+            self.local_side_metadata_specs(),
+        )
+        .is_err()
+        {
             // TODO(Javad): handle meta space allocation failure
             panic!("failed to mmap meta memory");
         }