Skip to content

Conversation

@qinsoon
Copy link
Member

@qinsoon qinsoon commented Apr 12, 2024

This PR refactors mark sweep.

@qinsoon qinsoon force-pushed the fix-ms-block-list-race branch from cd28069 to 44fb3f4 Compare April 12, 2024 05:55
@qinsoon qinsoon added the PR-extended-testing Run extended tests for the pull request label Apr 12, 2024
Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

I added some comments. One strange thing is that once a Block becomes BlockState::Marked, it remain marked forever. But I think the block state of all marked blocks should be reset to unmarked so that we can find blocks that actually have marked objects.

@wks
Copy link
Collaborator

wks commented Apr 16, 2024

Style check failed due to trailing spaces. If you use VSCode, there are extensions for highlighting trailing spaces and removing them.

Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

The field BlockList::lock and its methods BlockList::lock() and BlockList::unlock() are unused. We can remove them now.

@qinsoon
Copy link
Member Author

qinsoon commented Apr 17, 2024

The field BlockList::lock and its methods BlockList::lock() and BlockList::unlock() are unused. We can remove them now.

I kept the methods just in case we may need them in the future.

Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

LGTM

@qinsoon qinsoon added this pull request to the merge queue Apr 17, 2024
Merged via the queue into mmtk:master with commit 34dc0cc Apr 17, 2024
@qinsoon qinsoon deleted the fix-ms-block-list-race branch April 17, 2024 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR-extended-testing Run extended tests for the pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants