Skip to content

Commit af2bfe7

Browse files
qinsoonqinsoon
authored
Remove Plan.gc_init() and Space.init() (#629)
This pull request removes `Plan.gc_init()`, and `Space.init()`. I think this can close the issue: #57, I am not aware of any occurrences of two-step initialisation in MMTk after this PR. Note that We eagerly set SFT for each space in their `Space.init()` if they are contiguous and SFT needs a non-moving reference to the `Space`. When we create a `Space`, the reference points to an address on the stack, and will be moved. So we cannot set SFT at creation. I added a method `Space.initialize_sft()` which is called after the `Space` has a fixed address. `initialize_sft()` is only used to set SFT, not any other post-creation initialisation.
1 parent 38b99e8 commit af2bfe7

File tree

24 files changed

+207
-197
lines changed

24 files changed

+207
-197
lines changed

‎docs/tutorial/code/mygc_semispace/global.rs

+8-13
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use crate::util::alloc::allocators::AllocatorSelector;
1414
use crate::util::copy::*;
1515
use crate::util::heap::layout::heap_layout::Mmapper;
1616
use crate::util::heap::layout::heap_layout::VMMap;
17-
use crate::util::heap::layout::vm_layout_constants::{HEAP_END, HEAP_START};
1817
use crate::util::heap::HeapMeta;
1918
use crate::util::heap::VMRequest;
2019
use crate::util::metadata::side_metadata::{SideMetadataSanity, SideMetadataContext};
@@ -79,18 +78,14 @@ impl<VM: VMBinding> Plan for MyGC<VM> {
7978
}
8079
// ANCHOR_END: create_copy_config
8180

82-
// Modify
83-
// ANCHOR: gc_init
84-
fn gc_init(
85-
&mut self,
86-
heap_size: usize,
87-
vm_map: &'static VMMap,
88-
) {
89-
self.common.gc_init(heap_size, vm_map);
90-
self.copyspace0.init(&vm_map);
91-
self.copyspace1.init(&vm_map);
81+
// ANCHOR: get_spaces
82+
fn get_spaces(&self) -> Vec<&dyn Space<Self::VM>> {
83+
let mut ret = self.common.get_spaces();
84+
ret.push(&self.copyspace0);
85+
ret.push(&self.copyspace1);
86+
ret
9287
}
93-
// ANCHOR_END: gc_init
88+
// ANCHOR_EN: get_spaces
9489

9590
// Modify
9691
// ANCHOR: schedule_collection
@@ -182,7 +177,7 @@ impl<VM: VMBinding> MyGC<VM> {
182177
options: Arc<Options>,
183178
) -> Self {
184179
// Modify
185-
let mut heap = HeapMeta::new(HEAP_START, HEAP_END);
180+
let mut heap = HeapMeta::new(&options);
186181
let global_metadata_specs = SideMetadataContext::new_global_specs(&[]);
187182

188183
let res = MyGC {

‎src/lib.rs

-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ mod mmtk;
4848
pub use mmtk::MMTKBuilder;
4949
pub(crate) use mmtk::MMAPPER;
5050
pub use mmtk::MMTK;
51-
pub(crate) use mmtk::VM_MAP;
5251

5352
mod policy;
5453

‎src/mmtk.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,7 @@ impl MMTKBuilder {
6666

6767
/// Build an MMTk instance from the builder.
6868
pub fn build<VM: VMBinding>(&self) -> MMTK<VM> {
69-
let mut mmtk = MMTK::new(Arc::new(self.options.clone()));
70-
71-
let heap_size = *self.options.heap_size;
72-
// TODO: We should remove Plan.gc_init(). We create plan in `MMTKInner::new()`, and we
73-
// should be able move whatever we do in gc_init() to Plan::new().
74-
mmtk.plan.gc_init(heap_size, &crate::VM_MAP);
75-
76-
mmtk
69+
MMTK::new(Arc::new(self.options.clone()))
7770
}
7871
}
7972

@@ -118,6 +111,14 @@ impl<VM: VMBinding> MMTK<VM> {
118111
scheduler.clone(),
119112
);
120113

114+
// TODO: This probably does not work if we have multiple MMTk instances.
115+
VM_MAP.boot();
116+
// This needs to be called after we create Plan. It needs to use HeapMeta, which is gradually built when we create spaces.
117+
VM_MAP.finalize_static_space_map(
118+
plan.base().heap.get_discontig_start(),
119+
plan.base().heap.get_discontig_end(),
120+
);
121+
121122
MMTK {
122123
options,
123124
plan,

‎src/plan/generational/copying/global.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use crate::util::alloc::allocators::AllocatorSelector;
1515
use crate::util::copy::*;
1616
use crate::util::heap::layout::heap_layout::Mmapper;
1717
use crate::util::heap::layout::heap_layout::VMMap;
18-
use crate::util::heap::layout::vm_layout_constants::{HEAP_END, HEAP_START};
1918
use crate::util::heap::HeapMeta;
2019
use crate::util::heap::VMRequest;
2120
use crate::util::metadata::side_metadata::SideMetadataSanity;
@@ -79,10 +78,11 @@ impl<VM: VMBinding> Plan for GenCopy<VM> {
7978
self.gen.last_collection_full_heap()
8079
}
8180

82-
fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap) {
83-
self.gen.gc_init(heap_size, vm_map);
84-
self.copyspace0.init(vm_map);
85-
self.copyspace1.init(vm_map);
81+
fn get_spaces(&self) -> Vec<&dyn Space<Self::VM>> {
82+
let mut ret = self.gen.get_spaces();
83+
ret.push(&self.copyspace0);
84+
ret.push(&self.copyspace1);
85+
ret
8686
}
8787

8888
fn schedule_collection(&'static self, scheduler: &GCWorkScheduler<VM>) {
@@ -167,7 +167,7 @@ impl<VM: VMBinding> Plan for GenCopy<VM> {
167167

168168
impl<VM: VMBinding> GenCopy<VM> {
169169
pub fn new(vm_map: &'static VMMap, mmapper: &'static Mmapper, options: Arc<Options>) -> Self {
170-
let mut heap = HeapMeta::new(HEAP_START, HEAP_END);
170+
let mut heap = HeapMeta::new(&options);
171171
// We have no specific side metadata for copying. So just use the ones from generational.
172172
let global_metadata_specs =
173173
crate::plan::generational::new_generational_global_metadata_specs::<VM>();

‎src/plan/generational/global.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,11 @@ impl<VM: VMBinding> Gen<VM> {
7878
self.nursery.verify_side_metadata_sanity(sanity);
7979
}
8080

81-
/// Initialize Gen. This should be called by the gc_init() API call.
82-
pub fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap) {
83-
self.common.gc_init(heap_size, vm_map);
84-
self.nursery.init(vm_map);
81+
/// Get spaces in generation plans
82+
pub fn get_spaces(&self) -> Vec<&dyn Space<VM>> {
83+
let mut ret = self.common.get_spaces();
84+
ret.push(&self.nursery);
85+
ret
8586
}
8687

8788
/// Prepare Gen. This should be called by a single thread in GC prepare work.

‎src/plan/generational/immix/global.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use crate::util::alloc::allocators::AllocatorSelector;
1515
use crate::util::copy::*;
1616
use crate::util::heap::layout::heap_layout::Mmapper;
1717
use crate::util::heap::layout::heap_layout::VMMap;
18-
use crate::util::heap::layout::vm_layout_constants::{HEAP_END, HEAP_START};
1918
use crate::util::heap::HeapMeta;
2019
use crate::util::options::Options;
2120
use crate::util::VMWorkerThread;
@@ -103,9 +102,10 @@ impl<VM: VMBinding> Plan for GenImmix<VM> {
103102
self.gen.collection_required(self, space_full, space)
104103
}
105104

106-
fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap) {
107-
self.gen.gc_init(heap_size, vm_map);
108-
self.immix.init(vm_map);
105+
fn get_spaces(&self) -> Vec<&dyn Space<Self::VM>> {
106+
let mut ret = self.gen.get_spaces();
107+
ret.push(&self.immix);
108+
ret
109109
}
110110

111111
// GenImmixMatureProcessEdges<VM, { TraceKind::Defrag }> and GenImmixMatureProcessEdges<VM, { TraceKind::Fast }>
@@ -210,7 +210,7 @@ impl<VM: VMBinding> GenImmix<VM> {
210210
options: Arc<Options>,
211211
scheduler: Arc<GCWorkScheduler<VM>>,
212212
) -> Self {
213-
let mut heap = HeapMeta::new(HEAP_START, HEAP_END);
213+
let mut heap = HeapMeta::new(&options);
214214
// We have no specific side metadata for copying. So just use the ones from generational.
215215
let global_metadata_specs =
216216
crate::plan::generational::new_generational_global_metadata_specs::<VM>();

‎src/plan/global.rs

+44-41
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,9 @@ use crate::scheduler::*;
1313
use crate::util::alloc::allocators::AllocatorSelector;
1414
#[cfg(feature = "analysis")]
1515
use crate::util::analysis::AnalysisManager;
16-
use crate::util::conversions::bytes_to_pages;
1716
use crate::util::copy::{CopyConfig, GCWorkerCopyContext};
1817
use crate::util::heap::layout::heap_layout::Mmapper;
1918
use crate::util::heap::layout::heap_layout::VMMap;
20-
use crate::util::heap::layout::map::Map;
2119
use crate::util::heap::HeapMeta;
2220
use crate::util::heap::VMRequest;
2321
use crate::util::metadata::side_metadata::SideMetadataSanity;
@@ -70,30 +68,39 @@ pub fn create_plan<VM: VMBinding>(
7068
options: Arc<Options>,
7169
scheduler: Arc<GCWorkScheduler<VM>>,
7270
) -> Box<dyn Plan<VM = VM>> {
73-
match plan {
74-
PlanSelector::NoGC => Box::new(crate::plan::nogc::NoGC::new(vm_map, mmapper, options)),
71+
let plan = match plan {
72+
PlanSelector::NoGC => Box::new(crate::plan::nogc::NoGC::new(vm_map, mmapper, options))
73+
as Box<dyn Plan<VM = VM>>,
7574
PlanSelector::SemiSpace => Box::new(crate::plan::semispace::SemiSpace::new(
7675
vm_map, mmapper, options,
77-
)),
76+
)) as Box<dyn Plan<VM = VM>>,
7877
PlanSelector::GenCopy => Box::new(crate::plan::generational::copying::GenCopy::new(
7978
vm_map, mmapper, options,
80-
)),
79+
)) as Box<dyn Plan<VM = VM>>,
8180
PlanSelector::GenImmix => Box::new(crate::plan::generational::immix::GenImmix::new(
8281
vm_map, mmapper, options, scheduler,
83-
)),
82+
)) as Box<dyn Plan<VM = VM>>,
8483
PlanSelector::MarkSweep => Box::new(crate::plan::marksweep::MarkSweep::new(
8584
vm_map, mmapper, options,
86-
)),
85+
)) as Box<dyn Plan<VM = VM>>,
8786
PlanSelector::Immix => Box::new(crate::plan::immix::Immix::new(
8887
vm_map, mmapper, options, scheduler,
89-
)),
88+
)) as Box<dyn Plan<VM = VM>>,
9089
PlanSelector::PageProtect => Box::new(crate::plan::pageprotect::PageProtect::new(
9190
vm_map, mmapper, options,
92-
)),
91+
)) as Box<dyn Plan<VM = VM>>,
9392
PlanSelector::MarkCompact => Box::new(crate::plan::markcompact::MarkCompact::new(
9493
vm_map, mmapper, options,
95-
)),
96-
}
94+
)) as Box<dyn Plan<VM = VM>>,
95+
};
96+
97+
// We have created Plan in the heap, and we won't explicitly move it. So each space
98+
// now has a fixed address for its lifetime. It is safe now to initialize SFT.
99+
plan.get_spaces()
100+
.into_iter()
101+
.for_each(|s| s.initialize_sft());
102+
103+
plan
97104
}
98105

