Skip to content

Commit

Permalink
Disabling PrepareMutator from PlanConstraints (#964)
Browse files Browse the repository at this point in the history
Some plans do nothing when preparing mutators. We add a boolean flag so
that we do not create the PrepareMutator work packets in the first
place.

We also remove the function that prepares Mutator for Immix. It is
unnecessary to reset the ImmixAllocator of a mutator when preparing
mutator because the mutator's ImmixAllocator is not used during GC. When
a GC worker defragments the heap and promotes young objects, it uses the
ImmixAllocator instances in ImmixCopyContext or ImmixHybridCopyContext.

Fixes: #959
  • Loading branch information
wks authored Sep 29, 2023
1 parent 15e19a1 commit cb14881
Show file tree
Hide file tree
Showing 18 changed files with 82 additions and 81 deletions.
42 changes: 21 additions & 21 deletions docs/userguide/src/tutorial/mygc/ss/collection.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,19 @@ the new `tospace`.

### Prepare mutator

Going back to `mutator.rs`, create a new function called
`mygc_mutator_prepare(_mutator: &mut Mutator <MyGC<VM>>, _tls: OpaquePointer,)`.
This function will be called at the preparation stage of a collection
(at the start of a collection) for each mutator. Its body can stay empty, as
there aren't any preparation steps for the mutator in this GC.
In `create_mygc_mutator()`, find the field `prep_func` and change it from
`mygc_mutator_noop()` to `mygc_mutator_prepare()`.
Going back to `mutator.rs`, create a new function called
`mygc_mutator_prepare<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread)`.
This function will be called at the preparation stage of a collection (at the start of a
collection) for each mutator. Its body can stay empty, as there aren't any preparation steps for
the mutator in this GC. In `create_mygc_mutator()`, find the field `prepare_func` and change it
from `&unreachable_prepare_func` to `&mygc_mutator_prepare`.

> 💡 Hint: If your plan does nothing when preparing mutators, there is an optimization you can do.
You may set the plan constraints field `PlanConstraints::needs_prepare_mutator` to `false` so that
the `PrepareMutator` work packets which call `prepare_func` will not be created in the first place.
This optimization is helpful for VMs that run with a large number of mutator threads. If you do
this optimization, you may also leave the `MutatorConfig::prepare_func` field as
`&unreachable_prepare_func` to indicate it should not be called.

## Release

Expand All @@ -131,24 +137,18 @@ routines for the common plan spaces and the fromspace.

### Release in mutator

Go back to `mutator.rs`. In `create_mygc_mutator()`, replace
`mygc_mutator_noop()` in the `release_func` field with `mygc_mutator_release()`.
Leave the `release()` function in the `CopyContext` empty. There are no
release steps for `CopyContext` in this collector.

Create a new function called `mygc_mutator_release()` that takes the same
inputs as the `prepare()` function above. This function will be called at the
release stage of a collection (at the end of a collection) for each mutator.
It rebinds the allocator for the `Default` allocation semantics to the new
tospace. When the mutator threads resume, any new allocations for `Default`
will then go to the new tospace.

Go back to `mutator.rs`. Create a new function called `mygc_mutator_release()` that takes the same
inputs as the `mygc_mutator_prepare()` function above.

```rust
{{#include ../../code/mygc_semispace/mutator.rs:release}}
```

Delete `mygc_mutator_noop()`. It was a placeholder for the prepare and
release functions that you have now added, so it is now dead code.
Then go to `create_mygc_mutator()`, replace `&unreachable_release_func` in the `release_func` field
with `&mygc_mutator_release`. This function will be called at the release stage of a collection
(at the end of a collection) for each mutator. It rebinds the allocator for the `Default`
allocation semantics to the new tospace. When the mutator threads resume, any new allocations for
`Default` will then go to the new tospace.

## ProcessEdgesWork for MyGC

Expand Down
7 changes: 2 additions & 5 deletions src/plan/generational/copying/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use super::GenCopy;
use crate::plan::barriers::ObjectBarrier;
use crate::plan::generational::barrier::GenObjectBarrierSemantics;
use crate::plan::generational::create_gen_space_mapping;
use crate::plan::mutator_context::unreachable_prepare_func;
use crate::plan::mutator_context::Mutator;
use crate::plan::mutator_context::MutatorConfig;
use crate::plan::AllocationSemantics;
Expand All @@ -12,10 +13,6 @@ use crate::util::{VMMutatorThread, VMWorkerThread};
use crate::vm::VMBinding;
use crate::MMTK;

pub fn gencopy_mutator_prepare<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
// Do nothing
}

