Change the way we handle generic fields table

The upstream has changed its way to handle the generic fields table.
The generic_fields_tbl_ now maps each object to an imemo:fields, and the
imemo:fields object will hold all the fields.  We now treat the
imemo:fields object as a child of its corresponding key in
generic_fields_tbl_, which is just like CRuby's default GC.  Like the
default GC, we handle generic_fields_tbl_ like other weak tables during
weak processing time.

On the Rust side, we replaced the moved_gen_fields_tables hash map with
a simple "backwarding table" that simply maps each moved object using
generic_fields_tbl_ to its old address.

Author: wks

mmtk/src/abi.rs

@@ -11,9 +11,6 @@ pub const GC_THREAD_KIND_WORKER: libc::c_int = 1;
 
 pub const HIDDEN_SIZE_MASK: usize = 0x0000FFFFFFFFFFFF;
 
-// Should keep in sync with C code.
-const RUBY_FL_EXIVAR: usize = 1 << 10;
-
 // An opaque type for the C counterpart.
 #[allow(non_camel_case_types)]
 pub struct st_table;
@@ -92,8 +89,8 @@ impl RubyObjectAccess {
         unsafe { self.flags_field().load::<usize>() }
     }
 
-    pub fn has_exivar_flag(&self) -> bool {
-        (self.load_flags() & RUBY_FL_EXIVAR) != 0
+    pub fn has_exivar(&self) -> bool {
+        (upcalls().has_exivar)(self.objref)
     }
 
     pub fn prefix_size() -> usize {
@@ -109,29 +106,6 @@ impl RubyObjectAccess {
     pub fn object_size(&self) -> usize {
         Self::prefix_size() + self.payload_size() + Self::suffix_size()
     }
-
-    pub fn get_gen_fields_tbl(&self) -> *mut libc::c_void {
-        self.get_original_gen_fields_tbl()
-            .or_else(|| {
-                let moved_gen_fields_tables = crate::binding().moved_gen_fields_tables.lock().unwrap();
-                moved_gen_fields_tables.get(&self.objref).map(|entry| entry.gen_fields_tbl)
-            })
-            .unwrap_or_else(|| {
-                panic!(
-                    "The gen_fields_tbl of object {} is not found in generic_fields_tbl_ or binding().moved_gen_fields_tables",
-                    self.objref
-                )
-            })
-    }
-
-    pub fn get_original_gen_fields_tbl(&self) -> Option<*mut libc::c_void> {
-        let addr = (upcalls().get_original_gen_fields_tbl)(self.objref);
-        if addr.is_null() {
-            None
-        } else {
-            Some(addr)
-        }
-    }
 }
 
 type ObjectClosureFunction =
@@ -359,11 +333,9 @@ pub struct RubyUpcalls {
     pub scan_object_ruby_style: extern "C" fn(object: ObjectReference),
     pub call_gc_mark_children: extern "C" fn(object: ObjectReference),
     pub call_obj_free: extern "C" fn(object: ObjectReference),
-    pub cleanup_generic_fields_tbl: extern "C" fn(),
-    pub get_original_gen_fields_tbl: extern "C" fn(object: ObjectReference) -> *mut libc::c_void,
-    pub reinsert_generic_fields_tbl_entry:
-        extern "C" fn(old_objref: ObjectReference, new_objref: ObjectReference),
     pub vm_live_bytes: extern "C" fn() -> usize,
+    pub has_exivar: extern "C" fn(object: ObjectReference) -> bool,
+    pub update_generic_fields_table: extern "C" fn(),
     pub update_frozen_strings_table: extern "C" fn(),
     pub update_finalizer_and_obj_id_tables: extern "C" fn(),
     pub update_global_symbols_table: extern "C" fn(),

mmtk/src/api.rs

@@ -9,7 +9,6 @@ use crate::abi;
 use crate::abi::HiddenHeader;
 use crate::abi::RawVecOfObjRef;
 use crate::abi::RubyBindingOptions;
-use crate::abi::RubyObjectAccess;
 use crate::binding;
 use crate::binding::RubyBinding;
 use crate::mmtk;
@@ -320,9 +319,9 @@ pub unsafe extern "C" fn mmtk_register_ppps(objects: *const ObjectReference, len
 }
 
 #[no_mangle]
-pub extern "C" fn mmtk_get_gen_fields_tbl_during_gc(object: ObjectReference) -> *mut libc::c_void {
-    let acc = RubyObjectAccess::from_objref(object);
-    acc.get_gen_fields_tbl()
+pub extern "C" fn mmtk_get_backwarded_object(object: ObjectReference) -> ObjectReference {
+    let backwarding_table = crate::binding().backwarding_table.lock().unwrap();
+    backwarding_table.get(&object).copied().unwrap_or(object)
 }
 
 #[no_mangle]

mmtk/src/binding.rs

@@ -5,7 +5,6 @@ use std::sync::atomic::AtomicBool;
 use std::sync::Mutex;
 use std::thread::JoinHandle;
 
-use libc::c_void;
 use mmtk::util::ObjectReference;
 use mmtk::MMTK;
 
@@ -49,19 +48,16 @@ impl RubyBindingFastMut {
     }
 }
 
-pub(crate) struct MovedGenFieldsTablesEntry {
-    pub old_objref: ObjectReference,
-    pub gen_fields_tbl: *mut c_void,
-}
-
 pub struct RubyBinding {
     pub mmtk: &'static MMTK<Ruby>,
     pub options: RubyBindingOptions,
     pub upcalls: *const abi::RubyUpcalls,
     pub plan_name: Mutex<Option<CString>>,
     pub weak_proc: WeakProcessor,
     pub ppp_registry: PPPRegistry,
-    pub(crate) moved_gen_fields_tables: Mutex<HashMap<ObjectReference, MovedGenFieldsTablesEntry>>,
+    /// A "backwarding" table that can look up the old addresses of some moved objects,
+    /// specifically objects using generic fields table.
+    pub(crate) backwarding_table: Mutex<HashMap<ObjectReference, ObjectReference>>,
     pub gc_thread_join_handles: Mutex<Vec<JoinHandle<()>>>,
     pub wb_unprotected_objects: Mutex<HashSet<ObjectReference>>,
     pub st_entries_chunk_size: usize,
@@ -104,7 +100,7 @@ impl RubyBinding {
             plan_name: Mutex::new(None),
             weak_proc: WeakProcessor::new(),
             ppp_registry: PPPRegistry::new(),
-            moved_gen_fields_tables: Default::default(),
+            backwarding_table: Default::default(),
             gc_thread_join_handles: Default::default(),
             wb_unprotected_objects: Default::default(),
             st_entries_chunk_size,

mmtk/src/collection.rs

@@ -1,7 +1,7 @@
 use crate::abi::GCThreadTLS;
 
 use crate::api::RubyMutator;
-use crate::{mmtk, upcalls, Ruby};
+use crate::{binding, mmtk, upcalls, Ruby};
 use mmtk::memory_manager;
 use mmtk::scheduler::*;
 use mmtk::util::{VMMutatorThread, VMThread, VMWorkerThread};
@@ -74,6 +74,14 @@ impl Collection<Ruby> for VMCollection {
     fn vm_live_bytes() -> usize {
         (upcalls().vm_live_bytes)()
     }
+
+    fn post_forwarding(_tls: VMWorkerThread) {
+        let mut backwarding_table = binding()
+            .backwarding_table
+            .try_lock()
+            .expect("Should not have race during post_forwarding");
+        backwarding_table.clear();
+    }
 }
 
 impl VMCollection {

mmtk/src/object_model.rs

@@ -43,11 +43,7 @@ impl ObjectModel<Ruby> for VMObjectModel {
         copy_context: &mut GCWorkerCopyContext<Ruby>,
     ) -> ObjectReference {
         let from_acc = RubyObjectAccess::from_objref(from);
-        let maybe_gen_fields_tbl = from_acc.has_exivar_flag().then(|| {
-            from_acc
-                .get_original_gen_fields_tbl()
-                .unwrap_or_else(|| panic!("Object {} has FL_EXIVAR but no gen_fields_tbl.", from))
-        });
+        let has_exivar = from_acc.has_exivar();
         let from_start = from_acc.obj_start();
         let object_size = from_acc.object_size();
         let to_start = copy_context.alloc_copy(from, object_size, MIN_OBJ_ALIGN, 0, semantics);
@@ -74,16 +70,12 @@ impl ObjectModel<Ruby> for VMObjectModel {
             unsafe { std::ptr::write_bytes::<u8>(from_start.to_mut_ptr(), 0, object_size) }
         }
 
-        if let Some(gen_fields_tbl) = maybe_gen_fields_tbl {
-            let mut moved_gen_fields_tables =
-                crate::binding().moved_gen_fields_tables.lock().unwrap();
-            moved_gen_fields_tables.insert(
-                to_obj,
-                crate::binding::MovedGenFieldsTablesEntry {
-                    old_objref: from,
-                    gen_fields_tbl,
-                },
-            );
+        if has_exivar {
+            let mut backwarding_table = crate::binding().backwarding_table.lock().unwrap();
+            trace!("Inserting into backwarding table: from: {from} <- to_obj: {to_obj}");
+            backwarding_table.insert(to_obj, from);
+        } else {
+            trace!("No exivar: from: {from} <- to_obj: {to_obj}");
         }
 
         to_obj

mmtk/src/weak_proc.rs

@@ -8,7 +8,6 @@ use mmtk::{
 
 use crate::{
     abi::{st_table, GCThreadTLS},
-    binding::MovedGenFieldsTablesEntry,
     extra_assert, is_mmtk_object_safe, upcalls,
     utils::AfterAll,
     Ruby,
@@ -157,51 +156,6 @@ impl WeakProcessor {
 
         worker.scheduler().work_buckets[WorkBucketStage::VMRefClosure].bulk_add(entries_packets);
     }
-
-    /// Update generic instance variable tables.
-    ///
-    /// Objects moved during GC should have their entries in the global `generic_fields_tbl_` hash
-    /// table updated, and dead objects should have their entries removed.
-    fn update_generic_fields_tbl() {
-        // Update `generic_fields_tbl_` entries for moved objects.  We could update the entries in
-        // `ObjectModel::move`.  However, because `st_table` is not thread-safe, we postpone the
-        // update until now in the VMRefClosure stage.
-        log::debug!("Updating generic_fields_tbl_ entries...");
-        let items_moved = {
-            let mut moved_gen_fields_tables = crate::binding()
-                .moved_gen_fields_tables
-                .try_lock()
-                .expect("Should have no race in weak_proc");
-            let items_moved = moved_gen_fields_tables.len();
-            for (new_objref, MovedGenFieldsTablesEntry { old_objref, .. }) in
-                moved_gen_fields_tables.drain()
-            {
-                trace!(
-                    "  generic_fields_tbl_ entry: {} -> {}",
-                    old_objref,
-                    new_objref
-                );
-                (upcalls().reinsert_generic_fields_tbl_entry)(old_objref, new_objref);
-            }
-            items_moved
-        };
-        log::debug!("Updated generic_fields_tbl_ entries.  {items_moved} items moved.");
-
-        // Clean up entries for dead objects.
-        let generic_fields_tbl = (upcalls().get_generic_fields_tbl)();
-        let old_size = (upcalls().st_get_num_entries)(generic_fields_tbl);
-        log::debug!("Cleaning up generic_fields_tbl_ entries ({old_size} entries before)...");
-        (crate::upcalls().cleanup_generic_fields_tbl)();
-        let new_size = (upcalls().st_get_num_entries)(generic_fields_tbl);
-        log::debug!("Cleaned up generic_fields_tbl_ entries ({new_size} entries after).");
-        probe!(
-            mmtk_ruby,
-            update_generic_fields_tbl,
-            items_moved,
-            old_size,
-            new_size
-        );
-    }
 }
 
 struct ProcessObjFreeCandidates;
@@ -286,7 +240,10 @@ macro_rules! define_global_table_processor {
 }
 
 define_global_table_processor!(UpdateGenericFieldsTbl, {
-    WeakProcessor::update_generic_fields_tbl();
+    general_update_weak_table(
+        upcalls().get_generic_fields_tbl,
+        upcalls().update_generic_fields_table,
+    );
 });
 
 define_global_table_processor!(UpdateFrozenStringsTable, {