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

Allow setting object metadata for VM space objects. Expose VO bit under a feature. #1248

Merged
merged 5 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
10 changes: 10 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,16 @@ bpftrace_workaround = []
# users can disable such annotations by enabling this Cargo feature.
no_mmap_annotation = []

# Allow the binding to access Valid Object (VO) bit.
# MMTk uses VO bit to identify a valid object. Thus VO bit is carefully managed by MMTk in cooperation with the binding.
# So normally this feature is not needed, and its use should be discouraged.
# However, in rare cases, a binding may want to directly access and manipulate VO bit. For example, a binding cannot cooperate
# with MMTk to identify each object for setting the VO bit. This is usually due to the limitation of the VM.
# In such cases, a binding may use this feature and manipulate the VO bit to their needs. The binding's manipulation on VO bit
# may violate MMTk's semantics, and may result in undefined behaviors for VO bit related APIs. This feature should
# only be used if you understand how VO bit works internally. Use at your own risk.
vo_bit_access = []

# Do not modify the following line - ci-common.sh matches it
# -- Mutally exclusive features --
# Only one feature from each group can be provided. Otherwise build will fail.
Expand Down
1 change: 1 addition & 0 deletions src/util/metadata/side_metadata/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub(crate) const GLOBAL_SIDE_METADATA_BASE_OFFSET: SideMetadataOffset =
SideMetadataOffset::addr(GLOBAL_SIDE_METADATA_BASE_ADDRESS);

/// Base address of VO bit, public to VM bindings which may need to use this.
#[cfg(target_pointer_width = "64")]
pub const VO_BIT_SIDE_METADATA_ADDR: Address =
crate::util::metadata::vo_bit::VO_BIT_SIDE_METADATA_ADDR;

Expand Down
6 changes: 6 additions & 0 deletions src/util/metadata/side_metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ pub(crate) use helpers::*;
#[allow(unused_imports)]
pub(crate) use helpers_32::*;
pub(crate) use sanity::SideMetadataSanity;

/// Side metadata spec for VO bit. This is only available when the feature "vo_bit_access" is enabled.
/// Check the comments on "vo_bit_access" in `Cargo.toml` before use. Use at your own risk.
#[cfg(feature = "vo_bit_access")]
pub const VO_BIT_SIDE_METADATA_SPEC: SideMetadataSpec =
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
crate::util::metadata::side_metadata::spec_defs::VO_BIT;
21 changes: 21 additions & 0 deletions src/util/metadata/vo_bit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ use crate::vm::VMBinding;
pub(crate) const VO_BIT_SIDE_METADATA_SPEC: SideMetadataSpec =
crate::util::metadata::side_metadata::spec_defs::VO_BIT;

#[cfg(target_pointer_width = "64")]
pub const VO_BIT_SIDE_METADATA_ADDR: Address = VO_BIT_SIDE_METADATA_SPEC.get_absolute_offset();

/// Atomically set the VO bit for an object.
Expand Down Expand Up @@ -222,3 +223,23 @@ pub fn is_internal_ptr_from_vo_bit<VM: VMBinding>(
pub unsafe fn is_vo_addr(addr: Address) -> bool {
VO_BIT_SIDE_METADATA_SPEC.load::<u8>(addr) != 0
}

#[cfg(feature = "vm_space")]
use crate::MMTK;

/// Set VO bit for a VM space object.
/// Objects in the VM space are allocated/managed by the binding. This functio provides a way for
/// the binding to set VO bit for an object in the space.
#[cfg(feature = "vm_space")]
pub fn set_vo_bit_for_vm_space_object<VM: VMBinding>(
Copy link
Collaborator

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 is pub(crate), and set_vo_bit_for_vm_space_object is not re-exported. So this method is still inaccessible to the VM binding although it is pub.

I suggest we make the vo_bit module pub, but make every pub items inside the vo_bit module pub(crate), and make set_vo_bit_for_vm_space_object pub. Then the VM binding can call this function by mmtk::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 to is_mmtk_object into vo_bit itself (see this issue).

Copy link
Member Author

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 with pub are changed to pub(crate). VO_BIT_SIDE_METADATA_SPEC is exported from this module if the feature vo_bit_access is enabled.

Copy link
Member Author

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 function MMTK::initialize_vm_space_object() which calls SFT::initialize_object_metadata on vm_space.

mmtk: &'static MMTK<VM>,
object: ObjectReference,
) {
use crate::policy::space::Space;
debug_assert!(
mmtk.get_plan().base().vm_space.in_space(object),
"{} is not in VM space",
object
);
set_vo_bit(object);
Copy link
Collaborator

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).

Copy link
Member Author

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.

}
Loading