pub fn gencopy_mutator_release<VM: VMBinding>(mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
// reset nursery allocator
let bump_allocator = unsafe {
Expand All @@ -39,7 +36,7 @@ pub fn create_gencopy_mutator<VM: VMBinding>(
mmtk.get_plan(),
&gencopy.gen.nursery,
)),
prepare_func: &gencopy_mutator_prepare,
prepare_func: &unreachable_prepare_func,
release_func: &gencopy_mutator_release,
};

Expand Down
5 changes: 2 additions & 3 deletions src/plan/generational/immix/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::plan::barriers::ObjectBarrier;
use crate::plan::generational::barrier::GenObjectBarrierSemantics;
use crate::plan::generational::create_gen_space_mapping;
use crate::plan::generational::immix::GenImmix;
use crate::plan::mutator_context::unreachable_prepare_func;
use crate::plan::mutator_context::Mutator;
use crate::plan::mutator_context::MutatorConfig;
use crate::plan::AllocationSemantics;
Expand All @@ -12,8 +13,6 @@ use crate::util::{VMMutatorThread, VMWorkerThread};
use crate::vm::VMBinding;
use crate::MMTK;

pub fn genimmix_mutator_prepare<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {}

pub fn genimmix_mutator_release<VM: VMBinding>(mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
// reset nursery allocator
let bump_allocator = unsafe {
Expand All @@ -37,7 +36,7 @@ pub fn create_genimmix_mutator<VM: VMBinding>(
mmtk.get_plan(),
&genimmix.gen.nursery,
)),
prepare_func: &genimmix_mutator_prepare,
prepare_func: &unreachable_prepare_func,
release_func: &genimmix_mutator_release,
};

Expand Down
1 change: 1 addition & 0 deletions src/plan/generational/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub const GEN_CONSTRAINTS: PlanConstraints = PlanConstraints {
crate::plan::plan_constraints::MAX_NON_LOS_ALLOC_BYTES_COPYING_PLAN,
crate::util::options::NURSERY_SIZE,
),
needs_prepare_mutator: false,
..PlanConstraints::default()
};

Expand Down
1 change: 1 addition & 0 deletions src/plan/immix/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub const IMMIX_CONSTRAINTS: PlanConstraints = PlanConstraints {
num_specialized_scans: 1,
/// Max immix object size is half of a block.
max_non_los_default_alloc_bytes: crate::policy::immix::MAX_IMMIX_OBJECT_SIZE,
needs_prepare_mutator: false,
..PlanConstraints::default()
};

Expand Down
14 changes: 2 additions & 12 deletions src/plan/immix/mutator.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::Immix;
use crate::plan::mutator_context::create_allocator_mapping;
use crate::plan::mutator_context::create_space_mapping;
use crate::plan::mutator_context::unreachable_prepare_func;
use crate::plan::mutator_context::Mutator;
use crate::plan::mutator_context::MutatorConfig;
use crate::plan::mutator_context::ReservedAllocators;
Expand All @@ -15,17 +16,6 @@ use crate::{
};
use enum_map::EnumMap;

pub fn immix_mutator_prepare<VM: VMBinding>(mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
let immix_allocator = unsafe {
mutator
.allocators
.get_allocator_mut(mutator.config.allocator_mapping[AllocationSemantics::Default])
}
.downcast_mut::<ImmixAllocator<VM>>()
.unwrap();
immix_allocator.reset();
}

pub fn immix_mutator_release<VM: VMBinding>(mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
let immix_allocator = unsafe {
mutator
Expand Down Expand Up @@ -62,7 +52,7 @@ pub fn create_immix_mutator<VM: VMBinding>(
vec.push((AllocatorSelector::Immix(0), &immix.immix_space));
vec
}),
prepare_func: &immix_mutator_prepare,
prepare_func: &unreachable_prepare_func,
release_func: &immix_mutator_release,
};

Expand Down
1 change: 1 addition & 0 deletions src/plan/markcompact/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub const MARKCOMPACT_CONSTRAINTS: PlanConstraints = PlanConstraints {
needs_forward_after_liveness: true,
max_non_los_default_alloc_bytes:
crate::plan::plan_constraints::MAX_NON_LOS_ALLOC_BYTES_COPYING_PLAN,
needs_prepare_mutator: false,
..PlanConstraints::default()
};

