Two step space creation

docs/userguide/src/tutorial/code/mygc_semispace/global.rs

@@ -2,8 +2,8 @@
 use crate::plan::global::BasePlan; //Modify
 use crate::plan::global::CommonPlan; // Add
 use crate::plan::global::{CreateGeneralPlanArgs, CreateSpecificPlanArgs};
-use crate::plan::mygc::mutator::ALLOCATOR_MAPPING;
 use crate::plan::mygc::gc_work::MyGCWorkContext;
+use crate::plan::mygc::mutator::ALLOCATOR_MAPPING;
 use crate::plan::AllocationSemantics;
 use crate::plan::Plan;
 use crate::plan::PlanConstraints;
@@ -12,13 +12,13 @@ use crate::policy::space::Space;
 use crate::scheduler::*; // Modify
 use crate::util::alloc::allocators::AllocatorSelector;
 use crate::util::copy::*;
-use crate::util::heap::VMRequest;
+use crate::util::heap::heap_meta::VMRequest;
 use crate::util::metadata::side_metadata::SideMetadataContext;
 use crate::util::opaque_pointer::*;
 use crate::vm::VMBinding;
 use enum_map::EnumMap;
 use std::sync::atomic::{AtomicBool, Ordering}; // Add
-// ANCHOR_END: imports_no_gc_work
+                                               // ANCHOR_END: imports_no_gc_work
 
 // Remove #[allow(unused_imports)].
 // Remove handle_user_collection_request().
@@ -66,7 +66,7 @@ impl<VM: VMBinding> Plan for MyGC<VM> {
             },
             space_mapping: vec![
                 // The tospace argument doesn't matter, we will rebind before a GC anyway.
-                (CopySelector::CopySpace(0), &self.copyspace0)
+                (CopySelector::CopySpace(0), &self.copyspace0),
             ],
             constraints: &MYGC_CONSTRAINTS,
         }
@@ -167,13 +167,35 @@ impl<VM: VMBinding> MyGC<VM> {
             global_side_metadata_specs: SideMetadataContext::new_global_specs(&[]),
         };
 
+        // ANCHOR: specify_spaces
+        let copyspace0_resp = plan_args
+            .global_args
+            .heap
+            .specify_space(VMRequest::Unrestricted);
+        let copyspace1_resp = plan_args
+            .global_args
+            .heap
+            .specify_space(VMRequest::Unrestricted);
+        // ANCHOR_END: specify_spaces
+
+        // ANCHOR: create_common_plan
+        // Spaces will eventually be placed by `BasePlan`.
+        let common = CommonPlan::new(&mut plan_args);
+        // ANCHOR_END: create_common_plan
+
         let res = MyGC {
             hi: AtomicBool::new(false),
-            // ANCHOR: copyspace_new
-            copyspace0: CopySpace::new(plan_args.get_space_args("copyspace0", true, VMRequest::discontiguous()), false),
-            // ANCHOR_END: copyspace_new
-            copyspace1: CopySpace::new(plan_args.get_space_args("copyspace1", true, VMRequest::discontiguous()), true),
-            common: CommonPlan::new(plan_args),
+            // ANCHOR: copyspaces_new
+            copyspace0: CopySpace::new(
+                plan_args.get_space_args("copyspace0", true, copyspace0_resp.unwrap()),
+                false,
+            ),
+            copyspace1: CopySpace::new(
+                plan_args.get_space_args("copyspace1", true, copyspace1_resp.unwrap()),
+                true,
+            ),
+            // ANCHOR_END: copyspaces_new
+            common,
         };
 
         res.verify_side_metadata_sanity();

docs/userguide/src/tutorial/mygc/ss/alloc.md

@@ -82,24 +82,49 @@ use mmtk_macros::{HasSpaces, PlanTraceObject};
 
 #### Constructor
 
-Change `fn new()`. This section initialises and prepares the objects in MyGC 
+Change `fn new()`. This section initialises and prepares the objects in MyGC
 that you just defined.
 
-   1. Delete the definition of `mygc_space`. 
-   Instead, we will define the two copyspaces here.
-   2. Define one of the copyspaces by adding the following code: 
 ```rust
-{{#include ../../code/mygc_semispace/global.rs:copyspace_new}}
+{{#include ../../code/mygc_semispace/global.rs:plan_new}}
 ```
 
