Skip to content

Commit 698a737

Browse files
authored
Remove fl_map and refactor FreeListPageResoure (#953)
This PR removes the `shared_fl_map` field from `Map32`, and removes `fl_map` and `fl_page_resource` fields from `Map64`. These global maps were used to enumerate and update the starting address of spaces (stored in `CommonFreeListPageResource` instances), for several different purposes. 1. `Map32` can only decide the starting address of discontiguous spaces after all contiguous spaces are placed, therefore it needs to modify the start address of all discontiguous `FreeListPageResource` instances. - `shared_fl_map` was used to locate `CommonFreeListPageResource` instances by the space ordinal. - With this PR, we use `Plan::for_each_space_mut` to enumerate spaces, and use the newly added `Space::maybe_get_page_resource_mut` method to get the page resource (if it has one) in order to update their starting addresses. 2. `Map64` allocates `RawMemoryFreeList` in the beginning of each space that uses `FreeListPageResource`. It needs to update the start address of each space to take the address range occupied by `RawMemoryFreeList` into account. - `fl_page_resource` was used to locate `CommonFreeListPageResource` instances by the space ordinal. - `fl_map` was used to locate the underlying `RawMemoryFreeList` instances of the corresponding `FreeListPageResource` by the space ordinal. - With this PR, we initialize the start address of the `FreeListPageResource` immediately after a `RawMemoryFreeList` is created, taking the limit of `RawMemoryFreeList` into account. Therefore, it is no longer needed to update the starting address later. Because those global maps are removed, several changes are made to `VMMap` and its implementations `Map32` and `Map64`. - The `VMMap::boot` and the `VMMap::bind_freelist` no longer serve any purpose, and are removed. - `Map64::finalize_static_space_map` now does nothing but setting `finalized` to true. The `CommonFreeListPageResource` data structure was introduced for the sole purpose to be referenced by those global maps. This PR removes `CommonFreeListPageResource` and `FreeListPageResourceInner`, and relocates their fields. - `common: CommonPageResource` is now a field of `FreeListPageResource`. - The `free_list: Box<dyn FreeList>` and `start: Address` are moved into `FreeListPageResourceSync` because they have always been accessed while holding the mutex `FreeListPageResource::sync`. - Note that the method `FreeListPageResource::release_pages` used to call `free_list.size()` without holding the `sync` mutex, and subsequently calls `free_list.free()` with the `sync` mutex. The algorithm of `FreeList` guarantees `free()` will not race with `size()`. But the `Mutex` forbids calling the read-only operation `free()` without holding the lock because in general it is possible that a read-write operations may race with read-only operations, too. For now, we extended the range of the mutex to cover the whole function. After the changes above, the `FreeListPageResource` no longer needs `UnsafeCell`. This PR is one step of the greater goal of removing unsafe operations related to FreeList, PageResource and VMMap. See: #853 Related PRs: - #925: This PR is a subset of that PR.
1 parent 5f955bc commit 698a737

19 files changed

+268
-247
lines changed

src/mmtk.rs

+14-3
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ impl<VM: VMBinding> MMTK<VM> {
162162
// So we do not save it in MMTK. This may change in the future.
163163
let mut heap = HeapMeta::new();
164164

165-
let plan = crate::plan::create_plan(
165+
let mut plan = crate::plan::create_plan(
166166
*options.plan,
167167
CreateGeneralPlanArgs {
168168
vm_map: VM_MAP.as_ref(),
@@ -188,9 +188,20 @@ impl<VM: VMBinding> MMTK<VM> {
188188
}
189189

190190
// TODO: This probably does not work if we have multiple MMTk instances.
191-
VM_MAP.boot();
192191
// This needs to be called after we create Plan. It needs to use HeapMeta, which is gradually built when we create spaces.
193-
VM_MAP.finalize_static_space_map(heap.get_discontig_start(), heap.get_discontig_end());
192+
VM_MAP.finalize_static_space_map(
193+
heap.get_discontig_start(),
194+
heap.get_discontig_end(),
195+
&mut |start_address| {
196+
plan.for_each_space_mut(&mut |space| {
197+
// If the `VMMap` has a discontiguous memory range, we notify all discontiguous
198+
// space that the starting address has been determined.
199+
if let Some(pr) = space.maybe_get_page_resource_mut() {
200+
pr.update_discontiguous_start(start_address);
201+
}
202+
})
203+
},
204+
);
194205

195206
if *options.transparent_hugepages {
196207
MMAPPER.set_mmap_strategy(crate::util::memory::MmapStrategy::TransparentHugePages);

src/policy/copyspace.rs

+4
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ impl<VM: VMBinding> Space<VM> for CopySpace<VM> {
102102
&self.pr
103103
}
104104

105+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
106+
Some(&mut self.pr)
107+
}
108+
105109
fn common(&self) -> &CommonSpace<VM> {
106110
&self.common
107111
}

src/policy/immix/immixspace.rs

+3
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ impl<VM: VMBinding> Space<VM> for ImmixSpace<VM> {
162162
fn get_page_resource(&self) -> &dyn PageResource<VM> {
163163
&self.pr
164164
}
165+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
166+
Some(&mut self.pr)
167+
}
165168
fn common(&self) -> &CommonSpace<VM> {
166169
&self.common
167170
}

src/policy/immortalspace.rs

+3
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ impl<VM: VMBinding> Space<VM> for ImmortalSpace<VM> {
8787
fn get_page_resource(&self) -> &dyn PageResource<VM> {
8888
&self.pr
8989
}
90+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
91+
Some(&mut self.pr)
92+
}
9093
fn common(&self) -> &CommonSpace<VM> {
9194
&self.common
9295
}

src/policy/largeobjectspace.rs

+3
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ impl<VM: VMBinding> Space<VM> for LargeObjectSpace<VM> {
106106
fn get_page_resource(&self) -> &dyn PageResource<VM> {
107107
&self.pr
108108
}
109+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
110+
Some(&mut self.pr)
111+
}
109112

110113
fn initialize_sft(&self, sft_map: &mut dyn crate::policy::sft_map::SFTMap) {
111114
self.common().initialize_sft(self.as_sft(), sft_map)

src/policy/lockfreeimmortalspace.rs

+3
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ impl<VM: VMBinding> Space<VM> for LockFreeImmortalSpace<VM> {
9797
fn get_page_resource(&self) -> &dyn PageResource<VM> {
9898
unimplemented!()
9999
}
100+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
101+
None
102+
}
100103
fn common(&self) -> &CommonSpace<VM> {
101104
unimplemented!()
102105
}

src/policy/markcompactspace.rs

+4
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ impl<VM: VMBinding> Space<VM> for MarkCompactSpace<VM> {
104104
&self.pr
105105
}
106106

107+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
108+
Some(&mut self.pr)
109+
}
110+
107111
fn common(&self) -> &CommonSpace<VM> {
108112
&self.common
109113
}

src/policy/marksweepspace/malloc_ms/global.rs

+4
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ impl<VM: VMBinding> Space<VM> for MallocSpace<VM> {
137137
unreachable!()
138138
}
139139

140+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
141+
None
142+
}
143+
140144
fn common(&self) -> &CommonSpace<VM> {
141145
unreachable!()
142146
}

src/policy/marksweepspace/native_ms/global.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
scheduler::{GCWorkScheduler, GCWorker},
88
util::{
99
copy::CopySemantics,
10-
heap::FreeListPageResource,
10+
heap::{FreeListPageResource, PageResource},
1111
metadata::{self, side_metadata::SideMetadataSpec, MetadataSpec},
1212
ObjectReference,
1313
},
@@ -209,6 +209,10 @@ impl<VM: VMBinding> Space<VM> for MarkSweepSpace<VM> {
209209
&self.pr
210210
}
211211

212+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
213+
Some(&mut self.pr)
214+
}
215+
212216
fn initialize_sft(&self, sft_map: &mut dyn crate::policy::sft_map::SFTMap) {
213217
self.common().initialize_sft(self.as_sft(), sft_map)
214218
}

src/policy/space.rs

+4
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
4242
fn as_sft(&self) -> &(dyn SFT + Sync + 'static);
4343
fn get_page_resource(&self) -> &dyn PageResource<VM>;
4444

45+
/// Get a mutable reference to the underlying page resource, or `None` if the space does not
46+
/// have a page resource.
47+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>>;
48+
4549
/// Initialize entires in SFT map for the space. This is called when the Space object
4650
/// has a non-moving address, as we will use the address to set sft.
4751
/// Currently after we create a boxed plan, spaces in the plan have a non-moving address.

src/policy/vmspace.rs

+3
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ impl<VM: VMBinding> Space<VM> for VMSpace<VM> {
8787
fn get_page_resource(&self) -> &dyn PageResource<VM> {
8888
&self.pr
8989
}
90+
fn maybe_get_page_resource_mut(&mut self) -> Option<&mut dyn PageResource<VM>> {
91+
Some(&mut self.pr)
92+
}
9093
fn common(&self) -> &CommonSpace<VM> {
9194
&self.common
9295
}

src/util/heap/blockpageresource.rs

+4
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ impl<VM: VMBinding, B: Region> PageResource<VM> for BlockPageResource<VM, B> {
3636
self.flpr.common_mut()
3737
}
3838

39+
fn update_discontiguous_start(&mut self, start: Address) {
40+
self.flpr.update_discontiguous_start(start)
41+
}
42+
3943
fn alloc_pages(
4044
&self,
4145
space_descriptor: SpaceDescriptor,

0 commit comments

Comments
 (0)