Expand Down
9 changes: 2 additions & 7 deletions src/plan/markcompact/mutator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::MarkCompact; // Add
use crate::plan::barriers::NoBarrier;
use crate::plan::mutator_context::create_allocator_mapping;
use crate::plan::mutator_context::create_space_mapping;
use crate::plan::mutator_context::unreachable_prepare_func;
use crate::plan::mutator_context::Mutator;
use crate::plan::mutator_context::MutatorConfig;
use crate::plan::mutator_context::ReservedAllocators;
Expand Down Expand Up @@ -38,7 +39,7 @@ pub fn create_markcompact_mutator<VM: VMBinding>(
vec.push((AllocatorSelector::MarkCompact(0), markcompact.mc_space()));
vec
}),
prepare_func: &markcompact_mutator_prepare,
prepare_func: &unreachable_prepare_func,
release_func: &markcompact_mutator_release,
};

Expand All @@ -51,12 +52,6 @@ pub fn create_markcompact_mutator<VM: VMBinding>(
}
}

pub fn markcompact_mutator_prepare<VM: VMBinding>(
_mutator: &mut Mutator<VM>,
_tls: VMWorkerThread,
) {
}

pub fn markcompact_mutator_release<VM: VMBinding>(
_mutator: &mut Mutator<VM>,
_tls: VMWorkerThread,
Expand Down
2 changes: 2 additions & 0 deletions src/plan/marksweep/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub const MS_CONSTRAINTS: PlanConstraints = PlanConstraints {
num_specialized_scans: 1,
max_non_los_default_alloc_bytes: MAX_OBJECT_SIZE,
may_trace_duplicate_edges: true,
needs_prepare_mutator: !cfg!(feature = "malloc_mark_sweep")
&& !cfg!(feature = "eager_sweeping"),
..PlanConstraints::default()
};

Expand Down
22 changes: 22 additions & 0 deletions src/plan/mutator_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,28 @@ use enum_map::EnumMap;

pub(crate) type SpaceMapping<VM> = Vec<(AllocatorSelector, &'static dyn Space<VM>)>;

/// A place-holder implementation for `MutatorConfig::prepare_func` that should not be called.
/// It is the most often used by plans that sets `PlanConstraints::needs_prepare_mutator` to
/// `false`. It is also used by `NoGC` because it must not trigger GC.
pub(crate) fn unreachable_prepare_func<VM: VMBinding>(
_mutator: &mut Mutator<VM>,
_tls: VMWorkerThread,
) {
unreachable!("`MutatorConfig::prepare_func` must not be called for the current plan.")
}

/// A place-holder implementation for `MutatorConfig::release_func` that should not be called.
/// Currently only used by `NoGC`.
pub(crate) fn unreachable_release_func<VM: VMBinding>(
_mutator: &mut Mutator<VM>,
_tls: VMWorkerThread,
) {
unreachable!("`MutatorConfig::release_func` must not be called for the current plan.")
}

/// A place-holder implementation for `MutatorConfig::release_func` that does nothing.
pub(crate) fn no_op_release_func<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {}

// This struct is part of the Mutator struct.
// We are trying to make it fixed-sized so that VM bindings can easily define a Mutator type to have the exact same layout as our Mutator struct.
#[repr(C)]
Expand Down
12 changes: 5 additions & 7 deletions src/plan/nogc/mutator.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::plan::barriers::NoBarrier;
use crate::plan::mutator_context::unreachable_prepare_func;
use crate::plan::mutator_context::unreachable_release_func;
use crate::plan::mutator_context::Mutator;
use crate::plan::mutator_context::MutatorConfig;
use crate::plan::mutator_context::{
Expand All @@ -8,7 +10,7 @@ use crate::plan::nogc::NoGC;
use crate::plan::AllocationSemantics;
use crate::plan::Plan;
use crate::util::alloc::allocators::{AllocatorSelector, Allocators};
use crate::util::{VMMutatorThread, VMWorkerThread};
use crate::util::VMMutatorThread;
use crate::vm::VMBinding;
use enum_map::{enum_map, EnumMap};

Expand Down Expand Up @@ -36,10 +38,6 @@ lazy_static! {
};
}

pub fn nogc_mutator_noop<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
unreachable!();
}

pub fn create_nogc_mutator<VM: VMBinding>(
mutator_tls: VMMutatorThread,
plan: &'static dyn Plan<VM = VM>,
Expand All @@ -62,8 +60,8 @@ pub fn create_nogc_mutator<VM: VMBinding>(
));
vec
}),
prepare_func: &nogc_mutator_noop,
release_func: &nogc_mutator_noop,
prepare_func: &unreachable_prepare_func,
release_func: &unreachable_release_func,
};

