-
Notifications
You must be signed in to change notification settings - Fork 69
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
Introduce ArcFlexMut
for Space references
#965
base: master
Are you sure you want to change the base?
Conversation
plan. Introduce AllocatorContext
now takes an immutable reference to the plan.
Any comment and critics are welcome. I would need to know if this is on the right track to solve #852. My general feeling is that our current code is quite messy when dealing with mutability and lifetime for Just let me know what you think. If this looks like a good direction to continue on, I will start solving issues for PRs |
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: mmtk#959
I added the following micro benchmark to test the performance of c.bench_function("post_alloc", |b| {
b.iter(|| {
api::mmtk_post_alloc(fixture.mutator, obj, 8, AllocationSemantics::Default);
})
}); Using SFT to dispatch fn post_alloc(
&mut self,
refer: ObjectReference,
_bytes: usize,
_allocator: AllocationSemantics,
) {
unsafe { crate::mmtk::SFT_MAP.get_unchecked(refer.to_address::<VM>()) }
.initialize_object_metadata(refer, true)
} Running benches/main.rs (target/release/deps/main-041e7708dfacb17d)
post_alloc time: [4.0283 ns 4.0297 ns 4.0316 ns]
Found 11 outliers among 100 measurements (11.00%)
1 (1.00%) low severe
1 (1.00%) low mild
4 (4.00%) high mild
5 (5.00%) high severe Using allocator selector and invoke fn post_alloc(
&mut self,
refer: ObjectReference,
_bytes: usize,
allocator: AllocationSemantics,
) {
unsafe {
self.allocators
.get_allocator_mut(self.config.allocator_mapping[allocator])
}
.get_space()
.initialize_object_metadata(refer, true)
} Running benches/main.rs (target/release/deps/main-9b17a5e12d50ed82)
post_alloc time: [4.4596 ns 4.4609 ns 4.4623 ns]
change: [+10.622% +10.701% +10.775%] (p = 0.00 < 0.05)
Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
1 (1.00%) low mild
2 (2.00%) high mild
3 (3.00%) high severe |
This PR is based on #949.
It introduces
ArcFlexMut
for some shared references where 1. their mutability is managed by the programmer, 2. mutability is hard to reason about statically, 3. using locks orRefCell
/AtomicRefCell
is not plausible for the sake of performance, and 4. the shared reference could be both statically typed and a dyn ref to a trait object, depending on where it is used.ArcFlexMut
is a replacement forUnsafeCell
. BasicallyArcFlexMut
can be used for bothPlan
andSpace
. This PR only usesArcFlexMut
forSpace
, as currently with the refactoring, we do not need to mutate on a plan.In my preliminary evaluation, the PR should have no performance overhead over #949 on immix. I will rerun the benchmarks and provide results before we attempt to merge the PR.
Changes:
ArcFlexMut
and use it for spaces. With the change, we always access the plan with an immutable reference, and no longer needUnsafeCell
for the plan. We acquire mutable/immutable references to the spaces instead. This closes Removecast_ref_to_mut
related to Prepare and Release #852.Space::acquire()
. This avoids holding a reference to the space during GC.acquire()
now returns a result. For an error return value, the caller (the allocators) need to block for GC.SFT
to dispatchinitialize_object_metadata()
inpost_alloc
. With this change, we can removeAllocator::get_space()
. This actually has performance improvement over the current code. This closes Use SFT for initialize_object_metadata #963.