99106
/// Create thread local GC worker.
@@ -156,8 +163,8 @@ pub trait Plan: 'static + Sync + Downcast {
156163
&self.base().options
157164
}
158165

159-
// unsafe because this can only be called once by the init thread
160-
fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap);
166+
/// get all the spaces in the plan
167+
fn get_spaces(&self) -> Vec<&dyn Space<Self::VM>>;
161168

162169
fn get_allocator_mapping(&self) -> &'static EnumMap<AllocationSemantics, AllocatorSelector>;
163170

@@ -437,7 +444,7 @@ pub fn create_vm_space<VM: VMBinding>(
437444
use crate::util::heap::layout::vm_layout_constants::BYTES_IN_CHUNK;
438445
let boot_segment_mb = raw_align_up(boot_segment_bytes, BYTES_IN_CHUNK) >> LOG_BYTES_IN_MBYTE;
439446

440-
ImmortalSpace::new(
447+
let space = ImmortalSpace::new(
441448
"boot",
442449
false,
443450
VMRequest::fixed_size(boot_segment_mb),
@@ -446,7 +453,12 @@ pub fn create_vm_space<VM: VMBinding>(
446453
mmapper,
447454
heap,
448455
constraints,
449-
)
456+
);
457+
458+
// The space is mapped externally by the VM. We need to update our mmapper to mark the range as mapped.
459+
space.ensure_mapped();
460+
461+
space
450462
}
451463

452464
impl<VM: VMBinding> BasePlan<VM> {
@@ -539,27 +551,17 @@ impl<VM: VMBinding> BasePlan<VM> {
539551
}
540552
}
541553

542-
pub fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap) {
543-
vm_map.boot();
544-
vm_map.finalize_static_space_map(
545-
self.heap.get_discontig_start(),
546-
self.heap.get_discontig_end(),
547-
);
548-
self.heap
549-
.total_pages
550-
.store(bytes_to_pages(heap_size), Ordering::Relaxed);
551-
552-
#[cfg(feature = "code_space")]
553-
self.code_space.init(vm_map);
554-
#[cfg(feature = "code_space")]
555-
self.code_lo_space.init(vm_map);
556-
#[cfg(feature = "ro_space")]
557-
self.ro_space.init(vm_map);
558-
#[cfg(feature = "vm_space")]
559-
{
560-
self.vm_space.init(vm_map);
561-
self.vm_space.ensure_mapped();
562-
}
554+
pub fn get_spaces(&self) -> Vec<&dyn Space<VM>> {
555+
vec![
556+
#[cfg(feature = "code_space")]
557+
&self.code_space,
558+
#[cfg(feature = "code_space")]
559+
&self.code_lo_space,
560+
#[cfg(feature = "ro_space")]
561+
&self.ro_space,
562+
#[cfg(feature = "vm_space")]
563+
&self.vm_space,
564+
]
563565
}
564566

565567
/// The application code has requested a collection.
@@ -927,10 +929,11 @@ impl<VM: VMBinding> CommonPlan<VM> {
927929
}
928930
}
929931