Mutator {
Expand Down
1 change: 1 addition & 0 deletions src/plan/pageprotect/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub struct PageProtect<VM: VMBinding> {

pub const CONSTRAINTS: PlanConstraints = PlanConstraints {
moves_objects: false,
needs_prepare_mutator: false,
..PlanConstraints::default()
};

Expand Down
17 changes: 5 additions & 12 deletions src/plan/pageprotect/mutator.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::PageProtect;
use crate::plan::mutator_context::no_op_release_func;
use crate::plan::mutator_context::unreachable_prepare_func;
use crate::plan::mutator_context::Mutator;
use crate::plan::mutator_context::MutatorConfig;
use crate::plan::mutator_context::{
Expand All @@ -8,18 +10,9 @@ use crate::plan::AllocationSemantics;
use crate::plan::Plan;
use crate::util::alloc::allocators::{AllocatorSelector, Allocators};
use crate::vm::VMBinding;
use crate::{
plan::barriers::NoBarrier,
util::opaque_pointer::{VMMutatorThread, VMWorkerThread},
};
use crate::{plan::barriers::NoBarrier, util::opaque_pointer::VMMutatorThread};
use enum_map::EnumMap;

/// Prepare mutator. Do nothing.
fn pp_mutator_prepare<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {}

/// Release mutator. Do nothing.
fn pp_mutator_release<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {}

const RESERVED_ALLOCATORS: ReservedAllocators = ReservedAllocators {
n_large_object: 1,
..ReservedAllocators::DEFAULT
Expand Down Expand Up @@ -47,8 +40,8 @@ pub fn create_pp_mutator<VM: VMBinding>(
vec.push((AllocatorSelector::LargeObject(0), &page.space));
vec
}),
prepare_func: &pp_mutator_prepare,
release_func: &pp_mutator_release,
prepare_func: &unreachable_prepare_func,
release_func: &no_op_release_func,
};

Mutator {
Expand Down
5 changes: 5 additions & 0 deletions src/plan/plan_constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ pub struct PlanConstraints {
/// Some policies do object forwarding after the first liveness transitive closure, such as mark compact.
/// For plans that use those policies, they should set this as true.
pub needs_forward_after_liveness: bool,
/// Some (in fact, most) plans do nothing when preparing mutators before tracing (i.e. in
/// `MutatorConfig::prepare_func`). Those plans can set this to `false` so that the
/// `PrepareMutator` work packets will not be created at all.
pub needs_prepare_mutator: bool,
}

impl PlanConstraints {
Expand All @@ -51,6 +55,7 @@ impl PlanConstraints {
needs_forward_after_liveness: false,
needs_log_bit: false,
barrier: BarrierSelector::NoBarrier,
needs_prepare_mutator: true,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/plan/semispace/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub const SS_CONSTRAINTS: PlanConstraints = PlanConstraints {
num_specialized_scans: 1,
max_non_los_default_alloc_bytes:
crate::plan::plan_constraints::MAX_NON_LOS_ALLOC_BYTES_COPYING_PLAN,
needs_prepare_mutator: false,
..PlanConstraints::default()
};

Expand Down
7 changes: 2 additions & 5 deletions src/plan/semispace/mutator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::SemiSpace;
use crate::plan::barriers::NoBarrier;
use crate::plan::mutator_context::unreachable_prepare_func;
use crate::plan::mutator_context::Mutator;
use crate::plan::mutator_context::MutatorConfig;
use crate::plan::mutator_context::{
Expand All @@ -13,10 +14,6 @@ use crate::util::{VMMutatorThread, VMWorkerThread};
use crate::vm::VMBinding;
use enum_map::EnumMap;

pub fn ss_mutator_prepare<VM: VMBinding>(_mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
// Do nothing
}

pub fn ss_mutator_release<VM: VMBinding>(mutator: &mut Mutator<VM>, _tls: VMWorkerThread) {
// rebind the allocation bump pointer to the appropriate semispace
let bump_allocator = unsafe {
Expand Down Expand Up @@ -60,7 +57,7 @@ pub fn create_ss_mutator<VM: VMBinding>(
vec.push((AllocatorSelector::BumpPointer(0), ss.tospace()));
vec
}),
prepare_func: &ss_mutator_prepare,
prepare_func: &unreachable_prepare_func,
release_func: &ss_mutator_release,
};

Expand Down
Loading

0 comments on commit cb14881

Please sign in to comment.