Skip to content

Optimize object forwarding for single threaded GC #1283

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,7 @@ analysis = []
nogc_lock_free = []
# Use lock free with no zeroing NoGC
nogc_no_zeroing = ["nogc_lock_free"]
# For using a single GC thread
# Q: Why do we need this as a compile time flat? We can always set the number of GC threads through options.
# For using a single GC thread. We optimize for single thread at certain places such as object forwarding
single_worker = []

# To run expensive comprehensive runtime checks, such as checking duplicate edges
Expand Down
165 changes: 113 additions & 52 deletions src/util/object_forwarding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,36 @@ const FORWARDING_POINTER_MASK: usize = 0xffff_fffc;
/// Attempt to become the worker thread who will forward the object.
/// The successful worker will set the object forwarding bits to BEING_FORWARDED, preventing other workers from forwarding the same object.
pub fn attempt_to_forward<VM: VMBinding>(object: ObjectReference) -> u8 {
loop {
if cfg!(not(feature = "single_worker")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having this if-else statement with not(feature = "single_worker") as the condition makes the code a bit harder to read. Since it is simpler to do it with a single worker, we can rewrite it in the early exit style. For example,

pub fn attempt_to_forward<VM: VMBinding>(object: ObjectReference) -> u8 {
    if cfg!(feature = "single_worker") {
        return get_forwarding_status::<VM>(object); // Assume we remove BEING_FORWARDED.
    }

    // old code here
}

loop {
let old_value = get_forwarding_status::<VM>(object);
if old_value != FORWARDING_NOT_TRIGGERED_YET
|| VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC
.compare_exchange_metadata::<VM, u8>(
object,
old_value,
BEING_FORWARDED,
None,
Ordering::SeqCst,
Ordering::Relaxed,
)
.is_ok()
{
return old_value;
}
}
} else {
let old_value = get_forwarding_status::<VM>(object);
if old_value != FORWARDING_NOT_TRIGGERED_YET
|| VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC
.compare_exchange_metadata::<VM, u8>(
if old_value == FORWARDING_NOT_TRIGGERED_YET {
unsafe {
VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store::<VM, u8>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This store can be removed. The caller of attempt_to_forward only needs the old_value. As long as it is FORWARDING_NOT_TRIGGERED_YET, the caller will immediately try to forward the object.

object,
old_value,
BEING_FORWARDED,
None,
Ordering::SeqCst,
Ordering::Relaxed,
)
.is_ok()
{
return old_value;
}
}
old_value
}
}

Expand All @@ -54,23 +68,33 @@ pub fn spin_and_get_forwarded_object<VM: VMBinding>(
forwarding_bits: u8,
) -> ObjectReference {
let mut forwarding_bits = forwarding_bits;
while forwarding_bits == BEING_FORWARDED {
forwarding_bits = get_forwarding_status::<VM>(object);
}
if cfg!(not(feature = "single_worker")) {
while forwarding_bits == BEING_FORWARDED {
forwarding_bits = get_forwarding_status::<VM>(object);
}

if forwarding_bits == FORWARDED {
read_forwarding_pointer::<VM>(object)
if forwarding_bits == FORWARDED {
read_forwarding_pointer::<VM>(object)
} else {
// For some policies (such as Immix), we can have interleaving such that one thread clears
// the forwarding word while another thread was stuck spinning in the above loop.
// See: https://github.com/mmtk/mmtk-core/issues/579
debug_assert!(
forwarding_bits == FORWARDING_NOT_TRIGGERED_YET,
"Invalid/Corrupted forwarding word {:x} for object {}",
forwarding_bits,
object,
);
object
}
} else {
// For some policies (such as Immix), we can have interleaving such that one thread clears
// the forwarding word while another thread was stuck spinning in the above loop.
// See: https://github.com/mmtk/mmtk-core/issues/579
debug_assert!(
forwarding_bits == FORWARDING_NOT_TRIGGERED_YET,
forwarding_bits != BEING_FORWARDED,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can just assert forwarding_bits == FORWARDED if we don't use BEING_FORWARDED.

"Invalid/Corrupted forwarding word {:x} for object {}",
forwarding_bits,
object,
);
object
read_forwarding_pointer::<VM>(object)
}
}

Expand Down Expand Up @@ -99,12 +123,22 @@ pub fn forward_object<VM: VMBinding>(
let new_object = VM::VMObjectModel::copy(object, semantics, copy_context);
on_after_forwarding(new_object);
if let Some(shift) = forwarding_bits_offset_in_forwarding_pointer::<VM>() {
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store_atomic::<VM, usize>(
object,
new_object.to_raw_address().as_usize() | ((FORWARDED as usize) << shift),
None,
Ordering::SeqCst,
)
if cfg!(not(feature = "single_worker")) {
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store_atomic::<VM, usize>(
object,
new_object.to_raw_address().as_usize() | ((FORWARDED as usize) << shift),
None,
Ordering::SeqCst,
)
} else {
unsafe {
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store::<VM, usize>(
object,
new_object.to_raw_address().as_usize() | ((FORWARDED as usize) << shift),
None,
)
}
}
} else {
write_forwarding_pointer::<VM>(object, new_object);
VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store_atomic::<VM, u8>(
Expand All @@ -119,11 +153,15 @@ pub fn forward_object<VM: VMBinding>(

/// Return the forwarding bits for a given `ObjectReference`.
pub fn get_forwarding_status<VM: VMBinding>(object: ObjectReference) -> u8 {
VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load_atomic::<VM, u8>(
object,
None,
Ordering::SeqCst,
)
if cfg!(not(feature = "single_worker")) {
VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load_atomic::<VM, u8>(
object,
None,
Ordering::SeqCst,
)
} else {
unsafe { VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load::<VM, u8>(object, None) }
}
}

pub fn is_forwarded<VM: VMBinding>(object: ObjectReference) -> bool {
Expand All @@ -149,12 +187,16 @@ pub fn state_is_being_forwarded(forwarding_bits: u8) -> bool {
/// Zero the forwarding bits of an object.
/// This function is used on new objects.
pub fn clear_forwarding_bits<VM: VMBinding>(object: ObjectReference) {
VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store_atomic::<VM, u8>(
object,
0,
None,
Ordering::SeqCst,
)
if cfg!(not(feature = "single_worker")) {
VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store_atomic::<VM, u8>(
object,
0,
None,
Ordering::SeqCst,
)
} else {
unsafe { VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store::<VM, u8>(object, 0, None) }
}
}

/// Read the forwarding pointer of an object.
Expand All @@ -168,15 +210,24 @@ pub fn read_forwarding_pointer<VM: VMBinding>(object: ObjectReference) -> Object

// We write the forwarding poiner. We know it is an object reference.
unsafe {
// We use "unchecked" convertion becasue we guarantee the forwarding pointer we stored
// previously is from a valid `ObjectReference` which is never zero.
ObjectReference::from_raw_address_unchecked(crate::util::Address::from_usize(
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.load_atomic::<VM, usize>(
object,
Some(FORWARDING_POINTER_MASK),
Ordering::SeqCst,
),
))
if cfg!(not(feature = "single_worker")) {
// We use "unchecked" convertion becasue we guarantee the forwarding pointer we stored
// previously is from a valid `ObjectReference` which is never zero.
ObjectReference::from_raw_address_unchecked(crate::util::Address::from_usize(
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.load_atomic::<VM, usize>(
object,
Some(FORWARDING_POINTER_MASK),
Ordering::SeqCst,
),
))
} else {
// We use "unchecked" convertion becasue we guarantee the forwarding pointer we stored
// previously is from a valid `ObjectReference` which is never zero.
ObjectReference::from_raw_address_unchecked(crate::util::Address::from_usize(
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC
.load::<VM, usize>(object, Some(FORWARDING_POINTER_MASK)),
))
}
}
}

Expand All @@ -194,12 +245,22 @@ pub fn write_forwarding_pointer<VM: VMBinding>(
);

trace!("write_forwarding_pointer({}, {})", object, new_object);
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store_atomic::<VM, usize>(
object,
new_object.to_raw_address().as_usize(),
Some(FORWARDING_POINTER_MASK),
Ordering::SeqCst,
)
if cfg!(not(feature = "single_worker")) {
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store_atomic::<VM, usize>(
object,
new_object.to_raw_address().as_usize(),
Some(FORWARDING_POINTER_MASK),
Ordering::SeqCst,
)
} else {
unsafe {
VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store::<VM, usize>(
object,
new_object.to_raw_address().as_usize(),
Some(FORWARDING_POINTER_MASK),
)
}
}
}

/// (This function is only used internal to the `util` module)
Expand Down
Loading