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

Clear side forwarding bits properly #1138

Merged
merged 9 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions src/policy/copyspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,32 +168,26 @@ impl<VM: VMBinding> CopySpace<VM> {

pub fn prepare(&self, from_space: bool) {
self.from_space.store(from_space, Ordering::SeqCst);
// Clear the metadata if we are using side forwarding status table. Otherwise
// objects may inherit forwarding status from the previous GC.
// TODO: Fix performance.
if let MetadataSpec::OnSide(side_forwarding_status_table) =
*<VM::VMObjectModel as ObjectModel<VM>>::LOCAL_FORWARDING_BITS_SPEC
{
side_forwarding_status_table
.bzero_metadata(self.common.start, self.pr.cursor() - self.common.start);
}
}

pub fn release(&self) {
unsafe {
for (start, size) in self.pr.iterate_allocated_regions() {
// Clear the forwarding bits if it is on the side.
if let MetadataSpec::OnSide(side_forwarding_status_table) =
*<VM::VMObjectModel as ObjectModel<VM>>::LOCAL_FORWARDING_BITS_SPEC
{
side_forwarding_status_table.bzero_metadata(start, size);
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
}

// Clear VO bits because all objects in the space are dead.
#[cfg(feature = "vo_bit")]
self.reset_vo_bit();
self.pr.reset();
crate::util::metadata::vo_bit::bzero_vo_bit(start, size);
}
self.common.metadata.reset();
self.from_space.store(false, Ordering::SeqCst);
}

#[cfg(feature = "vo_bit")]
unsafe fn reset_vo_bit(&self) {
for (start, size) in self.pr.iterate_allocated_regions() {
crate::util::metadata::vo_bit::bzero_vo_bit(start, size);
unsafe {
self.pr.reset();
}
self.from_space.store(false, Ordering::SeqCst);
}

fn is_from_space(&self) -> bool {
Expand Down
20 changes: 15 additions & 5 deletions src/policy/immix/immixspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -841,10 +841,6 @@ 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 @@ -891,14 +887,17 @@ struct SweepChunk<VM: VMBinding> {
}

impl<VM: VMBinding> GCWork<VM> for SweepChunk<VM> {
fn do_work(&mut self, _worker: &mut GCWorker<VM>, _mmtk: &'static MMTK<VM>) {
fn do_work(&mut self, _worker: &mut GCWorker<VM>, mmtk: &'static MMTK<VM>) {
let mut histogram = self.space.defrag.new_histogram();
if self.space.chunk_map.get(self.chunk) == ChunkState::Allocated {
let line_mark_state = if super::BLOCK_ONLY {
None
} else {
Some(self.space.line_mark_state.load(Ordering::Acquire))
};
// Hints for clearing side forwarding bits.
let is_moving_gc = mmtk.get_plan().current_gc_may_move_object();
let is_defrag_gc = self.space.defrag.in_defrag();
// number of allocated blocks.
let mut allocated_blocks = 0;
// Iterate over all allocated blocks in this chunk.
Expand All @@ -911,6 +910,17 @@ impl<VM: VMBinding> GCWork<VM> for SweepChunk<VM> {
// Block is live. Increment the allocated block count.
allocated_blocks += 1;
}

// Clear side forwarding bits.
// In the beginning of the next GC, no side forwarding bits shall be set.
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
if let MetadataSpec::OnSide(side) = *VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC {
if is_moving_gc {
let objects_may_move = !is_defrag_gc || block.is_defrag_source();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably add some comments here to make it more clear: if this is a defrag GC, we only need to clear forwarding bits for defrag source blocks; if this is a moving GC but not a defrag GC, we have no other information, and we have to clear forwarding bits for all the blocks.

if objects_may_move {
side.bzero_metadata(block.start(), Block::BYTES);
}
}
}
}
// Set this chunk as free if there is not live blocks.
if allocated_blocks == 0 {
Expand Down
2 changes: 0 additions & 2 deletions src/util/metadata/side_metadata/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1080,8 +1080,6 @@ impl SideMetadataContext {
total
}

pub fn reset(&self) {}

// ** NOTE: **
// Regardless of the number of bits in a metadata unit, we always represent its content as a word.

Expand Down
Loading