930-
pub fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap) {
931-
self.base.gc_init(heap_size, vm_map);
932-
self.immortal.init(vm_map);
933-
self.los.init(vm_map);
932+
pub fn get_spaces(&self) -> Vec<&dyn Space<VM>> {
933+
let mut ret = self.base.get_spaces();
934+
ret.push(&self.immortal);
935+
ret.push(&self.los);
936+
ret
934937
}
935938

936939
pub fn get_used_pages(&self) -> usize {

‎src/plan/immix/global.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use crate::util::alloc::allocators::AllocatorSelector;
1313
use crate::util::copy::*;
1414
use crate::util::heap::layout::heap_layout::Mmapper;
1515
use crate::util::heap::layout::heap_layout::VMMap;
16-
use crate::util::heap::layout::vm_layout_constants::{HEAP_END, HEAP_START};
1716
use crate::util::heap::HeapMeta;
1817
use crate::util::metadata::side_metadata::SideMetadataContext;
1918
use crate::util::metadata::side_metadata::SideMetadataSanity;
@@ -75,9 +74,10 @@ impl<VM: VMBinding> Plan for Immix<VM> {
7574
}
7675
}
7776

78-
fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap) {
79-
self.common.gc_init(heap_size, vm_map);
80-
self.immix_space.init(vm_map);
77+
fn get_spaces(&self) -> Vec<&dyn Space<Self::VM>> {
78+
let mut ret = self.common.get_spaces();
79+
ret.push(&self.immix_space);
80+
ret
8181
}
8282

8383
fn schedule_collection(&'static self, scheduler: &GCWorkScheduler<VM>) {
@@ -140,7 +140,7 @@ impl<VM: VMBinding> Immix<VM> {
140140
options: Arc<Options>,
141141
scheduler: Arc<GCWorkScheduler<VM>>,
142142
) -> Self {
143-
let mut heap = HeapMeta::new(HEAP_START, HEAP_END);
143+
let mut heap = HeapMeta::new(&options);
144144
let global_metadata_specs = SideMetadataContext::new_global_specs(&[]);
145145
let immix = Immix {
146146
immix_space: ImmixSpace::new(

‎src/plan/markcompact/global.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ use crate::util::alloc_bit::ALLOC_SIDE_METADATA_SPEC;
2020
use crate::util::copy::CopySemantics;
2121
use crate::util::heap::layout::heap_layout::Mmapper;
2222
use crate::util::heap::layout::heap_layout::VMMap;
23-
use crate::util::heap::layout::vm_layout_constants::{HEAP_END, HEAP_START};
2423
use crate::util::heap::HeapMeta;
2524
use crate::util::heap::VMRequest;
2625
use crate::util::metadata::side_metadata::{SideMetadataContext, SideMetadataSanity};
@@ -57,9 +56,10 @@ impl<VM: VMBinding> Plan for MarkCompact<VM> {
5756
&MARKCOMPACT_CONSTRAINTS
5857
}
5958

60-
fn gc_init(&mut self, heap_size: usize, vm_map: &'static VMMap) {
61-
self.common.gc_init(heap_size, vm_map);
62-
self.mc_space.init(vm_map);
59+
fn get_spaces(&self) -> Vec<&dyn Space<Self::VM>> {
60+
let mut ret = self.common.get_spaces();
61+
ret.push(&self.mc_space);
62+
ret
6363
}
6464

6565
fn base(&self) -> &BasePlan<VM> {
@@ -178,7 +178,7 @@ impl<VM: VMBinding> Plan for MarkCompact<VM> {
178178

179179
impl<VM: VMBinding> MarkCompact<VM> {
180180
pub fn new(vm_map: &'static VMMap, mmapper: &'static Mmapper, options: Arc<Options>) -> Self {
181-
let mut heap = HeapMeta::new(HEAP_START, HEAP_END);
181+
let mut heap = HeapMeta::new(&options);
182182
// if global_alloc_bit is enabled, ALLOC_SIDE_METADATA_SPEC will be added to
183183
// SideMetadataContext by default, so we don't need to add it here.
184184
#[cfg(feature = "global_alloc_bit")]

0 commit comments

Comments
 (0)