Skip to content

Use atomic operations for SFT map and remove unsafe code #931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ repository = "https://github.com/mmtk/mmtk-core"
readme = "README.md"
categories = ["memory-management"]
keywords = ["gc", "garbage", "collection", "garbage-collection", "allocation"]
rust-version = "1.66.0"
rust-version = "1.70.0"
build = "build.rs"

[lib]
Expand Down Expand Up @@ -42,6 +42,7 @@ num_cpus = "1.8"
num-traits = "0.2"
pfm = { version = "0.1.0-beta.3", optional = true }
probe = "0.5"
portable-atomic = "1.4.3"
regex = "1.7.0"
spin = "0.9.5"
static_assertions = "1.1.0"
Expand Down
1 change: 1 addition & 0 deletions src/mmtk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ impl<VM: VMBinding> MMTK<VM> {
pub fn new(options: Arc<Options>) -> Self {
// Initialize SFT first in case we need to use this in the constructor.
// The first call will initialize SFT map. Other calls will be blocked until SFT map is initialized.
crate::policy::sft_map::SFTRefStorage::pre_use_check();
SFT_MAP.initialize_once(&create_sft_map);

let num_workers = if cfg!(feature = "single_worker") {
Expand Down
7 changes: 6 additions & 1 deletion src/plan/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,12 @@ pub fn create_plan<VM: VMBinding>(
}

// Each space now has a fixed address for its lifetime. It is safe now to initialize SFT.
plan.for_each_space(&mut |s| s.initialize_sft());
let sft_map: &mut dyn crate::policy::sft_map::SFTMap =
unsafe { crate::mmtk::SFT_MAP.get_mut() }.as_mut();
plan.for_each_space(&mut |s| {
sft_map.notify_space_creation(s.as_sft());
s.initialize_sft(sft_map);
});

plan
}
Expand Down
4 changes: 2 additions & 2 deletions src/policy/copyspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ impl<VM: VMBinding> Space<VM> for CopySpace<VM> {
&self.common
}

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

fn release_multiple_pages(&mut self, _start: Address) {
Expand Down
5 changes: 3 additions & 2 deletions src/policy/immix/immixspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::plan::VectorObjectQueue;
use crate::policy::gc_work::TraceKind;
use crate::policy::sft::GCWorkerMutRef;
use crate::policy::sft::SFT;
use crate::policy::sft_map::SFTMap;
use crate::policy::space::{CommonSpace, Space};
use crate::util::constants::LOG_BYTES_IN_PAGE;
use crate::util::copy::*;
Expand Down Expand Up @@ -172,8 +173,8 @@ impl<VM: VMBinding> Space<VM> for ImmixSpace<VM> {
fn common(&self) -> &CommonSpace<VM> {
&self.common
}
fn initialize_sft(&self) {
self.common().initialize_sft(self.as_sft())
fn initialize_sft(&self, sft_map: &mut dyn SFTMap) {
self.common().initialize_sft(self.as_sft(), sft_map)
}
fn release_multiple_pages(&mut self, _start: Address) {
panic!("immixspace only releases pages enmasse")
Expand Down
4 changes: 2 additions & 2 deletions src/policy/immortalspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ impl<VM: VMBinding> Space<VM> for ImmortalSpace<VM> {
&self.common
}

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

fn release_multiple_pages(&mut self, _start: Address) {
Expand Down
4 changes: 2 additions & 2 deletions src/policy/largeobjectspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ impl<VM: VMBinding> Space<VM> for LargeObjectSpace<VM> {
&self.pr
}

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

fn common(&self) -> &CommonSpace<VM> {
Expand Down
5 changes: 2 additions & 3 deletions src/policy/lockfreeimmortalspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use atomic::Atomic;
use std::marker::PhantomData;
use std::sync::atomic::Ordering;

use crate::mmtk::SFT_MAP;
use crate::policy::sft::GCWorkerMutRef;
use crate::policy::sft::SFT;
use crate::policy::space::{CommonSpace, Space};
Expand Down Expand Up @@ -104,8 +103,8 @@ impl<VM: VMBinding> Space<VM> for LockFreeImmortalSpace<VM> {
panic!("immortalspace only releases pages enmasse")
}

fn initialize_sft(&self) {
unsafe { SFT_MAP.update(self.as_sft(), self.start, self.extent) };
fn initialize_sft(&self, sft_map: &mut dyn crate::policy::sft_map::SFTMap) {
unsafe { sft_map.eager_initialize(self.as_sft(), self.start, self.extent) };
}

fn reserved_pages(&self) -> usize {
Expand Down
4 changes: 2 additions & 2 deletions src/policy/markcompactspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ impl<VM: VMBinding> Space<VM> for MarkCompactSpace<VM> {
&self.common
}

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

fn release_multiple_pages(&mut self, _start: Address) {
Expand Down
2 changes: 1 addition & 1 deletion src/policy/marksweepspace/malloc_ms/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl<VM: VMBinding> Space<VM> for MallocSpace<VM> {
self.gc_trigger.as_ref()
}

fn initialize_sft(&self) {
fn initialize_sft(&self, _sft_map: &mut dyn crate::policy::sft_map::SFTMap) {
// Do nothing - we will set sft when we get new results from malloc
}

Expand Down
4 changes: 2 additions & 2 deletions src/policy/marksweepspace/native_ms/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ impl<VM: VMBinding> Space<VM> for MarkSweepSpace<VM> {
&self.pr
}

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

fn common(&self) -> &CommonSpace<VM> {
Expand Down
Loading