Skip to content

Commit

Permalink
Clear all side forwarding bits (#974)
Browse files Browse the repository at this point in the history
This reverts an optimization which was intended to reduce the number of
side forwarding bits cleared for ImmixSpace by clearing the forwarding
bits metadata of defrag sources, only. This approach does not work for
StickyImmix because it may move young objects, which is not "defrag".

Now ImmixSpace clears the on-the-side forwarding bits for all chunks
just like how it clears the on-the-side marking bits.

This PR prefers correctness over performance, and may degrade the
performance of VMs that use on-the-side forwarding bits. Currently it
only affects the Ruby binding, but we plan to let the Ruby binding use
in-header forwarding states, too. The performance of VMs that use
in-header forwarding bits is not affected by this PR.

Fixes: #972
  • Loading branch information
wks authored Oct 12, 2023
1 parent 0328b05 commit 2d7b569
Showing 1 changed file with 6 additions and 25 deletions.
31 changes: 6 additions & 25 deletions src/policy/immix/immixspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ impl<VM: VMBinding> SFT for ImmixSpace<VM> {
}

fn get_forwarded_object(&self, object: ObjectReference) -> Option<ObjectReference> {
if !Block::containing::<VM>(object).is_defrag_source() {
// If we never move objects, look no further.
if super::NEVER_MOVE_OBJECTS {
return None;
}

Expand All @@ -108,17 +109,6 @@ impl<VM: VMBinding> SFT for ImmixSpace<VM> {
return false;
}

// If the forwarding bits are on the side,
if VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.is_on_side() {
// we need to ensure `object` is in a defrag source
// because `PrepareBlockState` does not clear forwarding bits
// for non-defrag-source blocks.
if !Block::containing::<VM>(object).is_defrag_source() {
// Objects not in defrag sources cannot be forwarded.
return false;
}
}

// If the object is forwarded, it is live, too.
ForwardingWord::is_forwarded::<VM>(object)
}
Expand Down Expand Up @@ -843,6 +833,10 @@ impl<VM: VMBinding> PrepareBlockState<VM> {
unimplemented!("We cannot bulk zero unlogged bit.")
}
}
// If the forwarding bits are on the side, we need to clear them, too.
if let MetadataSpec::OnSide(side) = *VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC {
side.bzero_metadata(self.chunk.start(), Chunk::BYTES);
}
}
}

Expand Down Expand Up @@ -876,19 +870,6 @@ impl<VM: VMBinding> GCWork<VM> for PrepareBlockState<VM> {
block.set_state(BlockState::Unmarked);
debug_assert!(!block.get_state().is_reusable());
debug_assert_ne!(block.get_state(), BlockState::Marked);
// Clear forwarding bits if necessary.
if is_defrag_source {
// Note that `ImmixSpace::is_live` depends on the fact that we only clear side
// forwarding bits for defrag sources. If we change the code here, we need to
// make sure `ImmixSpace::is_live` is fixed, too.
if let MetadataSpec::OnSide(side) = *VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC {
// Clear on-the-side forwarding bits.
side.bzero_metadata(block.start(), Block::BYTES);
}
}
// NOTE: We don't need to reset the forwarding pointer metadata because it is meaningless
// until the forwarding bits are also set, at which time we also write the forwarding
// pointer.
}
}
}
Expand Down

0 comments on commit 2d7b569

Please sign in to comment.