diff --git a/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs b/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs index 29be4f6184..f80ff56f9a 100644 --- a/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs +++ b/docs/userguide/src/tutorial/code/mygc_semispace/gc_work.rs @@ -56,9 +56,7 @@ impl ProcessEdgesWork for MyGCProcessEdges { } fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { - if object.is_null() { - return object; - } + debug_assert!(!object.is_null()); let worker = self.worker(); let queue = &mut self.base.nodes; if self.plan.tospace().in_space(object) { diff --git a/docs/userguide/src/tutorial/mygc/ss/exercise_solution.md b/docs/userguide/src/tutorial/mygc/ss/exercise_solution.md index 4f4717ed77..cb751aa082 100644 --- a/docs/userguide/src/tutorial/mygc/ss/exercise_solution.md +++ b/docs/userguide/src/tutorial/mygc/ss/exercise_solution.md @@ -149,9 +149,7 @@ In `gc_work.rs`: the tospace and fromspace: ```rust fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { - if object.is_null() { - return object; - } + debug_assert!(!object.is_null()); // Add this! else if self.plan().youngspace().in_space(object) { diff --git a/src/plan/generational/gc_work.rs b/src/plan/generational/gc_work.rs index a5c38e84ea..3a01b3bcd4 100644 --- a/src/plan/generational/gc_work.rs +++ b/src/plan/generational/gc_work.rs @@ -36,9 +36,8 @@ impl + PlanTraceObject> ProcessEdg Self { plan, base } } fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { - if object.is_null() { - return object; - } + debug_assert!(!object.is_null()); + // We cannot borrow `self` twice in a call, so we extract `worker` as a local variable. let worker = self.worker(); self.plan @@ -46,9 +45,16 @@ impl + PlanTraceObject> ProcessEdg } fn process_edge(&mut self, slot: EdgeOf) { let object = slot.load(); + if object.is_null() { + return; + } let new_object = self.trace_object(object); debug_assert!(!self.plan.is_object_in_nursery(new_object)); - slot.store(new_object); + // Note: If `object` is a mature object, `trace_object` will not call `space.trace_object`, + // but will still return `object`. In that case, we don't need to write it back. + if new_object != object { + slot.store(new_object); + } } fn create_scan_work( diff --git a/src/policy/copyspace.rs b/src/policy/copyspace.rs index 8d08ec1507..8193c7c63c 100644 --- a/src/policy/copyspace.rs +++ b/src/policy/copyspace.rs @@ -217,6 +217,7 @@ impl CopySpace { worker: &mut GCWorker, ) -> ObjectReference { trace!("copyspace.trace_object(, {:?}, {:?})", object, semantics,); + debug_assert!(!object.is_null()); // If this is not from space, we do not need to trace it (the object has been copied to the tosapce) if !self.is_from_space() { diff --git a/src/policy/immix/immixspace.rs b/src/policy/immix/immixspace.rs index 3809f7bd24..bf72394c01 100644 --- a/src/policy/immix/immixspace.rs +++ b/src/policy/immix/immixspace.rs @@ -184,6 +184,7 @@ impl crate::policy::gc_work::PolicyTraceObject for ImmixSpace copy: Option, worker: &mut GCWorker, ) -> ObjectReference { + debug_assert!(!object.is_null()); if KIND == TRACE_KIND_TRANSITIVE_PIN { self.trace_object_without_moving(queue, object) } else if KIND == TRACE_KIND_DEFRAG { diff --git a/src/policy/immortalspace.rs b/src/policy/immortalspace.rs index 5eeebd58c9..1e4d19eefa 100644 --- a/src/policy/immortalspace.rs +++ b/src/policy/immortalspace.rs @@ -187,6 +187,7 @@ impl ImmortalSpace { queue: &mut Q, object: ObjectReference, ) -> ObjectReference { + debug_assert!(!object.is_null()); #[cfg(feature = "vo_bit")] debug_assert!( crate::util::metadata::vo_bit::is_vo_bit_set::(object), diff --git a/src/policy/largeobjectspace.rs b/src/policy/largeobjectspace.rs index ec6b2f7506..64a13c8f37 100644 --- a/src/policy/largeobjectspace.rs +++ b/src/policy/largeobjectspace.rs @@ -189,6 +189,7 @@ impl LargeObjectSpace { queue: &mut Q, object: ObjectReference, ) -> ObjectReference { + debug_assert!(!object.is_null()); #[cfg(feature = "vo_bit")] debug_assert!( crate::util::metadata::vo_bit::is_vo_bit_set::(object), diff --git a/src/policy/markcompactspace.rs b/src/policy/markcompactspace.rs index 693218b492..e6d332919e 100644 --- a/src/policy/markcompactspace.rs +++ b/src/policy/markcompactspace.rs @@ -130,6 +130,7 @@ impl crate::policy::gc_work::PolicyTraceObject for MarkCompac _copy: Option, _worker: &mut GCWorker, ) -> ObjectReference { + debug_assert!(!object.is_null()); debug_assert!( KIND != TRACE_KIND_TRANSITIVE_PIN, "MarkCompact does not support transitive pin trace." diff --git a/src/policy/marksweepspace/malloc_ms/global.rs b/src/policy/marksweepspace/malloc_ms/global.rs index 7454fbb287..8d42b74f0d 100644 --- a/src/policy/marksweepspace/malloc_ms/global.rs +++ b/src/policy/marksweepspace/malloc_ms/global.rs @@ -400,9 +400,7 @@ impl MallocSpace { queue: &mut Q, object: ObjectReference, ) -> ObjectReference { - if object.is_null() { - return object; - } + debug_assert!(!object.is_null()); assert!( self.in_space(object), diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 8d8eae7d0e..a533fcde33 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -241,9 +241,7 @@ impl MarkSweepSpace { queue: &mut Q, object: ObjectReference, ) -> ObjectReference { - if object.is_null() { - return object; - } + debug_assert!(!object.is_null()); debug_assert!( self.in_space(object), "Cannot mark an object {} that was not alloced by free list allocator.", diff --git a/src/scheduler/gc_work.rs b/src/scheduler/gc_work.rs index 1de11db2b3..692873eb84 100644 --- a/src/scheduler/gc_work.rs +++ b/src/scheduler/gc_work.rs @@ -273,11 +273,8 @@ pub(crate) struct ProcessEdgesWorkTracer { impl ObjectTracer for ProcessEdgesWorkTracer { /// Forward the `trace_object` call to the underlying `ProcessEdgesWork`, /// and flush as soon as the underlying buffer of `process_edges_work` is full. - /// - /// This function is inlined because `trace_object` is probably the hottest function in MMTk. - /// If this function is called in small closures, please profile the program and make sure the - /// closure is inlined, too. fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { + debug_assert!(!object.is_null()); let result = self.process_edges_work.trace_object(object); self.flush_if_full(); result @@ -663,8 +660,12 @@ pub trait ProcessEdgesWork: /// trace the object and store back the new object reference if necessary. fn process_edge(&mut self, slot: EdgeOf) { let object = slot.load(); + if object.is_null() { + return; + } let new_object = self.trace_object(object); - if Self::OVERWRITE_REFERENCE { + debug_assert!(!new_object.is_null()); + if Self::OVERWRITE_REFERENCE && new_object != object { slot.store(new_object); } } @@ -721,9 +722,7 @@ impl ProcessEdgesWork for SFTProcessEdges { fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { use crate::policy::sft::GCWorkerMutRef; - if object.is_null() { - return object; - } + debug_assert!(!object.is_null()); // Erase type parameter let worker = GCWorkerMutRef::new(self.worker()); @@ -997,9 +996,7 @@ impl + Plan, const KIND: TraceKin } fn trace_object(&mut self, object: ObjectReference) -> ObjectReference { - if object.is_null() { - return object; - } + debug_assert!(!object.is_null()); // We cannot borrow `self` twice in a call, so we extract `worker` as a local variable. let worker = self.worker(); self.plan @@ -1008,8 +1005,12 @@ impl + Plan, const KIND: TraceKin fn process_edge(&mut self, slot: EdgeOf) { let object = slot.load(); + if object.is_null() { + return; + } let new_object = self.trace_object(object); - if P::may_move_objects::() { + debug_assert!(!new_object.is_null()); + if P::may_move_objects::() && new_object != object { slot.store(new_object); } } diff --git a/src/util/reference_processor.rs b/src/util/reference_processor.rs index 9683048881..718235ff67 100644 --- a/src/util/reference_processor.rs +++ b/src/util/reference_processor.rs @@ -216,6 +216,7 @@ impl ReferenceProcessor { e: &mut E, referent: ObjectReference, ) -> ObjectReference { + debug_assert!(!referent.is_null()); e.trace_object(referent) } @@ -223,6 +224,7 @@ impl ReferenceProcessor { e: &mut E, object: ObjectReference, ) -> ObjectReference { + debug_assert!(!object.is_null()); e.trace_object(object) } @@ -230,6 +232,7 @@ impl ReferenceProcessor { e: &mut E, referent: ObjectReference, ) -> ObjectReference { + debug_assert!(!referent.is_null()); e.trace_object(referent) } @@ -285,9 +288,6 @@ impl ReferenceProcessor { reference: ObjectReference, ) -> ObjectReference { let old_referent = ::VMReferenceGlue::get_referent(reference); - let new_referent = ReferenceProcessor::get_forwarded_referent(trace, old_referent); - ::VMReferenceGlue::set_referent(reference, new_referent); - let new_reference = ReferenceProcessor::get_forwarded_reference(trace, reference); { use crate::vm::ObjectModel; trace!( @@ -295,13 +295,22 @@ impl ReferenceProcessor { reference, ::VMObjectModel::get_current_size(reference) ); + } + + if !::VMReferenceGlue::is_referent_cleared(old_referent) { + let new_referent = ReferenceProcessor::get_forwarded_referent(trace, old_referent); + ::VMReferenceGlue::set_referent(reference, new_referent); + trace!( " referent: {} (forwarded to {})", old_referent, new_referent ); - trace!(" reference: forwarded to {}", new_reference); } + + let new_reference = ReferenceProcessor::get_forwarded_reference(trace, reference); + trace!(" reference: forwarded to {}", new_reference); + debug_assert!( !new_reference.is_null(), "reference {:?}'s forwarding pointer is NULL", diff --git a/src/vm/edge_shape.rs b/src/vm/edge_shape.rs index dd23efd393..45e7141c25 100644 --- a/src/vm/edge_shape.rs +++ b/src/vm/edge_shape.rs @@ -7,8 +7,13 @@ use atomic::Atomic; use crate::util::constants::{BYTES_IN_ADDRESS, LOG_BYTES_IN_ADDRESS}; use crate::util::{Address, ObjectReference}; -/// An abstract edge. An edge holds an object reference. When we load from it, we get an -/// ObjectReference; we can also store an ObjectReference into it. +/// An `Edge` represents a slot in an object (a.k.a. a field), on the stack (i.e. a local variable) +/// or any other places (such as global variables). A slot may hold an object reference. We can +/// load the object reference from it, and we can store an ObjectReference into it. For some VMs, +/// a slot may sometimes not hold an object reference. For example, it can hold a special `NULL` +/// pointer which does not point to any object, or it can hold a tagged non-reference value, such +/// as small integers and special values such as `true`, `false`, `null` (a.k.a. "none", "nil", +/// etc. for other VMs), `undefined`, etc. /// /// This intends to abstract out the differences of reference field representation among different /// VMs. If the VM represent a reference field as a word that holds the pointer to the object, it @@ -40,10 +45,31 @@ use crate::util::{Address, ObjectReference}; /// semantics, such as whether it holds strong or weak references. If a VM holds a weak reference /// in a word as a pointer, it can also use `SimpleEdge` for weak reference fields. pub trait Edge: Copy + Send + Debug + PartialEq + Eq + Hash { - /// Load object reference from the edge. + /// Load object reference from the slot. + /// + /// If the slot is not holding an object reference (For example, if it is holding NULL or a + /// tagged non-reference value. See trait-level doc comment.), this method should return + /// `ObjectReference::NULL`. + /// + /// If the slot holds an object reference with tag bits, the returned value shall be the object + /// reference with the tag bits removed. fn load(&self) -> ObjectReference; - /// Store the object reference `object` into the edge. + /// Store the object reference `object` into the slot. + /// + /// If the slot holds an object reference with tag bits, this method must preserve the tag + /// bits while updating the object reference so that it points to the forwarded object given by + /// the parameter `object`. + /// + /// FIXME: This design is inefficient for handling object references with tag bits. Consider + /// introducing a new updating function to do the load, trace and store in one function. + /// See: + /// + /// FIXME: This method is currently used by both moving GC algorithms and the subsuming write + /// barrier ([`crate::memory_manager::object_reference_write`]). The two reference writing + /// operations have different semantics, and need to be implemented differently if the VM + /// supports offsetted or tagged references. + /// See: fn store(&self, object: ObjectReference); /// Prefetch the edge so that a subsequent `load` will be faster. diff --git a/src/vm/scanning.rs b/src/vm/scanning.rs index 44ae056190..21768f4337 100644 --- a/src/vm/scanning.rs +++ b/src/vm/scanning.rs @@ -26,11 +26,14 @@ impl EdgeVisitor for F { /// Callback trait of scanning functions that directly trace through edges. pub trait ObjectTracer { - /// Call this function for the content of each edge, - /// and assign the returned value back to the edge. + /// Call this function to trace through an object graph edge which points to `object`. + /// `object` must point to a valid object, and cannot be `ObjectReference::NULL`. /// - /// Note: This function is performance-critical. - /// Implementations should consider inlining if necessary. + /// The return value is the new object reference for `object` if it is moved, or `object` if + /// not moved. If moved, the caller should update the slot that holds the reference to + /// `object` so that it points to the new location. + /// + /// Note: This function is performance-critical, therefore must be implemented efficiently. fn trace_object(&mut self, object: ObjectReference) -> ObjectReference; }