From 4eeac51f7f617bb55356fdf78ec9d40f5f3ba7ac Mon Sep 17 00:00:00 2001 From: Kunal Sareen Date: Mon, 17 Mar 2025 00:06:43 +0000 Subject: [PATCH 1/2] Optimize object forwarding for single threaded GC --- Cargo.toml | 3 +- src/util/object_forwarding.rs | 172 ++++++++++++++++++++++++---------- 2 files changed, 121 insertions(+), 54 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9fa74fa2f7..561c9ee1e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -156,8 +156,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 diff --git a/src/util/object_forwarding.rs b/src/util/object_forwarding.rs index 4b3f2411e9..c7a5a4ba24 100644 --- a/src/util/object_forwarding.rs +++ b/src/util/object_forwarding.rs @@ -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(object: ObjectReference) -> u8 { - loop { + if cfg!(not(feature = "single_worker")) { + loop { + let old_value = get_forwarding_status::(object); + if old_value != FORWARDING_NOT_TRIGGERED_YET + || VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC + .compare_exchange_metadata::( + object, + old_value, + BEING_FORWARDED, + None, + Ordering::SeqCst, + Ordering::Relaxed, + ) + .is_ok() + { + return old_value; + } + } + } else { let old_value = get_forwarding_status::(object); - if old_value != FORWARDING_NOT_TRIGGERED_YET - || VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC - .compare_exchange_metadata::( + if old_value == FORWARDING_NOT_TRIGGERED_YET { + unsafe { + VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store::( object, - old_value, BEING_FORWARDED, None, - Ordering::SeqCst, - Ordering::Relaxed, ) - .is_ok() - { - return old_value; + } } + old_value } } @@ -54,23 +68,33 @@ pub fn spin_and_get_forwarded_object( forwarding_bits: u8, ) -> ObjectReference { let mut forwarding_bits = forwarding_bits; - while forwarding_bits == BEING_FORWARDED { - forwarding_bits = get_forwarding_status::(object); - } + if cfg!(not(feature = "single_worker")) { + while forwarding_bits == BEING_FORWARDED { + forwarding_bits = get_forwarding_status::(object); + } - if forwarding_bits == FORWARDED { - read_forwarding_pointer::(object) + if forwarding_bits == FORWARDED { + read_forwarding_pointer::(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, "Invalid/Corrupted forwarding word {:x} for object {}", forwarding_bits, object, ); - object + read_forwarding_pointer::(object) } } @@ -99,12 +123,22 @@ pub fn forward_object( 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::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store_atomic::( - 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::( + object, + new_object.to_raw_address().as_usize() | ((FORWARDED as usize) << shift), + None, + Ordering::SeqCst, + ) + } else { + unsafe { + VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store::( + object, + new_object.to_raw_address().as_usize() | ((FORWARDED as usize) << shift), + None, + ) + } + } } else { write_forwarding_pointer::(object, new_object); VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store_atomic::( @@ -119,11 +153,20 @@ pub fn forward_object( /// Return the forwarding bits for a given `ObjectReference`. pub fn get_forwarding_status(object: ObjectReference) -> u8 { - VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load_atomic::( - object, - None, - Ordering::SeqCst, - ) + if cfg!(not(feature = "single_worker")) { + VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load_atomic::( + object, + None, + Ordering::SeqCst, + ) + } else { + unsafe { + VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load::( + object, + None, + ) + } + } } pub fn is_forwarded(object: ObjectReference) -> bool { @@ -149,12 +192,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(object: ObjectReference) { - VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store_atomic::( - object, - 0, - None, - Ordering::SeqCst, - ) + if cfg!(not(feature = "single_worker")) { + VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store_atomic::( + object, + 0, + None, + Ordering::SeqCst, + ) + } else { + unsafe { VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store::(object, 0, None) } + } } /// Read the forwarding pointer of an object. @@ -168,15 +215,26 @@ pub fn read_forwarding_pointer(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::( - 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::( + 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::( + object, + Some(FORWARDING_POINTER_MASK), + ), + )) + } } } @@ -194,12 +252,22 @@ pub fn write_forwarding_pointer( ); trace!("write_forwarding_pointer({}, {})", object, new_object); - VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store_atomic::( - 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::( + object, + new_object.to_raw_address().as_usize(), + Some(FORWARDING_POINTER_MASK), + Ordering::SeqCst, + ) + } else { + unsafe { + VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC.store::( + object, + new_object.to_raw_address().as_usize(), + Some(FORWARDING_POINTER_MASK), + ) + } + } } /// (This function is only used internal to the `util` module) From c88dfd770d1c754b88d2e19886cf37ea66642f3f Mon Sep 17 00:00:00 2001 From: Kunal Sareen Date: Mon, 17 Mar 2025 00:10:02 +0000 Subject: [PATCH 2/2] cargo fmt --- src/util/object_forwarding.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/util/object_forwarding.rs b/src/util/object_forwarding.rs index c7a5a4ba24..f29c85686e 100644 --- a/src/util/object_forwarding.rs +++ b/src/util/object_forwarding.rs @@ -160,12 +160,7 @@ pub fn get_forwarding_status(object: ObjectReference) -> u8 { Ordering::SeqCst, ) } else { - unsafe { - VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load::( - object, - None, - ) - } + unsafe { VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.load::(object, None) } } } @@ -229,10 +224,8 @@ pub fn read_forwarding_pointer(object: ObjectReference) -> Object // 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::( - object, - Some(FORWARDING_POINTER_MASK), - ), + VM::VMObjectModel::LOCAL_FORWARDING_POINTER_SPEC + .load::(object, Some(FORWARDING_POINTER_MASK)), )) } }