Moving to stable Rust (#147)

This PR removes use of nightly features.
* `once_cell`: replaced by the `once_cell` crate.
* `vec_into_raw_parts`: replaced by the actual implementation of the method in the binding.
* `box_syntax`: `box X` -> `Box::new(X)`.
* `specialization`: The code used to use `specialization` for type equality in traits `ArrayOopDesc<T>`. 
  * `ArrayOopDesc<T>` is used to describe different Java arrays. In OpenJDK's code base (C++), `arrayOopDesc` has subclasses `objArrayOopDesc` (for arrays of objects) and `typeArrayOopDesc` (for arrays of primitive types). In the binding, `ArrayOopDesc<Oop>` is `objArrayOopDesc`, and `ArrayOopDesc<f64/i64/etc>` is `typeArrayOopDesc`.
  * Now for type equality, an enum for types is used for where type equality is needed.
  * `ArrayOopsDesc` no longer takes any type parameter.
  * `ArrayOopsDesc::data() -> &[T]` takes a Rust type parameter that provides a Rust slice to access the array data.

Author: qinsoon

mmtk/Cargo.toml

@@ -20,13 +20,14 @@ openjdk_version = "67d5d2b16aacb2ea948552fab2323ebd0abbe924"
 [dependencies]
 libc = "0.2"
 lazy_static = "1.1"
+once_cell = "1.10.0"
 # Be very careful to commit any changes to the following mmtk dependency, as our CI scripts (including mmtk-core CI)
 # rely on matching these lines to modify them: e.g. comment out the git dependency and use the local path.
 # These changes are safe:
 # - 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 = "cd6d8984c10c294c991dcd5f154ce41073c06ab9" }
+mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "cacded40b7622e97dea56a83179aaa6af42920d0" }
 # Uncomment the following to build locally
 # mmtk = { path = "../repos/mmtk-core" }
 

mmtk/rust-toolchain

@@ -1 +1 @@
-nightly-2022-02-11
+stable

mmtk/src/abi.rs

@@ -5,25 +5,8 @@ use mmtk::util::ObjectReference;
 use mmtk::util::{Address, OpaquePointer};
 use std::ffi::CStr;
 use std::fmt;
-use std::marker::PhantomData;
 use std::{mem, slice};
 
-trait EqualTo<T> {
-    const VALUE: bool;
-}
-
-impl<T, U> EqualTo<U> for T {
-    default const VALUE: bool = false;
-}
-
-impl<T> EqualTo<T> for T {
-    const VALUE: bool = true;
-}
-
-pub const fn type_equal<T, U>() -> bool {
-    <T as EqualTo<U>>::VALUE
-}
-
 #[repr(i32)]
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 #[allow(dead_code)]
@@ -36,6 +19,30 @@ pub enum KlassID {
     ObjArray,
 }
 
