Change sft from pointers to static refs (#303)

Co-authored-by: Javad Amiri <[email protected]>

Author: qinsoon

src/mmtk.rs

@@ -26,7 +26,7 @@ lazy_static! {
     // TODO: We should refactor this when we know more about how multiple MMTK instances work.
     pub static ref VM_MAP: VMMap = VMMap::new();
     pub static ref MMAPPER: Mmapper = Mmapper::new();
-    pub static ref SFT_MAP: SFTMap = SFTMap::new();
+    pub static ref SFT_MAP: SFTMap<'static> = SFTMap::new();
 }
 
 /// An MMTk instance. MMTk allows mutiple instances to run independently, and each instance gives users a separate heap.
@@ -35,7 +35,7 @@ pub struct MMTK<VM: VMBinding> {
     pub plan: Box<dyn Plan<VM = VM>>,
     pub vm_map: &'static VMMap,
     pub mmapper: &'static Mmapper,
-    pub sftmap: &'static SFTMap,
+    pub sftmap: &'static SFTMap<'static>,
     pub reference_processors: ReferenceProcessors,
     pub finalizable_processor: Mutex<FinalizableProcessor>,
     pub options: Arc<UnsafeOptionsWrapper>,

src/policy/space.rs

@@ -100,16 +100,16 @@ impl SFT for EmptySpaceSFT {
 }
 
 #[derive(Default)]
-pub struct SFTMap {
-    sft: Vec<*const (dyn SFT + Sync)>,
+pub struct SFTMap<'a> {
+    sft: Vec<&'a (dyn SFT + Sync + 'static)>,
 }
 
 // TODO: MMTK<VM> holds a reference to SFTMap. We should have a safe implementation rather than use raw pointers for dyn SFT.
-unsafe impl Sync for SFTMap {}
+unsafe impl<'a> Sync for SFTMap<'a> {}
 
 static EMPTY_SPACE_SFT: EmptySpaceSFT = EmptySpaceSFT {};
 
-impl SFTMap {
+impl<'a> SFTMap<'a> {
     pub fn new() -> Self {
         SFTMap {
             sft: vec![&EMPTY_SPACE_SFT; MAX_CHUNKS],
@@ -124,28 +124,23 @@ impl SFTMap {
         &mut *(self as *const _ as *mut _)
     }
 
-    pub fn get(&self, address: Address) -> &'static dyn SFT {
+    pub fn get(&self, address: Address) -> &'a dyn SFT {
         let res = self.sft[address.chunk_index()];
         if DEBUG_SFT {
             trace!(
                 "Get SFT for {} #{} = {}",
                 address,
                 address.chunk_index(),
-                unsafe { &(*res) }.name()
+                res.name()
             );
         }
-        unsafe { &(*res) }
+        res
     }
 
-    fn log_update(&self, space: *const (dyn SFT + Sync), start: Address, chunks: usize) {
+    fn log_update(&self, space: &(dyn SFT + Sync + 'static), start: Address, chunks: usize) {
         let first = start.chunk_index();
         let end = start + (chunks << LOG_BYTES_IN_CHUNK);
-        debug!(
-            "Update SFT for [{}, {}) as {}",
-            start,
-            end,
-            unsafe { &(*space) }.name()
-        );
+        debug!("Update SFT for [{}, {}) as {}", start, end, space.name());
         let start_chunk = chunk_index_to_address(first);
         let end_chunk = chunk_index_to_address(first + chunks);
         debug!(
@@ -170,18 +165,15 @@ impl SFTMap {
                     i + SPACE_PER_LINE
                 };
                 let chunks: Vec<usize> = (i..max).collect();
-                let space_names: Vec<&str> = chunks
-                    .iter()
-                    .map(|&x| unsafe { &*self.sft[x] }.name())
-                    .collect();
+                let space_names: Vec<&str> = chunks.iter().map(|&x| self.sft[x].name()).collect();
                 trace!("Chunk {}: {}", i, space_names.join(","));
             }
         }
     }
 
     /// Update SFT map for the given address range.
     /// It should be used in these cases: 1. when a space grows, 2. when initializing a contiguous space, 3. when ensure_mapped() is called on a space.
-    pub fn update(&self, space: *const (dyn SFT + Sync), start: Address, chunks: usize) {
+    pub fn update(&self, space: &(dyn SFT + Sync + 'static), start: Address, chunks: usize) {
         if DEBUG_SFT {
             self.log_update(space, start, chunks);
         }
@@ -198,7 +190,7 @@ impl SFTMap {
         self.set(chunk_idx, &EMPTY_SPACE_SFT);
     }
 
-    fn set(&self, chunk: usize, sft: *const (dyn SFT + Sync)) {
+    fn set(&self, chunk: usize, sft: &(dyn SFT + Sync + 'static)) {
         /*
          * This is safe (only) because a) this is only called during the
          * allocation and deallocation of chunks, which happens under a global
@@ -208,11 +200,11 @@ impl SFTMap {
          * which are reasonable), and in the other case it would either see the
          * old (valid) space or an empty space, both of which are valid.
          */
-        let self_mut: &mut Self = unsafe { self.mut_self() };
+        let self_mut = unsafe { self.mut_self() };
         // It is okay to set empty to valid, or set valid to empty. It is wrong if we overwrite a valid value with another valid value.
         if cfg!(debug_assertions) {
-            let old = unsafe { self_mut.sft[chunk].as_ref() }.unwrap().name();
-            let new = unsafe { sft.as_ref() }.unwrap().name();
+            let old = self_mut.sft[chunk].name();
+            let new = sft.name();
             // Allow overwriting the same SFT pointer. E.g., if we have set SFT map for a space, then ensure_mapped() is called on the same,
             // in which case, we still set SFT map again.
             debug_assert!(
@@ -342,7 +334,7 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
         );
         if new_chunk {
             let chunks = conversions::bytes_to_chunks_up(bytes);
-            SFT_MAP.update(self.as_sft() as *const (dyn SFT + Sync), start, chunks);
+            SFT_MAP.update(self.as_sft(), start, chunks);
         }
     }
 
@@ -361,11 +353,7 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
             // TODO(Javad): handle meta space allocation failure
             panic!("failed to mmap meta memory");
         }
-        SFT_MAP.update(
-            self.as_sft() as *const (dyn SFT + Sync),
-            self.common().start,
-            chunks,
-        );
+        SFT_MAP.update(self.as_sft(), self.common().start, chunks);
         use crate::util::heap::layout::mmapper::Mmapper;
         self.common()
             .mmapper