Skip to content

Commit

Permalink
Only reset unlog bits of objects in modbuf for nursery GCs
Browse files Browse the repository at this point in the history
This fixes a bug wherein we were erroneously resetting the unlog bits
for objects in the modbuf for full-heap GCs. If we reset the unlog bits
in a full-heap GC, we may end up setting the unlog bit for an object
that actually died which may cause issues for future allocations.

We now set the unlog bits for mature objects when tracing for VM
space (if compiled with `set_unlog_bits_vm_space`), immortal space, and
large object space.

This PR also adds debug assertions checking that any object that has
been added to the modbuf is considered "live" by MMTk, or in other
words, it is a mature object.
  • Loading branch information
k-sareen committed Jul 28, 2024
1 parent b8355f8 commit 4a29049
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/plan/barriers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ impl<S: BarrierSemantics> Barrier<S::VM> for ObjectBarrier<S> {
target: Option<ObjectReference>,
) {
if self.log_object(src) {
debug_assert!(src.is_live::<S::VM>(), "{} was logged but is not live", src);
self.semantics
.object_reference_write_slow(src, slot, target);
}
Expand Down
26 changes: 16 additions & 10 deletions src/plan/generational/gc_work.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,28 @@ impl<E: ProcessEdgesWork> ProcessModBuf<E> {

impl<E: ProcessEdgesWork> GCWork<E::VM> for ProcessModBuf<E> {
fn do_work(&mut self, worker: &mut GCWorker<E::VM>, mmtk: &'static MMTK<E::VM>) {
// Flip the per-object unlogged bits to "unlogged" state.
for obj in &self.modbuf {
<E::VM as VMBinding>::VMObjectModel::GLOBAL_LOG_BIT_SPEC.store_atomic::<E::VM, u8>(
*obj,
1,
None,
Ordering::SeqCst,
);
}
// scan modbuf only if the current GC is a nursery GC
// Process and scan modbuf only if the current GC is a nursery GC
if mmtk
.get_plan()
.generational()
.unwrap()
.is_current_gc_nursery()
{
// Flip the per-object unlogged bits to "unlogged" state.
for obj in &self.modbuf {
debug_assert!(
(*obj).is_live::<E::VM>(),
"{} was logged but is not live",
*obj
);

<E::VM as VMBinding>::VMObjectModel::GLOBAL_LOG_BIT_SPEC.store_atomic::<E::VM, u8>(
*obj,
1,
None,
Ordering::SeqCst,
);
}
// Scan objects in the modbuf and forward pointers
let modbuf = std::mem::take(&mut self.modbuf);
GCWork::do_work(
Expand Down
5 changes: 5 additions & 0 deletions src/policy/immortalspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ impl<VM: VMBinding> ImmortalSpace<VM> {
object
);
if self.mark_state.test_and_mark::<VM>(object) {
// Set the unlog bit if required
if self.common.needs_log_bit {
<E::VM as VMBinding>::VMObjectModel::GLOBAL_LOG_BIT_SPEC
.store_atomic::<E::VM, u8>(object, 1, None, Ordering::SeqCst);
}
queue.enqueue(object);
}
object
Expand Down
9 changes: 8 additions & 1 deletion src/policy/largeobjectspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,9 @@ impl<VM: VMBinding> LargeObjectSpace<VM> {
trace!("LOS object {} is being marked now", object);
self.treadmill.copy(object, nursery_object);
// We just moved the object out of the logical nursery, mark it as unlogged.
if nursery_object && self.common.needs_log_bit {
// We also unlog mature objects as their unlog bit may have been unset before the
// full-heap GC
if self.common.needs_log_bit {
VM::VMObjectModel::GLOBAL_LOG_BIT_SPEC
.mark_as_unlogged::<VM>(object, Ordering::SeqCst);
}
Expand All @@ -230,6 +232,11 @@ impl<VM: VMBinding> LargeObjectSpace<VM> {
let sweep = |object: ObjectReference| {
#[cfg(feature = "vo_bit")]
crate::util::metadata::vo_bit::unset_vo_bit::<VM>(object);
// Clear log bits for dead objects to prevent a new nursery object having the unlog bit set
if self.common.needs_log_bit {
<E::VM as VMBinding>::VMObjectModel::GLOBAL_LOG_BIT_SPEC
.store_atomic::<E::VM, u8>(object, 0, None, Ordering::SeqCst);
}
self.pr
.release_pages(get_super_page(object.to_object_start::<VM>()));
};
Expand Down
7 changes: 7 additions & 0 deletions src/policy/vmspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,13 @@ impl<VM: VMBinding> VMSpace<VM> {
);
debug_assert!(self.in_space(object));
if self.mark_state.test_and_mark::<VM>(object) {
// Flip the per-object unlogged bits to "unlogged" state for objects inside the
// bootimage
#[cfg(set_unlog_bits_vm_space)]
if self.common.needs_log_bit {
<E::VM as VMBinding>::VMObjectModel::GLOBAL_LOG_BIT_SPEC
.store_atomic::<E::VM, u8>(object, 1, None, Ordering::SeqCst);
}
queue.enqueue(object);
}
object
Expand Down

0 comments on commit 4a29049

Please sign in to comment.