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

Prepare plan before compacting in MarkCompact #527

Closed
wants to merge 4 commits into from

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Jan 20, 2022

This PR closes #520.

This PR

  1. adds a proper max_non_los_default_alloc_bytes for mark compact
  2. adds a prepare() call for mark compact before the compact transitive closure
  3. adds some assertions to check if the two traces in mark compact traverse the same set of objects
  4. removes an assertion from LOS prepare

@qinsoon qinsoon added the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label Jan 20, 2022
@qinsoon qinsoon marked this pull request as ready for review January 21, 2022 01:14
@@ -153,7 +153,6 @@ impl<VM: VMBinding> LargeObjectSpace<VM> {

pub fn prepare(&mut self, full_heap: bool) {
if full_heap {
debug_assert!(self.treadmill.from_space_empty());
self.mark_state = MARK_BIT - self.mark_state;
}
self.treadmill.flip(full_heap);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel that flip the from/to space will leak dead objects. After the first trace, live objects have been copied to the to space and what left in from space are dead objects. Now after the flip, the from space will become the to space, and those dead objects will be treated as live. At this stage, sweep has not been executed yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. I will work on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

#944 fixes the issues by calling release before preparing it again.

@qinsoon
Copy link
Member Author

qinsoon commented Sep 6, 2023

#944 fixes the issue.

@qinsoon qinsoon closed this Sep 6, 2023
@qinsoon qinsoon deleted the fix-mark-compact-los-trace branch January 12, 2024 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plans with multiple traces
2 participants