Skip to content
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

Remove cast ref to mut everywhere #893

Merged
merged 12 commits into from
Aug 30, 2023
2 changes: 1 addition & 1 deletion .github/scripts/ci-style.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
. $(dirname "$0")/ci-common.sh

export RUSTFLAGS="-D warnings"
export RUSTFLAGS="-D warnings -A unknown-lints"
qinsoon marked this conversation as resolved.
Show resolved Hide resolved

# check base
cargo clippy
Expand Down
37 changes: 23 additions & 14 deletions src/memory_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,12 @@ pub fn mmtk_init<VM: VMBinding>(builder: &MMTKBuilder) -> Box<MMTK<VM>> {

#[cfg(feature = "vm_space")]
pub fn lazy_init_vm_space<VM: VMBinding>(mmtk: &'static mut MMTK<VM>, start: Address, size: usize) {
mmtk.plan.base_mut().vm_space.lazy_initialize(start, size);
unsafe {
mmtk.get_plan_mut()
.base_mut()
.vm_space
.lazy_initialize(start, size);
}
}

/// Request MMTk to create a mutator for the given thread. The ownership
Expand Down Expand Up @@ -347,7 +352,7 @@ pub fn get_allocator_mapping<VM: VMBinding>(
mmtk: &MMTK<VM>,
semantics: AllocationSemantics,
) -> AllocatorSelector {
mmtk.plan.get_allocator_mapping()[semantics]
mmtk.get_plan().get_allocator_mapping()[semantics]
}

/// The standard malloc. MMTk either uses its own allocator, or forward the call to a
Expand Down Expand Up @@ -469,11 +474,14 @@ pub fn start_worker<VM: VMBinding>(
/// Collection::spawn_gc_thread() so that the VM knows the context.
pub fn initialize_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThread) {
assert!(
!mmtk.plan.is_initialized(),
!mmtk.get_plan().is_initialized(),
"MMTk collection has been initialized (was initialize_collection() already called before?)"
);
mmtk.scheduler.spawn_gc_threads(mmtk, tls);
mmtk.plan.base().initialized.store(true, Ordering::SeqCst);
mmtk.get_plan()
.base()
.initialized
.store(true, Ordering::SeqCst);
probe!(mmtk, collection_initialized);
}

Expand All @@ -485,10 +493,10 @@ pub fn initialize_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThre
/// * `mmtk`: A reference to an MMTk instance.
pub fn enable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
debug_assert!(
!mmtk.plan.should_trigger_gc_when_heap_is_full(),
!mmtk.get_plan().should_trigger_gc_when_heap_is_full(),
"enable_collection() is called when GC is already enabled."
);
mmtk.plan
mmtk.get_plan()
.base()
.trigger_gc_when_heap_is_full
.store(true, Ordering::SeqCst);
Expand All @@ -506,10 +514,10 @@ pub fn enable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
/// * `mmtk`: A reference to an MMTk instance.
pub fn disable_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>) {
debug_assert!(
mmtk.plan.should_trigger_gc_when_heap_is_full(),
mmtk.get_plan().should_trigger_gc_when_heap_is_full(),
"disable_collection() is called when GC is not enabled."
);
mmtk.plan
mmtk.get_plan()
.base()
.trigger_gc_when_heap_is_full
.store(false, Ordering::SeqCst);
Expand Down Expand Up @@ -540,7 +548,7 @@ pub fn process_bulk(builder: &mut MMTKBuilder, options: &str) -> bool {
/// Arguments:
/// * `mmtk`: A reference to an MMTk instance.
pub fn used_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
mmtk.plan.get_used_pages() << LOG_BYTES_IN_PAGE
mmtk.get_plan().get_used_pages() << LOG_BYTES_IN_PAGE
}

/// Return free memory in bytes. MMTk accounts for memory in pages, thus this method always returns a value in
Expand All @@ -549,7 +557,7 @@ pub fn used_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
/// Arguments:
/// * `mmtk`: A reference to an MMTk instance.
pub fn free_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
mmtk.plan.get_free_pages() << LOG_BYTES_IN_PAGE
mmtk.get_plan().get_free_pages() << LOG_BYTES_IN_PAGE
}

