Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Allow setting object metadata for VM space objects. Expose VO bit under a feature. #1248
Allow setting object metadata for VM space objects. Expose VO bit under a feature. #1248
Changes from 3 commits
8087bef
021a01d
21d008c
ed4ea46
ba27407
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the
mmtk::util::metadata::vo_bit
module ispub(crate)
, andset_vo_bit_for_vm_space_object
is not re-exported. So this method is still inaccessible to the VM binding although it ispub
.I suggest we make the
vo_bit
modulepub
, but make everypub
items inside thevo_bit
modulepub(crate)
, and makeset_vo_bit_for_vm_space_object
pub
. Then the VM binding can call this function bymmtk::util::metadata::vo_bit::set_vo_bit_for_vm_space_object
.I intended to do this refactoring anyway, and remove the
is_mmtk_object
feature, and move everything related tois_mmtk_object
intovo_bit
itself (see this issue).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the
vo_bit
module public. All the existing functions withpub
are changed topub(crate)
.VO_BIT_SIDE_METADATA_SPEC
is exported from this module if the featurevo_bit_access
is enabled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I removed this function
set_vo_bit_for_vm_space_object
. I realized that a binding needs to set all the object metadata to properly initialize an object in the VM space. I added a functionMMTK::initialize_vm_space_object()
which callsSFT::initialize_object_metadata
onvm_space
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sets a single VO bit at a time. Depending on the density of objects in the VM space, if this is a bottleneck, we can provide another function that allows the VM binding to provide a bitmap directly, and memcpy into our VO bit metadata (in a single pass, or chunk by chunk).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but I feel this is out of scope for this PR.