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
17 changes: 13 additions & 4 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.get_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 @@ -473,7 +478,10 @@ pub fn initialize_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThre
"MMTk collection has been initialized (was initialize_collection() already called before?)"
);
mmtk.scheduler.spawn_gc_threads(mmtk, tls);
mmtk.get_plan().base().initialized.store(true, Ordering::SeqCst);
mmtk.get_plan()
.base()
.initialized
.store(true, Ordering::SeqCst);
probe!(mmtk, collection_initialized);
}

Expand Down Expand Up @@ -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 Down Expand Up @@ -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.get_plan().handle_user_collection_request(tls, false, false);
mmtk.get_plan()
.handle_user_collection_request(tls, false, false);
}

/// Is the object alive?
Expand Down
8 changes: 4 additions & 4 deletions src/mmtk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ pub struct MMTK<VM: VMBinding> {
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 @@ -146,7 +145,8 @@ impl<VM: VMBinding> MMTK<VM> {

pub fn harness_begin(&self, tls: VMMutatorThread) {
probe!(mmtk, harness_begin);
self.get_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.get_plan().base().stats.start_all();
self.scheduler.enable_stat();
Expand All @@ -163,9 +163,9 @@ impl<VM: VMBinding> MMTK<VM> {
}

/// 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> {
Expand Down
5 changes: 4 additions & 1 deletion src/plan/generational/copying/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ pub fn create_gencopy_mutator<VM: VMBinding>(
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.get_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,
};
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.get_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.get_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
5 changes: 4 additions & 1 deletion src/plan/generational/immix/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ pub fn create_genimmix_mutator<VM: VMBinding>(
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.get_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,
};
Expand Down
7 changes: 5 additions & 2 deletions src/plan/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ pub fn create_mutator<VM: VMBinding>(
PlanSelector::MarkSweep => {
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.get_plan())
}
PlanSelector::PageProtect => {
crate::plan::pageprotect::mutator::create_pp_mutator(tls, mmtk.get_plan())
}
Expand Down Expand Up @@ -854,9 +856,10 @@ 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: &SideMetadataSanity,
side_metadata_sanity_checker: &mut SideMetadataSanity,
) {
#[cfg(feature = "code_space")]
self.code_space
Expand Down
2 changes: 1 addition & 1 deletion src/plan/nogc/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl<VM: VMBinding> NoGC<VM> {
// side metadata in extreme_assertions.
let mut side_metadata_sanity_checker = SideMetadataSanity::new();
res.base()
.verify_side_metadata_sanity(&side_metadata_sanity_checker);
.verify_side_metadata_sanity(&mut side_metadata_sanity_checker);
res.nogc_space
.verify_side_metadata_sanity(&mut side_metadata_sanity_checker);

Expand Down
7 changes: 6 additions & 1 deletion src/scheduler/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,12 @@ impl<VM: VMBinding> GCController<VM> {
self.scheduler.deactivate_all();

// Tell GC trigger that GC ended - this happens before EndOfGC where we resume mutators.
self.mmtk.get_plan().base().gc_trigger.policy.on_gc_end(self.mmtk);
self.mmtk
.get_plan()
.base()
.gc_trigger
.policy
.on_gc_end(self.mmtk);

// Finalization: Resume mutators, reset gc states
// Note: Resume-mutators must happen after all work buckets are closed.
Expand Down
4 changes: 2 additions & 2 deletions src/scheduler/gc_work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl<C: GCWorkContext + 'static> GCWork<C::VM> for Release<C> {
.scheduler
.worker_group
.get_and_clear_worker_live_bytes();
self.plan
mmtk.get_plan()
.base()
.live_bytes_in_last_gc
.store(live_bytes, std::sync::atomic::Ordering::SeqCst);
Expand Down Expand Up @@ -246,7 +246,7 @@ impl<VM: VMBinding> GCWork<VM> for EndOfGC {
#[cfg(feature = "count_live_bytes_in_gc")]
{
let live_bytes = mmtk
.plan
.get_plan()
.base()
.live_bytes_in_last_gc
.load(std::sync::atomic::Ordering::SeqCst);
Expand Down
2 changes: 1 addition & 1 deletion src/scheduler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod scheduler;
pub(crate) use scheduler::GCWorkScheduler;

mod stat;
pub(self) mod work_counter;
mod work_counter;

mod work;
pub use work::GCWork;
Expand Down
3 changes: 1 addition & 2 deletions src/util/heap/layout/fragmented_mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,7 @@ impl FragmentedMapper {
}

fn get_or_allocate_slab_table(&self, addr: Address) -> &Slab {
self
.get_or_optionally_allocate_slab_table(addr, true)
self.get_or_optionally_allocate_slab_table(addr, true)
.unwrap()
}

Expand Down
4 changes: 2 additions & 2 deletions src/util/heap/layout/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ pub trait VMMap: Sync {

/// Bind a created freelist with the page resource.
/// This must called after create_freelist() or create_parent_freelist().
///
///
/// # Safety
///
///
/// * `pr` must be a valid pointer to a CommonFreeListPageResource and be alive
/// for the duration of the VMMap.
unsafe fn bind_freelist(&self, pr: *const CommonFreeListPageResource);
Expand Down
1 change: 0 additions & 1 deletion src/util/heap/layout/map32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ impl Map32 {
}
chunks as _
}

}

fn get_discontig_freelist_pr_ordinal(&self) -> usize {
Expand Down
8 changes: 2 additions & 6 deletions src/util/int_array_freelist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,10 @@ impl IntArrayFreeList {
}

// FIXME: We need a safe implementation

fn table_mut(&mut self) -> &mut Vec<i32> {
match self.parent {
Some(mut p) => {
unsafe {
p.as_mut().table_mut()
}
}
Some(mut p) => unsafe { p.as_mut().table_mut() },
None => self.table.as_mut().unwrap(),
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/util/reference_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,12 @@ impl ReferenceProcessors {
mmtk.get_plan().constraints().needs_forward_after_liveness,
"A plan with needs_forward_after_liveness=false does not need a separate forward step"
);
self.soft.forward::<E>(trace, is_nursery_gc(mmtk.get_plan()));
self.weak.forward::<E>(trace, is_nursery_gc(mmtk.get_plan()));
self.phantom.forward::<E>(trace, is_nursery_gc(mmtk.get_plan()));
self.soft
.forward::<E>(trace, is_nursery_gc(mmtk.get_plan()));
self.weak
.forward::<E>(trace, is_nursery_gc(mmtk.get_plan()));
self.phantom
.forward::<E>(trace, is_nursery_gc(mmtk.get_plan()));
}

// Methods for scanning weak references. It needs to be called in a decreasing order of reference strengths, i.e. soft > weak > phantom
Expand Down Expand Up @@ -101,7 +104,8 @@ impl ReferenceProcessors {
trace: &mut E,
mmtk: &'static MMTK<E::VM>,
) {
self.phantom.scan::<E>(trace, is_nursery_gc(mmtk.get_plan()));
self.phantom
.scan::<E>(trace, is_nursery_gc(mmtk.get_plan()));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/util/sanity/sanity_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl<P: Plan> ScheduleSanityGC<P> {
impl<P: Plan> GCWork<P::VM> for ScheduleSanityGC<P> {
fn do_work(&mut self, worker: &mut GCWorker<P::VM>, mmtk: &'static MMTK<P::VM>) {
let scheduler = worker.scheduler();
let plan = &mmtk.plan;
let plan = mmtk.get_plan();

scheduler.reset_state();

Expand Down Expand Up @@ -205,7 +205,7 @@ impl<VM: VMBinding> ProcessEdgesWork for SanityGCProcessEdges<VM> {

// Let plan check object
assert!(
self.mmtk().plan.sanity_check_object(object),
self.mmtk().get_plan().sanity_check_object(object),
"Invalid reference {:?}",
object
);
Expand Down