+#[repr(i32)]
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[allow(dead_code)]
+#[allow(non_camel_case_types)]
+pub enum BasicType {
+    T_BOOLEAN = 4,
+    T_CHAR = 5,
+    T_FLOAT = 6,
+    T_DOUBLE = 7,
+    T_BYTE = 8,
+    T_SHORT = 9,
+    T_INT = 10,
+    T_LONG = 11,
+    T_OBJECT = 12,
+    T_ARRAY = 13,
+    T_VOID = 14,
+    T_ADDRESS = 15,
+    T_NARROWOOP = 16,
+    T_METADATA = 17,
+    T_NARROWKLASS = 18,
+    T_CONFLICT = 19, // for stack value type with conflicting contents
+    T_ILLEGAL = 99,
+}
+
 #[repr(C)]
 pub struct Klass {
     vptr: OpaquePointer,
@@ -296,8 +303,8 @@ impl From<&OopDesc> for ObjectReference {
 }
 
 impl OopDesc {
-    pub unsafe fn as_array_oop<T>(&self) -> ArrayOop<T> {
-        &*(self as *const OopDesc as *const ArrayOopDesc<T>)
+    pub unsafe fn as_array_oop(&self) -> ArrayOop {
+        &*(self as *const OopDesc as *const ArrayOopDesc)
     }
 
     pub fn get_field_address(&self, offset: i32) -> Address {
@@ -325,7 +332,7 @@ impl OopDesc {
         } else if lh <= Klass::LH_NEUTRAL_VALUE {
             if lh < Klass::LH_NEUTRAL_VALUE {
                 // Calculate array size
-                let array_length = self.as_array_oop::<()>().length();
+                let array_length = self.as_array_oop().length();
                 let mut size_in_bytes: usize =
                     (array_length as usize) << Klass::layout_helper_log2_element_size(lh);
                 size_in_bytes += Klass::layout_helper_header_size(lh) as usize;
@@ -340,17 +347,21 @@ impl OopDesc {
 }
 
 #[repr(C)]
-pub struct ArrayOopDesc<T>(OopDesc, PhantomData<T>);
+pub struct ArrayOopDesc(OopDesc);
 
-pub type ArrayOop<T> = &'static ArrayOopDesc<T>;
+pub type ArrayOop = &'static ArrayOopDesc;
 
-impl<T> ArrayOopDesc<T> {
-    const ELEMENT_TYPE_SHOULD_BE_ALIGNED: bool = type_equal::<T, f64>() || type_equal::<T, i64>();
+impl ArrayOopDesc {
     const LENGTH_OFFSET: usize = mem::size_of::<Self>();
-    fn header_size() -> usize {
+
+    fn element_type_should_be_aligned(ty: BasicType) -> bool {
+        ty == BasicType::T_DOUBLE || ty == BasicType::T_LONG
+    }
+
+    fn header_size(ty: BasicType) -> usize {
         let typesize_in_bytes =
             conversions::raw_align_up(Self::LENGTH_OFFSET + BYTES_IN_INT, BYTES_IN_LONG);
-        if Self::ELEMENT_TYPE_SHOULD_BE_ALIGNED {
+        if Self::element_type_should_be_aligned(ty) {
             conversions::raw_align_up(typesize_in_bytes / BYTES_IN_WORD, BYTES_IN_LONG)
         } else {
             typesize_in_bytes / BYTES_IN_WORD
@@ -359,12 +370,16 @@ impl<T> ArrayOopDesc<T> {
     fn length(&self) -> i32 {
         unsafe { *((self as *const _ as *const u8).add(Self::LENGTH_OFFSET) as *const i32) }
     }
-    fn base(&self) -> *const T {
-        let base_offset_in_bytes = Self::header_size() * BYTES_IN_WORD;
-        unsafe { (self as *const _ as *const u8).add(base_offset_in_bytes) as _ }
+    fn base(&self, ty: BasicType) -> Address {
+        let base_offset_in_bytes = Self::header_size(ty) * BYTES_IN_WORD;
+        Address::from_ptr(unsafe { (self as *const _ as *const u8).add(base_offset_in_bytes) })
     }
-    pub fn data(&self) -> &[T] {
-        unsafe { slice::from_raw_parts(self.base(), self.length() as _) }
+    // This provides an easy way to access the array data in Rust. However, the array data
+    // is Java types, so we have to map Java types to Rust types. The caller needs to guarantee:
+    // 1. <T> matches the actual Java type
+    // 2. <T> matches the argument, BasicType `ty`
+    pub unsafe fn data<T>(&self, ty: BasicType) -> &[T] {
+        slice::from_raw_parts(self.base(ty).to_ptr(), self.length() as _)
     }
 }
 

mmtk/src/api.rs

@@ -14,12 +14,13 @@ use mmtk::AllocationSemantics;
 use mmtk::Mutator;
 use mmtk::MutatorContext;
 use mmtk::MMTK;
+use once_cell::sync;
 use std::ffi::{CStr, CString};
-use std::lazy::SyncLazy;
 
 // Supported barriers:
-static NO_BARRIER: SyncLazy<CString> = SyncLazy::new(|| CString::new("NoBarrier").unwrap());
-static OBJECT_BARRIER: SyncLazy<CString> = SyncLazy::new(|| CString::new("ObjectBarrier").unwrap());
+static NO_BARRIER: sync::Lazy<CString> = sync::Lazy::new(|| CString::new("NoBarrier").unwrap());
+static OBJECT_BARRIER: sync::Lazy<CString> =
+    sync::Lazy::new(|| CString::new("ObjectBarrier").unwrap());
 
 #[no_mangle]
 pub extern "C" fn mmtk_active_barrier() -> *const c_char {

mmtk/src/lib.rs

@@ -1,14 +1,8 @@
-// specialization is considered as an incomplete feature.
-#![allow(incomplete_features)]
-#![feature(specialization)]
-#![feature(box_syntax)]
-#![feature(vec_into_raw_parts)]
-#![feature(once_cell)]
-
 extern crate libc;
 extern crate mmtk;
 #[macro_use]
 extern crate lazy_static;
+extern crate once_cell;
 
 use std::ptr::null_mut;
 

mmtk/src/object_scanning.rs

@@ -93,8 +93,8 @@ impl OopIterate for InstanceClassLoaderKlass {
 impl OopIterate for ObjArrayKlass {
     #[inline]
     fn oop_iterate(&self, oop: Oop, closure: &mut impl TransitiveClosure) {
-        let array = unsafe { oop.as_array_oop::<Oop>() };
-        for oop in array.data() {
+        let array = unsafe { oop.as_array_oop() };
+        for oop in unsafe { array.data::<Oop>(BasicType::T_OBJECT) } {
             closure.process_edge(Address::from_ref(oop as &Oop));
         }
     }

mmtk/src/scanning.rs

@@ -25,7 +25,13 @@ pub(crate) extern "C" fn create_process_edges_work<W: ProcessEdgesWork<VM = Open
             W::new(buf, true, &SINGLETON),
         );
     }
-    let (ptr, _, capacity) = Vec::with_capacity(W::CAPACITY).into_raw_parts();
+    let (ptr, _, capacity) = {
+        // TODO: Use Vec::into_raw_parts() when the method is available.
+        use std::mem::ManuallyDrop;
+        let new_vec = Vec::with_capacity(W::CAPACITY);
+        let mut me = ManuallyDrop::new(new_vec);
+        (me.as_mut_ptr(), me.len(), me.capacity())
+    };
     NewBuffer { ptr, capacity }
 }
 
@@ -76,17 +82,17 @@ impl Scanning<OpenJDK> for VMScanning {
             &SINGLETON,
             WorkBucketStage::Prepare,
             vec![
-                box ScanUniverseRoots::<W>::new(),
-                box ScanJNIHandlesRoots::<W>::new(),
-                box ScanObjectSynchronizerRoots::<W>::new(),
-                box ScanManagementRoots::<W>::new(),
-                box ScanJvmtiExportRoots::<W>::new(),
-                box ScanAOTLoaderRoots::<W>::new(),
-                box ScanSystemDictionaryRoots::<W>::new(),
-                box ScanCodeCacheRoots::<W>::new(),
-                box ScanStringTableRoots::<W>::new(),
-                box ScanClassLoaderDataGraphRoots::<W>::new(),
-                box ScanWeakProcessorRoots::<W>::new(),
+                Box::new(ScanUniverseRoots::<W>::new()),
+                Box::new(ScanJNIHandlesRoots::<W>::new()),
+                Box::new(ScanObjectSynchronizerRoots::<W>::new()),
+                Box::new(ScanManagementRoots::<W>::new()),
+                Box::new(ScanJvmtiExportRoots::<W>::new()),
+                Box::new(ScanAOTLoaderRoots::<W>::new()),
+                Box::new(ScanSystemDictionaryRoots::<W>::new()),
+                Box::new(ScanCodeCacheRoots::<W>::new()),
+                Box::new(ScanStringTableRoots::<W>::new()),
+                Box::new(ScanClassLoaderDataGraphRoots::<W>::new()),
+                Box::new(ScanWeakProcessorRoots::<W>::new()),
             ],
         );
         if !(Self::SCAN_MUTATORS_IN_SAFEPOINT && Self::SINGLE_THREAD_MUTATOR_SCANNING) {