Skip to content

Remove cast ref to mut everywhere #893

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 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"

# --- Check main crate ---

37 changes: 23 additions & 14 deletions src/memory_manager.rs
Original file line number Diff line number Diff line change
@@ -88,7 +88,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
@@ -345,7 +350,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
@@ -467,11 +472,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);
}

@@ -483,10 +491,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);
@@ -504,10 +512,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);
@@ -538,7 +546,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
@@ -547,7 +555,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.
@@ -558,7 +566,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)
@@ -581,7 +589,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.
@@ -590,7 +598,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?
@@ -709,7 +718,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
31 changes: 23 additions & 8 deletions src/mmtk.rs
Original file line number Diff line number Diff line change
@@ -15,6 +15,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;
@@ -30,10 +31,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;
@@ -88,7 +89,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>>,
@@ -100,6 +101,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.
@@ -136,7 +140,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,
@@ -152,20 +156,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 {
9 changes: 6 additions & 3 deletions src/plan/generational/copying/mutator.rs
Original file line number Diff line number Diff line change
@@ -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,
))),
14 changes: 12 additions & 2 deletions src/plan/generational/gc_work.rs
Original file line number Diff line number Diff line change
@@ -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(
@@ -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 {
9 changes: 6 additions & 3 deletions src/plan/generational/immix/mutator.rs
Original file line number Diff line number Diff line change
@@ -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,
))),
17 changes: 10 additions & 7 deletions src/plan/global.rs
Original file line number Diff line number Diff line change
@@ -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)
@@ -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)
@@ -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.
@@ -857,6 +859,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,
2 changes: 1 addition & 1 deletion src/plan/markcompact/gc_work.rs
Original file line number Diff line number Diff line change
@@ -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();

8 changes: 4 additions & 4 deletions src/plan/sticky/immix/mutator.rs
Original file line number Diff line number Diff line change
@@ -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
}),
@@ -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
@@ -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(
Loading