Update MMTk core PR #817 (alternative approach) (#220)

This is an alternative approach for #218 that addresses the upstream API change in mmtk/mmtk-core#817.  This PR simply saves the mutator pointers into a `VecDeque` and embeds it into the iterator.

Author: wks

mmtk/Cargo.lock

@@ -30,7 +30,7 @@ once_cell = "1.10.0"
 # - change branch
 # - change repo name
 # But other changes including adding/removing whitespaces in commented lines may break the CI.
-mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "df146b7af6cf41cc7d6996e1ca538fd2b32950f5" }
+mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "de8bbfe623eee5e915f32870de8f81128df8ad41" }
 # Uncomment the following to build locally
 # mmtk = { path = "../repos/mmtk-core" }
 

mmtk/Cargo.toml

@@ -1,11 +1,41 @@
+use crate::MutatorClosure;
 use crate::OpenJDK;
 use crate::SINGLETON;
 use crate::UPCALLS;
 use mmtk::util::opaque_pointer::*;
 use mmtk::vm::ActivePlan;
 use mmtk::Mutator;
 use mmtk::Plan;
-use std::sync::Mutex;
+use std::collections::VecDeque;
+use std::marker::PhantomData;
+
+struct OpenJDKMutatorIterator<'a> {
+    mutators: VecDeque<&'a mut Mutator<OpenJDK>>,
+    phantom_data: PhantomData<&'a ()>,
+}
+
+impl<'a> OpenJDKMutatorIterator<'a> {
+    fn new() -> Self {
+        let mut mutators = VecDeque::new();
+        unsafe {
+            ((*UPCALLS).get_mutators)(MutatorClosure::from_rust_closure(&mut |mutator| {
+                mutators.push_back(mutator);
+            }));
+        }
+        Self {
+            mutators,
+            phantom_data: PhantomData,
+        }
+    }
+}
+
+impl<'a> Iterator for OpenJDKMutatorIterator<'a> {
+    type Item = &'a mut Mutator<OpenJDK>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.mutators.pop_front()
+    }
+}
 
 pub struct VMActivePlan {}
 
@@ -25,29 +55,11 @@ impl ActivePlan<OpenJDK> for VMActivePlan {
         }
     }
 
-    fn reset_mutator_iterator() {
-        unsafe {
-            ((*UPCALLS).reset_mutator_iterator)();
-        }
-    }
-
-    fn get_next_mutator() -> Option<&'static mut Mutator<OpenJDK>> {
-        let _guard = MUTATOR_ITERATOR_LOCK.lock().unwrap();
-        unsafe {
-            let m = ((*UPCALLS).get_next_mutator)();
-            if m.is_null() {
-                None
-            } else {
-                Some(&mut *m)
-            }
-        }
+    fn mutators<'a>() -> Box<dyn Iterator<Item = &'a mut Mutator<OpenJDK>> + 'a> {
+        Box::new(OpenJDKMutatorIterator::new())
     }
 
     fn number_of_mutators() -> usize {
         unsafe { ((*UPCALLS).number_of_mutators)() }
     }
 }
-
-lazy_static! {
-    pub static ref MUTATOR_ITERATOR_LOCK: Mutex<()> = Mutex::new(());
-}

mmtk/src/active_plan.rs

@@ -8,26 +8,6 @@ use crate::{MutatorClosure, OpenJDK};
 
 pub struct VMCollection {}
 
-extern "C" fn report_mutator_stop<F>(
-    mutator: *mut Mutator<OpenJDK>,
-    callback_ptr: *mut libc::c_void,
-) where
-    F: FnMut(&'static mut Mutator<OpenJDK>),
-{
-    let callback: &mut F = unsafe { &mut *(callback_ptr as *mut F) };
-    callback(unsafe { &mut *mutator });
-}
-
-fn to_mutator_closure<F>(callback: &mut F) -> MutatorClosure
-where
-    F: FnMut(&'static mut Mutator<OpenJDK>),
-{
-    MutatorClosure {
-        func: report_mutator_stop::<F>,
-        data: callback as *mut F as *mut libc::c_void,
-    }
-}
-
 const GC_THREAD_KIND_CONTROLLER: libc::c_int = 0;
 const GC_THREAD_KIND_WORKER: libc::c_int = 1;
 
@@ -43,7 +23,7 @@ impl Collection<OpenJDK> for VMCollection {
             ((*UPCALLS).stop_all_mutators)(
                 tls,
                 scan_mutators_in_safepoint,
-                to_mutator_closure(&mut mutator_visitor),
+                MutatorClosure::from_rust_closure(&mut mutator_visitor),
             );
         }
     }

mmtk/src/collection.rs

@@ -39,6 +39,28 @@ pub struct MutatorClosure {
     pub data: *mut libc::c_void,
 }
 
+impl MutatorClosure {
+    fn from_rust_closure<F>(callback: &mut F) -> Self
+    where
+        F: FnMut(&'static mut Mutator<OpenJDK>),
+    {
+        Self {
+            func: Self::call_rust_closure::<F>,
+            data: callback as *mut F as *mut libc::c_void,
+        }
+    }
+
+    extern "C" fn call_rust_closure<F>(
+        mutator: *mut Mutator<OpenJDK>,
+        callback_ptr: *mut libc::c_void,
+    ) where
+        F: FnMut(&'static mut Mutator<OpenJDK>),
+    {
+        let callback: &mut F = unsafe { &mut *(callback_ptr as *mut F) };
+        callback(unsafe { &mut *mutator });
+    }
+}
+
 /// A closure for reporting root edges.  The C++ code should pass `data` back as the last argument.
 #[repr(C)]
 pub struct EdgesClosure {
@@ -62,8 +84,7 @@ pub struct OpenJDK_Upcalls {
     pub spawn_gc_thread: extern "C" fn(tls: VMThread, kind: libc::c_int, ctx: *mut libc::c_void),
     pub block_for_gc: extern "C" fn(),
     pub out_of_memory: extern "C" fn(tls: VMThread, err_kind: AllocationError),
-    pub get_next_mutator: extern "C" fn() -> *mut Mutator<OpenJDK>,
-    pub reset_mutator_iterator: extern "C" fn(),
+    pub get_mutators: extern "C" fn(closure: MutatorClosure),
     pub scan_object: extern "C" fn(trace: *mut c_void, object: ObjectReference, tls: OpaquePointer),
     pub dump_object: extern "C" fn(object: ObjectReference),
     pub get_object_size: extern "C" fn(object: ObjectReference) -> usize,

mmtk/src/lib.rs

@@ -149,8 +149,7 @@ typedef struct {
     void (*spawn_gc_thread) (void *tls, int kind, void *ctx);
     void (*block_for_gc) ();
     void (*out_of_memory) (void *tls, MMTkAllocationError err_kind);
-    void* (*get_next_mutator) ();
-    void (*reset_mutator_iterator) ();
+    void (*get_mutators) (MutatorClosure closure);
     void (*scan_object) (void* trace, void* object, void* tls);
     void (*dump_object) (void* object);
     size_t (*get_object_size) (void* object);

openjdk/mmtk.h

@@ -177,40 +177,13 @@ static bool mmtk_is_mutator(void* tls) {
   return ((Thread*) tls)->third_party_heap_collector == NULL;
 }
 
-template <class T>
-struct MaybeUninit {
-  MaybeUninit() {}
-  T* operator->() {
-    return (T*) &_data;
+static void mmtk_get_mutators(MutatorClosure closure) {
+  JavaThread *thr;
+  for (JavaThreadIteratorWithHandle jtiwh; thr = jtiwh.next();) {
+    closure.invoke(&thr->third_party_heap_mutator);
   }
-  T& operator*() {
-    return *((T*) &_data);
-  }
-private:
-  char _data[sizeof(T)];
-};
-
-static MaybeUninit<JavaThreadIteratorWithHandle> jtiwh;
-static bool mutator_iteration_start = true;
-
-static void* mmtk_get_next_mutator() {
-  if (mutator_iteration_start) {
-    *jtiwh = JavaThreadIteratorWithHandle();
-    mutator_iteration_start = false;
-  }
-  JavaThread *thr = jtiwh->next();
-  if (thr == NULL) {
-    mutator_iteration_start = true;
-    return NULL;
-  }
-  return (void*) &thr->third_party_heap_mutator;
-}
-
-static void mmtk_reset_mutator_iterator() {
-  mutator_iteration_start = true;
 }
 
-
 static void mmtk_scan_all_thread_roots(EdgesClosure closure) {
   MMTkRootsClosure2 cl(closure);
   MMTkHeap::heap()->scan_thread_roots(cl);
@@ -347,8 +320,7 @@ OpenJDK_Upcalls mmtk_upcalls = {
   mmtk_spawn_gc_thread,
   mmtk_block_for_gc,
   mmtk_out_of_memory,
-  mmtk_get_next_mutator,
-  mmtk_reset_mutator_iterator,
+  mmtk_get_mutators,
   mmtk_scan_object,
   mmtk_dump_object,
   mmtk_get_object_size,