Skip to content

Commit

Permalink
Use to_address for SFT access (#1122)
Browse files Browse the repository at this point in the history
Changed convenient methods in `ObjectReference` so that if they use SFT,
we use `self.to_address()` to get the address instead of `self.0` which
returns the raw address.

Also removed `ObjectReference::value()` to force users to use the more
explicit `ObjectReference::to_raw_address()`.
  • Loading branch information
wks authored Apr 18, 2024
1 parent 1bfd558 commit e79e94e
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 29 deletions.
4 changes: 2 additions & 2 deletions src/memory_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,8 @@ pub fn handle_user_collection_request<VM: VMBinding>(mmtk: &MMTK<VM>, tls: VMMut
///
/// Arguments:
/// * `object`: The object reference to query.
pub fn is_live_object(object: ObjectReference) -> bool {
object.is_live()
pub fn is_live_object<VM: VMBinding>(object: ObjectReference) -> bool {
object.is_live::<VM>()
}

/// Check if `addr` is the address of an object reference to an MMTk object.
Expand Down
2 changes: 1 addition & 1 deletion src/policy/immix/immixspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ impl<VM: VMBinding> ImmixSpace<VM> {
);

queue.enqueue(new_object);
debug_assert!(new_object.is_live());
debug_assert!(new_object.is_live::<VM>());
self.unlog_object_if_needed(new_object);
new_object
}
Expand Down
29 changes: 12 additions & 17 deletions src/util/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,49 +546,44 @@ impl ObjectReference {
self.0 == 0
}

/// returns the ObjectReference
pub fn value(self) -> usize {
self.0
}

/// Is the object reachable, determined by the policy?
/// Note: Objects in ImmortalSpace may have `is_live = true` but are actually unreachable.
pub fn is_reachable(self) -> bool {
pub fn is_reachable<VM: VMBinding>(self) -> bool {
if self.is_null() {
false
} else {
unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_reachable(self)
unsafe { SFT_MAP.get_unchecked(self.to_address::<VM>()) }.is_reachable(self)
}
}

/// Is the object live, determined by the policy?
pub fn is_live(self) -> bool {
pub fn is_live<VM: VMBinding>(self) -> bool {
if self.0 == 0 {
false
} else {
unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_live(self)
unsafe { SFT_MAP.get_unchecked(self.to_address::<VM>()) }.is_live(self)
}
}

/// Can the object be moved?
pub fn is_movable(self) -> bool {
unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_movable()
pub fn is_movable<VM: VMBinding>(self) -> bool {
unsafe { SFT_MAP.get_unchecked(self.to_address::<VM>()) }.is_movable()
}

/// Get forwarding pointer if the object is forwarded.
pub fn get_forwarded_object(self) -> Option<Self> {
unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.get_forwarded_object(self)
pub fn get_forwarded_object<VM: VMBinding>(self) -> Option<Self> {
unsafe { SFT_MAP.get_unchecked(self.to_address::<VM>()) }.get_forwarded_object(self)
}

/// Is the object in any MMTk spaces?
pub fn is_in_any_space(self) -> bool {
unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_in_space(self)
pub fn is_in_any_space<VM: VMBinding>(self) -> bool {
unsafe { SFT_MAP.get_unchecked(self.to_address::<VM>()) }.is_in_space(self)
}

/// Is the object sane?
#[cfg(feature = "sanity")]
pub fn is_sane(self) -> bool {
unsafe { SFT_MAP.get_unchecked(Address(self.0)) }.is_sane()
pub fn is_sane<VM: VMBinding>(self) -> bool {
unsafe { SFT_MAP.get_unchecked(self.to_address::<VM>()) }.is_sane()
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/util/finalizable_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<F: Finalizable> FinalizableProcessor<F> {
for mut f in self.candidates.drain(start..).collect::<Vec<F>>() {
let reff = f.get_reference();
trace!("Pop {:?} for finalization", reff);
if reff.is_live() {
if reff.is_live::<E::VM>() {
FinalizableProcessor::<F>::forward_finalizable_reference(e, &mut f);
trace!("{:?} is live, push {:?} back to candidates", reff, f);
self.candidates.push(f);
Expand Down
14 changes: 7 additions & 7 deletions src/util/reference_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,11 @@ impl ReferenceProcessor {
// For references in the table, the reference needs to be valid, and if the referent is not null, it should be valid as well
sync.references.iter().for_each(|reff| {
debug_assert!(!reff.is_null());
debug_assert!(reff.is_in_any_space());
debug_assert!(reff.is_in_any_space::<VM>());
let referent = VM::VMReferenceGlue::get_referent(*reff);
if !VM::VMReferenceGlue::is_referent_cleared(referent) {
debug_assert!(
referent.is_in_any_space(),
referent.is_in_any_space::<VM>(),
"Referent {:?} (of reference {:?}) is not in any space",
referent,
reff
Expand All @@ -260,7 +260,7 @@ impl ReferenceProcessor {
// For references that will be enqueue'd, the referent needs to be valid, and the referent needs to be null.
sync.enqueued_references.iter().for_each(|reff| {
debug_assert!(!reff.is_null());
debug_assert!(reff.is_in_any_space());
debug_assert!(reff.is_in_any_space::<VM>());
let referent = VM::VMReferenceGlue::get_referent(*reff);
debug_assert!(VM::VMReferenceGlue::is_referent_cleared(referent));
});
Expand Down Expand Up @@ -397,7 +397,7 @@ impl ReferenceProcessor {

trace!("Processing reference: {:?}", reference);

if !reference.is_live() {
if !reference.is_live::<E::VM>() {
// Reference is currently unreachable but may get reachable by the
// following trace. We postpone the decision.
continue;
Expand Down Expand Up @@ -433,7 +433,7 @@ impl ReferenceProcessor {

// If the reference is dead, we're done with it. Let it (and
// possibly its referent) be garbage-collected.
if !reference.is_live() {
if !reference.is_live::<E::VM>() {
<E::VM as VMBinding>::VMReferenceGlue::clear_referent(reference);
trace!(" UNREACHABLE reference: {}", reference);
trace!(" (unreachable)");
Expand All @@ -456,11 +456,11 @@ impl ReferenceProcessor {

trace!(" => {}", new_reference);

if old_referent.is_live() {
if old_referent.is_live::<E::VM>() {
// Referent is still reachable in a way that is as strong as
// or stronger than the current reference level.
let new_referent = Self::get_forwarded_referent(trace, old_referent);
debug_assert!(new_referent.is_live());
debug_assert!(new_referent.is_live::<E::VM>());
trace!(" ~> {}", new_referent);

// The reference object stays on the waiting list, and the
Expand Down
2 changes: 1 addition & 1 deletion src/util/sanity/sanity_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl<VM: VMBinding> ProcessEdgesWork for SanityGCProcessEdges<VM> {
let mut sanity_checker = self.mmtk().sanity_checker.lock().unwrap();
if !sanity_checker.refs.contains(&object) {
// FIXME steveb consider VM-specific integrity check on reference.
assert!(object.is_sane(), "Invalid reference {:?}", object);
assert!(object.is_sane::<VM>(), "Invalid reference {:?}", object);

// Let plan check object
assert!(
Expand Down

0 comments on commit e79e94e

Please sign in to comment.