Split the TransitiveClosure trait (#566)

* Split the TransitiveClosure trait

Moved the `process_edge` function of the `TransitiveClosure` trait into
a dedicated trait `EdgeVisitor` for visiting edges of an object (or
stack roots).  As a result,

1.  VM bindings only require the `EdgeVisitor` trait for object
    scanning.  VM bindings no longer use the `TransitiveClosure` in any
    way.

2.  Now `TransitiveClosure` only serves one purpose in `mmtk-core`, i.e.
    as a super-trait of ProcessEdgesWork for the `process_node` method.
    It is only called by `trace_object` of different spaces.

With this change, it will be easy to refactor `trace_object` and remove
`TransitiveClosure` completely.

* Fix scan_object{,s} API

Now both scan_object and scan_objects take EdgeVisitor as parameter.

We now implement scan_objects in mmtk-core.  We no longer expect the
VM bindings to create ProcessEdgesWork in scan_objects.

Author: wks

src/plan/transitive_closure.rs

@@ -5,22 +5,18 @@ use std::mem;
 use crate::scheduler::gc_work::ProcessEdgesWork;
 use crate::scheduler::{GCWorker, WorkBucketStage};
 use crate::util::{Address, ObjectReference};
-use crate::MMTK;
+use crate::vm::EdgeVisitor;
 
 /// This trait is the fundamental mechanism for performing a
 /// transitive closure over an object graph.
 pub trait TransitiveClosure {
     // The signature of this function changes during the port
     // because the argument `ObjectReference source` is never used in the original version
     // See issue #5
-    fn process_edge(&mut self, slot: Address);
     fn process_node(&mut self, object: ObjectReference);
 }
 
 impl<T: ProcessEdgesWork> TransitiveClosure for T {
-    fn process_edge(&mut self, _slot: Address) {
-        unreachable!();
-    }
     #[inline]
     fn process_node(&mut self, object: ObjectReference) {
         ProcessEdgesWork::process_node(self, object);
@@ -29,28 +25,31 @@ impl<T: ProcessEdgesWork> TransitiveClosure for T {
 
 /// A transitive closure visitor to collect all the edges of an object.
 pub struct ObjectsClosure<'a, E: ProcessEdgesWork> {
-    mmtk: &'static MMTK<E::VM>,
     buffer: Vec<Address>,
     worker: &'a mut GCWorker<E::VM>,
 }
 
 impl<'a, E: ProcessEdgesWork> ObjectsClosure<'a, E> {
-    pub fn new(
-        mmtk: &'static MMTK<E::VM>,
-        buffer: Vec<Address>,
-        worker: &'a mut GCWorker<E::VM>,
-    ) -> Self {
+    pub fn new(worker: &'a mut GCWorker<E::VM>) -> Self {
         Self {
-            mmtk,
-            buffer,
+            buffer: vec![],
             worker,
         }
     }
+
+    fn flush(&mut self) {
+        let mut new_edges = Vec::new();
+        mem::swap(&mut new_edges, &mut self.buffer);
+        self.worker.add_work(
+            WorkBucketStage::Closure,
+            E::new(new_edges, false, self.worker.mmtk),
+        );
+    }
 }
 
-impl<'a, E: ProcessEdgesWork> TransitiveClosure for ObjectsClosure<'a, E> {
+impl<'a, E: ProcessEdgesWork> EdgeVisitor for ObjectsClosure<'a, E> {
     #[inline(always)]
-    fn process_edge(&mut self, slot: Address) {
+    fn visit_edge(&mut self, slot: Address) {
         if self.buffer.is_empty() {
             self.buffer.reserve(E::CAPACITY);
         }
@@ -60,23 +59,15 @@ impl<'a, E: ProcessEdgesWork> TransitiveClosure for ObjectsClosure<'a, E> {
             mem::swap(&mut new_edges, &mut self.buffer);
             self.worker.add_work(
                 WorkBucketStage::Closure,
-                E::new(new_edges, false, self.mmtk),
+                E::new(new_edges, false, self.worker.mmtk),
             );
         }
     }
-    fn process_node(&mut self, _object: ObjectReference) {
-        unreachable!()
-    }
 }
 
 impl<'a, E: ProcessEdgesWork> Drop for ObjectsClosure<'a, E> {
     #[inline(always)]
     fn drop(&mut self) {
-        let mut new_edges = Vec::new();
-        mem::swap(&mut new_edges, &mut self.buffer);
-        self.worker.add_work(
-            WorkBucketStage::Closure,
-            E::new(new_edges, false, self.mmtk),
-        );
+        self.flush();
     }
 }

src/policy/immix/immixspace.rs

@@ -592,15 +592,12 @@ impl<Edges: ProcessEdgesWork> ScanObjectsAndMarkLines<Edges> {
 }
 
 impl<E: ProcessEdgesWork> GCWork<E::VM> for ScanObjectsAndMarkLines<E> {
-    fn do_work(&mut self, worker: &mut GCWorker<E::VM>, mmtk: &'static MMTK<E::VM>) {
+    fn do_work(&mut self, worker: &mut GCWorker<E::VM>, _mmtk: &'static MMTK<E::VM>) {
         trace!("ScanObjectsAndMarkLines");
-        let mut closure = ObjectsClosure::<E>::new(mmtk, vec![], worker);
+        let tls = worker.tls;
+        let mut closure = ObjectsClosure::<E>::new(worker);
         for object in &self.buffer {
-            <E::VM as VMBinding>::VMScanning::scan_object(
-                &mut closure,
-                *object,
-                VMWorkerThread(VMThread::UNINITIALIZED),
-            );
+            <E::VM as VMBinding>::VMScanning::scan_object(tls, *object, &mut closure);
             if super::MARK_LINE_AT_SCAN_TIME
                 && !super::BLOCK_ONLY
                 && self.immix_space.in_space(*object)

src/scheduler/gc_work.rs

@@ -1,6 +1,7 @@
 use super::work_bucket::WorkBucketStage;
 use super::*;
 use crate::plan::GcStatus;
+use crate::plan::ObjectsClosure;
 use crate::util::metadata::*;
 use crate::util::*;
 use crate::vm::*;
@@ -551,7 +552,11 @@ impl<Edges: ProcessEdgesWork> ScanObjects<Edges> {
 impl<E: ProcessEdgesWork> GCWork<E::VM> for ScanObjects<E> {
     fn do_work(&mut self, worker: &mut GCWorker<E::VM>, _mmtk: &'static MMTK<E::VM>) {
         trace!("ScanObjects");
-        <E::VM as VMBinding>::VMScanning::scan_objects::<E>(&self.buffer, worker);
+        {
+            let tls = worker.tls;
+            let mut closure = ObjectsClosure::<E>::new(worker);
+            <E::VM as VMBinding>::VMScanning::scan_objects(tls, &self.buffer, &mut closure);
+        }
         trace!("ScanObjects End");
     }
 }

src/scheduler/worker.rs

@@ -48,7 +48,7 @@ pub struct GCWorker<VM: VMBinding> {
     /// The sending end of the channel to send message to the controller thread.
     pub sender: Sender<CoordinatorMessage<VM>>,
     /// The reference to the MMTk instance.
-    mmtk: &'static MMTK<VM>,
+    pub mmtk: &'static MMTK<VM>,
     /// True if this struct is the embedded GCWorker of the controller thread.
     /// False if this struct belongs to a standalone GCWorker thread.
     is_coordinator: bool,

src/vm/mod.rs

@@ -28,6 +28,7 @@ pub use self::collection::GCThreadContext;
 pub use self::object_model::specs::*;
 pub use self::object_model::ObjectModel;
 pub use self::reference_glue::ReferenceGlue;
+pub use self::scanning::EdgeVisitor;
 pub use self::scanning::Scanning;
 
 /// The `VMBinding` trait associates with each trait, and provides VM-specific constants.

src/vm/scanning.rs

@@ -1,10 +1,16 @@
-use crate::plan::{Mutator, TransitiveClosure};
-use crate::scheduler::GCWorker;
+use crate::plan::Mutator;
 use crate::scheduler::ProcessEdgesWork;
-use crate::util::ObjectReference;
 use crate::util::VMWorkerThread;
+use crate::util::{Address, ObjectReference};
 use crate::vm::VMBinding;
 
+// Callback trait of scanning functions that report edges.
+pub trait EdgeVisitor {
+    /// Call this function for each edge.
+    fn visit_edge(&mut self, edge: Address);
+    // TODO: Add visit_soft_edge, visit_weak_edge, ... here.
+}
+
 /// VM-specific methods for scanning roots/objects.
 pub trait Scanning<VM: VMBinding> {
     /// Scan stack roots after all mutators are paused.
@@ -15,18 +21,17 @@ pub trait Scanning<VM: VMBinding> {
     /// `SCAN_MUTATORS_IN_SAFEPOINT` should also be enabled
     const SINGLE_THREAD_MUTATOR_SCANNING: bool = true;
 
-    /// Delegated scanning of a object, processing each pointer field
-    /// encountered. This method probably will be removed in the future,
-    /// in favor of bulk scanning `scan_objects`.
+    /// Delegated scanning of a object, visiting each pointer field
+    /// encountered.
     ///
     /// Arguments:
-    /// * `trace`: The `TransitiveClosure` to use for scanning.
+    /// * `tls`: The VM-specific thread-local storage for the current worker.
     /// * `object`: The object to be scanned.
-    /// * `tls`: The GC worker thread that is doing this tracing.
-    fn scan_object<T: TransitiveClosure>(
-        trace: &mut T,
-        object: ObjectReference,
+    /// * `edge_visitor`: Called back for each edge.
+    fn scan_object<EV: EdgeVisitor>(
         tls: VMWorkerThread,
+        object: ObjectReference,
+        edge_visitor: &mut EV,
     );
 
     /// MMTk calls this method at the first time during a collection that thread's stacks
@@ -41,11 +46,18 @@ pub trait Scanning<VM: VMBinding> {
     /// Bulk scanning of objects, processing each pointer field for each object.
     ///
     /// Arguments:
+    /// * `tls`: The VM-specific thread-local storage for the current worker.
     /// * `objects`: The slice of object references to be scanned.
-    fn scan_objects<W: ProcessEdgesWork<VM = VM>>(
+    /// * `edge_visitor`: Called back for each edge in each object in `objects`.
+    fn scan_objects<EV: EdgeVisitor>(
+        tls: VMWorkerThread,
         objects: &[ObjectReference],
-        worker: &mut GCWorker<VM>,
-    );
+        edge_visitor: &mut EV,
+    ) {
+        for object in objects.iter() {
+            Self::scan_object(tls, *object, edge_visitor);
+        }
+    }
 
     /// Scan all the mutators for roots.
     fn scan_thread_roots<W: ProcessEdgesWork<VM = VM>>();

vmbindings/dummyvm/src/scanning.rs

@@ -2,18 +2,13 @@ use crate::DummyVM;
 use mmtk::scheduler::*;
 use mmtk::util::opaque_pointer::*;
 use mmtk::util::ObjectReference;
+use mmtk::vm::EdgeVisitor;
 use mmtk::vm::Scanning;
-use mmtk::{Mutator, TransitiveClosure};
+use mmtk::Mutator;
 
 pub struct VMScanning {}
 
 impl Scanning<DummyVM> for VMScanning {
-    fn scan_objects<W: ProcessEdgesWork<VM = DummyVM>>(
-        _objects: &[ObjectReference],
-        _worker: &mut GCWorker<DummyVM>,
-    ) {
-        unimplemented!()
-    }
     fn scan_thread_roots<W: ProcessEdgesWork<VM = DummyVM>>() {
         unimplemented!()
     }
@@ -26,10 +21,10 @@ impl Scanning<DummyVM> for VMScanning {
     fn scan_vm_specific_roots<W: ProcessEdgesWork<VM = DummyVM>>() {
         unimplemented!()
     }
-    fn scan_object<T: TransitiveClosure>(
-        _trace: &mut T,
-        _object: ObjectReference,
+    fn scan_object<EV: EdgeVisitor>(
         _tls: VMWorkerThread,
+        _object: ObjectReference,
+        _edge_visitor: &mut EV,
     ) {
         unimplemented!()
     }