/// Return the size of all the live objects in bytes in the last GC. MMTk usually accounts for memory in pages.
Expand All @@ -560,7 +568,7 @@ pub fn free_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
/// to call this method is at the end of a GC (e.g. when the runtime is about to resume threads).
#[cfg(feature = "count_live_bytes_in_gc")]
pub fn live_bytes_in_last_gc<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
mmtk.plan
mmtk.get_plan()
.base()
.live_bytes_in_last_gc
.load(Ordering::SeqCst)
Expand All @@ -583,7 +591,7 @@ pub fn last_heap_address() -> Address {
/// Arguments:
/// * `mmtk`: A reference to an MMTk instance.
pub fn total_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
mmtk.plan.get_total_pages() << LOG_BYTES_IN_PAGE
mmtk.get_plan().get_total_pages() << LOG_BYTES_IN_PAGE
}

/// Trigger a garbage collection as requested by the user.
Expand All @@ -592,7 +600,8 @@ pub fn total_bytes<VM: VMBinding>(mmtk: &MMTK<VM>) -> usize {
/// * `mmtk`: A reference to an MMTk instance.
/// * `tls`: The thread that triggers this collection request.
pub fn handle_user_collection_request<VM: VMBinding>(mmtk: &MMTK<VM>, tls: VMMutatorThread) {
mmtk.plan.handle_user_collection_request(tls, false, false);
mmtk.get_plan()
.handle_user_collection_request(tls, false, false);
}

/// Is the object alive?
Expand Down Expand Up @@ -711,7 +720,7 @@ pub fn is_mapped_address(address: Address) -> bool {
/// * `mmtk`: A reference to an MMTk instance.
/// * `object`: The object to check.
pub fn modify_check<VM: VMBinding>(mmtk: &MMTK<VM>, object: ObjectReference) {
mmtk.plan.modify_check(object);
mmtk.get_plan().modify_check(object);
}

/// Add a reference to the list of weak references. A binding may
Expand Down
31 changes: 23 additions & 8 deletions src/mmtk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::util::reference_processor::ReferenceProcessors;
use crate::util::sanity::sanity_checker::SanityChecker;
use crate::vm::ReferenceGlue;
use crate::vm::VMBinding;
use std::cell::UnsafeCell;
use std::default::Default;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
Expand All @@ -29,10 +30,10 @@ lazy_static! {
// TODO: We should refactor this when we know more about how multiple MMTK instances work.

/// A global VMMap that manages the mapping of spaces to virtual memory ranges.
pub static ref VM_MAP: Box<dyn VMMap> = layout::create_vm_map();
pub static ref VM_MAP: Box<dyn VMMap + Send + Sync> = layout::create_vm_map();

/// A global Mmapper for mmaping and protection of virtual memory.
pub static ref MMAPPER: Box<dyn Mmapper> = layout::create_mmapper();
pub static ref MMAPPER: Box<dyn Mmapper + Send + Sync> = layout::create_mmapper();
}

use crate::util::rust_util::InitializeOnce;
Expand Down Expand Up @@ -81,7 +82,7 @@ impl Default for MMTKBuilder {
/// *Note that multi-instances is not fully supported yet*
pub struct MMTK<VM: VMBinding> {
pub(crate) options: Arc<Options>,
pub(crate) plan: Box<dyn Plan<VM = VM>>,
pub(crate) plan: UnsafeCell<Box<dyn Plan<VM = VM>>>,
pub(crate) reference_processors: ReferenceProcessors,
pub(crate) finalizable_processor:
Mutex<FinalizableProcessor<<VM::VMReferenceGlue as ReferenceGlue<VM>>::FinalizableType>>,
Expand All @@ -93,6 +94,9 @@ pub struct MMTK<VM: VMBinding> {
inside_harness: AtomicBool,
}

unsafe impl<VM: VMBinding> Sync for MMTK<VM> {}
unsafe impl<VM: VMBinding> Send for MMTK<VM> {}

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.
Expand Down Expand Up @@ -125,7 +129,7 @@ impl<VM: VMBinding> MMTK<VM> {

MMTK {
options,
plan,
plan: UnsafeCell::new(plan),
reference_processors: ReferenceProcessors::new(),
finalizable_processor: Mutex::new(FinalizableProcessor::<
<VM::VMReferenceGlue as ReferenceGlue<VM>>::FinalizableType,
Expand All @@ -141,20 +145,31 @@ impl<VM: VMBinding> MMTK<VM> {

pub fn harness_begin(&self, tls: VMMutatorThread) {
probe!(mmtk, harness_begin);
self.plan.handle_user_collection_request(tls, true, true);
self.get_plan()
.handle_user_collection_request(tls, true, true);
self.inside_harness.store(true, Ordering::SeqCst);
self.plan.base().stats.start_all();
self.get_plan().base().stats.start_all();
self.scheduler.enable_stat();
}

pub fn harness_end(&'static self) {
self.plan.base().stats.stop_all(self);
self.get_plan().base().stats.stop_all(self);
self.inside_harness.store(false, Ordering::SeqCst);
probe!(mmtk, harness_end);
}

pub fn get_plan(&self) -> &dyn Plan<VM = VM> {
self.plan.as_ref()
unsafe { &**(self.plan.get()) }
}

/// Get the plan as mutable reference.
///
/// # Safety
///
/// This is unsafe because the caller must ensure that the plan is not used by other threads.
#[allow(clippy::mut_from_ref)]
pub unsafe fn get_plan_mut(&self) -> &mut dyn Plan<VM = VM> {
&mut **(self.plan.get())
}

pub fn get_options(&self) -> &Options {
Expand Down
9 changes: 6 additions & 3 deletions src/plan/generational/copying/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,19 @@ pub fn create_gencopy_mutator<VM: VMBinding>(
mutator_tls: VMMutatorThread,
mmtk: &'static MMTK<VM>,
) -> Mutator<VM> {
let gencopy = mmtk.plan.downcast_ref::<GenCopy<VM>>().unwrap();
let gencopy = mmtk.get_plan().downcast_ref::<GenCopy<VM>>().unwrap();
let config = MutatorConfig {
allocator_mapping: &ALLOCATOR_MAPPING,
space_mapping: Box::new(create_gen_space_mapping(&*mmtk.plan, &gencopy.gen.nursery)),
space_mapping: Box::new(create_gen_space_mapping(
mmtk.get_plan(),
&gencopy.gen.nursery,
)),
prepare_func: &gencopy_mutator_prepare,
release_func: &gencopy_mutator_release,
};

Mutator {
allocators: Allocators::<VM>::new(mutator_tls, &*mmtk.plan, &config.space_mapping),
allocators: Allocators::<VM>::new(mutator_tls, mmtk.get_plan(), &config.space_mapping),
barrier: Box::new(ObjectBarrier::new(GenObjectBarrierSemantics::new(
mmtk, gencopy,
))),
Expand Down
14 changes: 12 additions & 2 deletions src/plan/generational/gc_work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,12 @@ impl<E: ProcessEdgesWork> GCWork<E::VM> for ProcessModBuf<E> {
);
}
// scan modbuf only if the current GC is a nursery GC
if mmtk.plan.generational().unwrap().is_current_gc_nursery() {
if mmtk
.get_plan()
.generational()
.unwrap()
.is_current_gc_nursery()
{
// Scan objects in the modbuf and forward pointers
let modbuf = std::mem::take(&mut self.modbuf);
GCWork::do_work(
Expand Down Expand Up @@ -135,7 +140,12 @@ impl<E: ProcessEdgesWork> ProcessRegionModBuf<E> {
impl<E: ProcessEdgesWork> GCWork<E::VM> for ProcessRegionModBuf<E> {
fn do_work(&mut self, worker: &mut GCWorker<E::VM>, mmtk: &'static MMTK<E::VM>) {
// Scan modbuf only if the current GC is a nursery GC
if mmtk.plan.generational().unwrap().is_current_gc_nursery() {
if mmtk
.get_plan()
.generational()
.unwrap()
.is_current_gc_nursery()
{
// Collect all the entries in all the slices
let mut edges = vec![];
for slice in &self.modbuf {
Expand Down
9 changes: 6 additions & 3 deletions src/plan/generational/immix/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,19 @@ pub fn create_genimmix_mutator<VM: VMBinding>(
mutator_tls: VMMutatorThread,
mmtk: &'static MMTK<VM>,
) -> Mutator<VM> {
let genimmix = mmtk.plan.downcast_ref::<GenImmix<VM>>().unwrap();
let genimmix = mmtk.get_plan().downcast_ref::<GenImmix<VM>>().unwrap();
let config = MutatorConfig {
allocator_mapping: &ALLOCATOR_MAPPING,
space_mapping: Box::new(create_gen_space_mapping(&*mmtk.plan, &genimmix.gen.nursery)),
space_mapping: Box::new(create_gen_space_mapping(
mmtk.get_plan(),
&genimmix.gen.nursery,
)),
prepare_func: &genimmix_mutator_prepare,
release_func: &genimmix_mutator_release,
};

Mutator {
allocators: Allocators::<VM>::new(mutator_tls, &*mmtk.plan, &config.space_mapping),
allocators: Allocators::<VM>::new(mutator_tls, mmtk.get_plan(), &config.space_mapping),
barrier: Box::new(ObjectBarrier::new(GenObjectBarrierSemantics::new(
mmtk, genimmix,
))),
Expand Down
17 changes: 10 additions & 7 deletions src/plan/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ pub fn create_mutator<VM: VMBinding>(
mmtk: &'static MMTK<VM>,
) -> Box<Mutator<VM>> {
Box::new(match *mmtk.options.plan {
PlanSelector::NoGC => crate::plan::nogc::mutator::create_nogc_mutator(tls, &*mmtk.plan),
PlanSelector::NoGC => crate::plan::nogc::mutator::create_nogc_mutator(tls, mmtk.get_plan()),
PlanSelector::SemiSpace => {
crate::plan::semispace::mutator::create_ss_mutator(tls, &*mmtk.plan)
crate::plan::semispace::mutator::create_ss_mutator(tls, mmtk.get_plan())
}
PlanSelector::GenCopy => {
crate::plan::generational::copying::mutator::create_gencopy_mutator(tls, mmtk)
Expand All @@ -51,14 +51,16 @@ pub fn create_mutator<VM: VMBinding>(
crate::plan::generational::immix::mutator::create_genimmix_mutator(tls, mmtk)
}
PlanSelector::MarkSweep => {
crate::plan::marksweep::mutator::create_ms_mutator(tls, &*mmtk.plan)
crate::plan::marksweep::mutator::create_ms_mutator(tls, mmtk.get_plan())
}
PlanSelector::Immix => {
crate::plan::immix::mutator::create_immix_mutator(tls, mmtk.get_plan())
}
PlanSelector::Immix => crate::plan::immix::mutator::create_immix_mutator(tls, &*mmtk.plan),
PlanSelector::PageProtect => {
crate::plan::pageprotect::mutator::create_pp_mutator(tls, &*mmtk.plan)
crate::plan::pageprotect::mutator::create_pp_mutator(tls, mmtk.get_plan())
}
PlanSelector::MarkCompact => {
crate::plan::markcompact::mutator::create_markcompact_mutator(tls, &*mmtk.plan)
crate::plan::markcompact::mutator::create_markcompact_mutator(tls, mmtk.get_plan())
}
PlanSelector::StickyImmix => {
crate::plan::sticky::immix::mutator::create_stickyimmix_mutator(tls, mmtk)
Expand Down Expand Up @@ -137,7 +139,7 @@ pub fn create_gc_worker_context<VM: VMBinding>(
tls: VMWorkerThread,
mmtk: &'static MMTK<VM>,
) -> GCWorkerCopyContext<VM> {
GCWorkerCopyContext::<VM>::new(tls, &*mmtk.plan, mmtk.plan.create_copy_config())
GCWorkerCopyContext::<VM>::new(tls, mmtk.get_plan(), mmtk.get_plan().create_copy_config())
}

/// A plan describes the global core functionality for all memory management schemes.
Expand Down Expand Up @@ -854,6 +856,7 @@ impl<VM: VMBinding> BasePlan<VM> {
}

#[allow(unused_variables)] // depending on the enabled features, base may not be used.
#[allow(clippy::needless_pass_by_ref_mut)] // depending on the enabled features, base may not be used.
pub(crate) fn verify_side_metadata_sanity(
&self,
side_metadata_sanity_checker: &mut SideMetadataSanity,
Expand Down
2 changes: 1 addition & 1 deletion src/plan/markcompact/gc_work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl<VM: VMBinding> GCWork<VM> for UpdateReferences<VM> {
fn do_work(&mut self, _worker: &mut GCWorker<VM>, mmtk: &'static MMTK<VM>) {
// The following needs to be done right before the second round of root scanning
VM::VMScanning::prepare_for_roots_re_scanning();
mmtk.plan.base().prepare_for_stack_scanning();
mmtk.get_plan().base().prepare_for_stack_scanning();
#[cfg(feature = "extreme_assertions")]
mmtk.edge_logger.reset();

Expand Down
8 changes: 4 additions & 4 deletions src/plan/sticky/immix/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ pub fn create_stickyimmix_mutator<VM: VMBinding>(
mutator_tls: VMMutatorThread,
mmtk: &'static MMTK<VM>,
) -> Mutator<VM> {
let stickyimmix = mmtk.plan.downcast_ref::<StickyImmix<VM>>().unwrap();
let stickyimmix = mmtk.get_plan().downcast_ref::<StickyImmix<VM>>().unwrap();
let config = MutatorConfig {
allocator_mapping: &ALLOCATOR_MAPPING,
space_mapping: Box::new({
let mut vec =
create_space_mapping(immix::mutator::RESERVED_ALLOCATORS, true, &*mmtk.plan);
create_space_mapping(immix::mutator::RESERVED_ALLOCATORS, true, mmtk.get_plan());
vec.push((AllocatorSelector::Immix(0), stickyimmix.get_immix_space()));
vec
}),
Expand All @@ -38,13 +38,13 @@ pub fn create_stickyimmix_mutator<VM: VMBinding>(
};

Mutator {
allocators: Allocators::<VM>::new(mutator_tls, &*mmtk.plan, &config.space_mapping),
allocators: Allocators::<VM>::new(mutator_tls, mmtk.get_plan(), &config.space_mapping),
barrier: Box::new(ObjectBarrier::new(GenObjectBarrierSemantics::new(
mmtk,
stickyimmix,
))),
mutator_tls,
config,
plan: &*mmtk.plan,
plan: mmtk.get_plan(),
}
}
2 changes: 1 addition & 1 deletion src/policy/immix/immixspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl<VM: VMBinding> crate::policy::gc_work::PolicyTraceObject<VM> for ImmixSpace
if Block::containing::<VM>(object).is_defrag_source() {
debug_assert!(self.in_defrag());
debug_assert!(
!crate::plan::is_nursery_gc(&*worker.mmtk.plan),
!crate::plan::is_nursery_gc(worker.mmtk.get_plan()),
"Calling PolicyTraceObject on Immix in nursery GC"
);
self.trace_object_with_opportunistic_copy(
Expand Down
Loading