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

Mechanism to track currently-dirtiest region. #361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dsm9000
Copy link
Contributor

@dsm9000 dsm9000 commented Jan 30, 2024

Part 1 of 2 to resolve #270 .

@dsm9000 dsm9000 force-pushed the track_dirtiest_region branch from 47f5af4 to 8dc7f99 Compare January 30, 2024 04:41
Comment on lines +686 to +689
freeRun(addresses[0 .. 2]);
assert(regionAllocator.dirtyBlockCount == 11);
verifyUniqueRegion(addresses[0], 2, 2, 2);
verifyDirtiestRegion(addresses[10], 5, 5);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the order changed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it so as to test the region at addresses[10] remaining the longest dirty region at the end.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It needed a case where a region is freed, but the currently-dirtiest region does not change. The re-ordering gives this.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's generally better to add new tests cases for what we want to test, as it prove none of the existing things are broken. Nevertheless, I get it now.

That being said, it seems that every single call to verifyUniqueRegion is now followed by a call to verifyDirtiestRegion. Then why have two functions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keep track of dirty regions
2 participants