-   3. Create another copyspace, called `copyspace1`, defining it as a fromspace 
-   instead of a tospace. (Hint: the definitions for 
-   copyspaces are in `src/policy/copyspace.rs`.) 
-   4. Finally, replace the old MyGC initializer.
+We now look into the steps.
+
+First, we use the `HeapMeta` object provided from the argument to specify all
+spaces that will be created in the current plan.
+
 ```rust
-{{#include ../../code/mygc_semispace/global.rs:plan_new}}
+{{#include ../../code/mygc_semispace/global.rs:specify_spaces}}
 ```
 
+We do not have special requirements for either of the copy-spaces, so we just
+specify `VMRequest::Unrestricted` here.  At this step, the return values
+`copyspace0_resp` and `copyspace1_resp` has not become usable, yet.
+
+Then, we construct the parent structure `CommonPlan::new()`.
+
+```rust
+{{#include ../../code/mygc_semispace/global.rs:create_common_plan}}
+```
+
+`CommonPlan::new()` will call `BasePlan::new()` which, in turn, will call
+`HeapMeta::place_spaces()`.  That will determine the address range of all
+spaces we specified.
+
+After this, we can call `copyspace0_resp.unwrap()` to retrieve the computed
+placement information for creating `copyspace0`.  `copyspace1` is similar.  We
+can now create the two `CopySpace` instances.
+
+```rust
+{{#include ../../code/mygc_semispace/global.rs:copyspaces_new}}
+```
+
+Note that `CommonSpace` and `BaseSpace` also define other spaces, such as the
+large object space.  Their constructors specify their spaces before
+determining their address ranges and instantiating them, just like we
+discribed here.
+
 ### Access MyGC spaces
 
 Add a new section of methods for MyGC:

src/mmtk.rs

@@ -12,15 +12,15 @@ use crate::util::analysis::AnalysisManager;
 use crate::util::edge_logger::EdgeLogger;
 use crate::util::finalizable_processor::FinalizableProcessor;
 use crate::util::heap::gc_trigger::GCTrigger;
+use crate::util::heap::heap_meta::HeapMeta;
 use crate::util::heap::layout::vm_layout::VMLayout;
 use crate::util::heap::layout::{self, Mmapper, VMMap};
-use crate::util::heap::HeapMeta;
-use crate::util::opaque_pointer::*;
 use crate::util::options::Options;
 use crate::util::reference_processor::ReferenceProcessors;
 #[cfg(feature = "sanity")]
 use crate::util::sanity::sanity_checker::SanityChecker;
 use crate::util::statistics::stats::Stats;
+use crate::util::{opaque_pointer::*, Address};
 use crate::vm::ReferenceGlue;
 use crate::vm::VMBinding;
 use std::cell::UnsafeCell;
@@ -188,8 +188,14 @@ impl<VM: VMBinding> MMTK<VM> {
 
         // TODO: This probably does not work if we have multiple MMTk instances.
         VM_MAP.boot();
-        // This needs to be called after we create Plan. It needs to use HeapMeta, which is gradually built when we create spaces.
-        VM_MAP.finalize_static_space_map(heap.get_discontig_start(), heap.get_discontig_end());
+
+        // `Map32` uses `finalize_static_space_map` this to initialize the global freelists, which is reasonable.
+        // `Map64` uses `finalize_static_space_map` this to fix the starting addresses of `RawMemoryFreeList` instances, which is a bug and should be fixed.
+        // Since `Map64` doesn't read the start and end of the discontiguous range in the function at all, we can leave them as zeroes.
+        let discontig_range = heap
+            .get_discontiguous_range()
+            .unwrap_or(Address::ZERO..Address::ZERO);
+        VM_MAP.finalize_static_space_map(discontig_range.start, discontig_range.end);
 
         if *options.transparent_hugepages {
             MMAPPER.set_mmap_strategy(crate::util::memory::MmapStrategy::TransparentHugePages);

src/plan/generational/copying/global.rs

@@ -16,7 +16,7 @@ use crate::policy::space::Space;
 use crate::scheduler::*;
 use crate::util::alloc::allocators::AllocatorSelector;
 use crate::util::copy::*;
-use crate::util::heap::VMRequest;
+use crate::util::heap::heap_meta::VMRequest;
 use crate::util::Address;
 use crate::util::ObjectReference;
 use crate::util::VMWorkerThread;
@@ -200,17 +200,29 @@ impl<VM: VMBinding> GenCopy<VM> {
                 crate::plan::generational::new_generational_global_metadata_specs::<VM>(),
         };
 
+        let copyspace0_resp = plan_args
+            .global_args
+            .heap
+            .specify_space(VMRequest::Unrestricted);
+        let copyspace1_resp = plan_args
+            .global_args
+            .heap
+            .specify_space(VMRequest::Unrestricted);
+
+        // Spaces will eventually be placed by `BasePlan`.
+        let gen = CommonGenPlan::new(&mut plan_args);
+
         let copyspace0 = CopySpace::new(
-            plan_args.get_space_args("copyspace0", true, VMRequest::discontiguous()),
+            plan_args.get_space_args("copyspace0", true, copyspace0_resp.unwrap()),
             false,
         );
         let copyspace1 = CopySpace::new(
-            plan_args.get_space_args("copyspace1", true, VMRequest::discontiguous()),
+            plan_args.get_space_args("copyspace1", true, copyspace1_resp.unwrap()),
             true,
         );
 
         let res = GenCopy {
-            gen: CommonGenPlan::new(plan_args),
+            gen,
             hi: AtomicBool::new(false),
             copyspace0,
             copyspace1,

src/plan/generational/global.rs

@@ -6,7 +6,7 @@ use crate::policy::copyspace::CopySpace;
 use crate::policy::space::Space;
 use crate::scheduler::*;
 use crate::util::copy::CopySemantics;
-use crate::util::heap::VMRequest;
+use crate::util::heap::heap_meta::VMRequest;
 use crate::util::statistics::counter::EventCounter;
 use crate::util::Address;
 use crate::util::ObjectReference;
@@ -37,20 +37,24 @@ pub struct CommonGenPlan<VM: VMBinding> {
 }
 
 impl<VM: VMBinding> CommonGenPlan<VM> {
-    pub fn new(mut args: CreateSpecificPlanArgs<VM>) -> Self {
+    pub fn new(args: &mut CreateSpecificPlanArgs<VM>) -> Self {
+        let nursery_resp = args.global_args.heap.specify_space(VMRequest::Extent {
+            extent: args.global_args.options.get_max_nursery_bytes(),
+            top: false,
+        });
+
+        // Spaces will eventually be placed by `BasePlan`.
+        let common = CommonPlan::new(args);
+
         let nursery = CopySpace::new(
-            args.get_space_args(
-                "nursery",
-                true,
-                VMRequest::fixed_extent(args.global_args.options.get_max_nursery_bytes(), false),
-            ),
+            args.get_space_args("nursery", true, nursery_resp.unwrap()),
             true,
         );
+
         let full_heap_gc_count = args
             .global_args
             .stats
             .new_event_counter("majorGC", true, true);
-        let common = CommonPlan::new(args);
 
         CommonGenPlan {
             nursery,

src/plan/generational/immix/global.rs

@@ -17,7 +17,7 @@ use crate::scheduler::GCWorkScheduler;
 use crate::scheduler::GCWorker;
 use crate::util::alloc::allocators::AllocatorSelector;
 use crate::util::copy::*;
-use crate::util::heap::VMRequest;
+use crate::util::heap::heap_meta::VMRequest;
 use crate::util::Address;
 use crate::util::ObjectReference;
 use crate::util::VMWorkerThread;
@@ -231,8 +231,17 @@ impl<VM: VMBinding> GenImmix<VM> {
             global_side_metadata_specs:
                 crate::plan::generational::new_generational_global_metadata_specs::<VM>(),
         };
+
+        let immix_space_resp = plan_args
+            .global_args
+            .heap
+            .specify_space(VMRequest::Unrestricted);
+
+        // Spaces will eventually be placed by `BasePlan`.
+        let gen = CommonGenPlan::new(&mut plan_args);
+
         let immix_space = ImmixSpace::new(
-            plan_args.get_space_args("immix_mature", true, VMRequest::discontiguous()),
+            plan_args.get_space_args("immix_mature", true, immix_space_resp.unwrap()),
             ImmixSpaceArgs {
                 reset_log_bit_in_major_gc: false,
                 // We don't need to unlog objects at tracing. Instead, we unlog objects at copying.
@@ -244,7 +253,7 @@ impl<VM: VMBinding> GenImmix<VM> {
         );
 
         let genimmix = GenImmix {
-            gen: CommonGenPlan::new(plan_args),
+            gen,
             immix_space,
             last_gc_was_defrag: AtomicBool::new(false),
             last_gc_was_full_heap: AtomicBool::new(false),