Unregister PPPs when they are no longer PPPs

.gitignore

@@ -10,3 +10,6 @@ target/
 
 # These are backup files generated by rustfmt
 **/*.rs.bk
+
+# Python scripts in eBPF tools may generate such temporary caches.
+__pycache__

mmtk/Cargo.toml

@@ -12,7 +12,7 @@ edition = "2021"
 # Metadata for the Ruby repository
 [package.metadata.ci-repos.ruby]
 repo = "mmtk/ruby" # This is used by actions/checkout, so the format is "owner/repo", not URL.
-rev = "8c96eaf5cf4bb39610fc03797a42870f7eb8dca2"
+rev = "0511ec851cd290fac520dbe5a862b409488e159e"
 
 [lib]
 name = "mmtk_ruby"

mmtk/src/abi.rs

@@ -385,6 +385,7 @@ pub struct RubyUpcalls {
     pub scan_final_jobs_roots: extern "C" fn(),
     pub scan_roots_in_mutator_thread:
         extern "C" fn(mutator_tls: VMMutatorThread, worker_tls: VMWorkerThread),
+    pub is_no_longer_ppp: extern "C" fn(ObjectReference) -> bool,
     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),

mmtk/src/ppp.rs

@@ -2,8 +2,9 @@ use std::sync::Mutex;
 
 use mmtk::{
     memory_manager,
-    scheduler::{GCWork, WorkBucketStage},
+    scheduler::{GCWork, GCWorker, WorkBucketStage},
     util::{ObjectReference, VMWorkerThread},
+    MMTK,
 };
 
 use crate::{abi::GCThreadTLS, upcalls, Ruby};
@@ -65,42 +66,32 @@ impl PPPRegistry {
         }
     }
 
-    pub fn cleanup_ppps(&self) {
-        log::debug!("Removing dead PPPs...");
-        {
-            let mut ppps = self
-                .ppps
-                .try_lock()
-                .expect("PPPRegistry::ppps should not have races during GC.");
-
-            probe!(mmtk_ruby, remove_dead_ppps_start, ppps.len());
-            ppps.retain_mut(|obj| {
-                if obj.is_live() {
-                    *obj = obj.get_forwarded_object().unwrap_or(*obj);
-                    true
-                } else {
-                    log::trace!("  PPP removed: {}", *obj);
-                    false
+    pub fn cleanup_ppps(&self, worker: &mut GCWorker<Ruby>) {
+        worker.scheduler().work_buckets[WorkBucketStage::VMRefClosure].add(RemoveDeadPPPs);
+        if crate::mmtk().get_plan().current_gc_may_move_object() {
+            let packet = {
+                let mut pinned_ppp_children = self
+                    .pinned_ppp_children
+                    .try_lock()
+                    .expect("Unexpected contention on pinned_ppp_children");
+                UnpinPPPChildren {
+                    children: std::mem::take(&mut pinned_ppp_children),
                 }
-            });
-            probe!(mmtk_ruby, remove_dead_ppps_end);
-        }
-
-        log::debug!("Unpinning pinned PPP children...");
+            };
 
-        if !crate::mmtk().get_plan().current_gc_may_move_object() {
-            log::debug!("The current GC is non-moving.  Skipped unpinning PPP children.");
+            worker.scheduler().work_buckets[WorkBucketStage::VMRefClosure].add(packet);
         } else {
-            let mut pinned_ppps = self
-                .pinned_ppp_children
-                .try_lock()
-                .expect("PPPRegistry::pinned_ppp_children should not have races during GC.");
-            probe!(mmtk_ruby, unpin_ppp_children_start, pinned_ppps.len());
-            for obj in pinned_ppps.drain(..) {
-                let unpinned = memory_manager::unpin_object(obj);
-                debug_assert!(unpinned);
-            }
-            probe!(mmtk_ruby, unpin_ppp_children_end);
+            debug!("Skipping unpinning PPP children because the current GC is non-copying.");
+            debug_assert_eq!(
+                {
+                    let pinned_ppp_children = self
+                        .pinned_ppp_children
+                        .try_lock()
+                        .expect("Unexpected contention on pinned_ppp_children");
+                    pinned_ppp_children.len()
+                },
+                0
+            );
         }
     }
 }
@@ -116,14 +107,12 @@ struct PinPPPChildren {
 }
 
 impl GCWork<Ruby> for PinPPPChildren {
-    fn do_work(
-        &mut self,
-        worker: &mut mmtk::scheduler::GCWorker<Ruby>,
-        _mmtk: &'static mmtk::MMTK<Ruby>,
-    ) {
+    fn do_work(&mut self, worker: &mut GCWorker<Ruby>, _mmtk: &'static MMTK<Ruby>) {
         let gc_tls = unsafe { GCThreadTLS::from_vwt_check(worker.tls) };
+        let num_ppps = self.ppps.len();
         let mut ppp_children = vec![];
         let mut newly_pinned_ppp_children = vec![];
+        let mut num_no_longer_ppps = 0usize;
 
         let visit_object = |_worker, target_object: ObjectReference, pin| {
             log::trace!(
@@ -142,6 +131,11 @@ impl GCWork<Ruby> for PinPPPChildren {
             .set_temporarily_and_run_code(visit_object, || {
                 for obj in self.ppps.iter().cloned() {
                     log::trace!("  PPP: {}", obj);
+                    if (upcalls().is_no_longer_ppp)(obj) {
+                        num_no_longer_ppps += 1;
+                        log::trace!("    No longer PPP. Skip: {}", obj);
+                        continue;
+                    }
                     (upcalls().call_gc_mark_children)(obj);
                 }
             });
@@ -152,6 +146,16 @@ impl GCWork<Ruby> for PinPPPChildren {
             }
         }
 
+        let num_pinned_children = newly_pinned_ppp_children.len();
+
+        probe!(
+            mmtk_ruby,
+            pin_ppp_children,
+            num_ppps,
+            num_no_longer_ppps,
+            num_pinned_children
+        );
+
         {
             let mut pinned_ppp_children = crate::binding()
                 .ppp_registry
@@ -162,3 +166,68 @@ impl GCWork<Ruby> for PinPPPChildren {
         }
     }
 }
+
+struct RemoveDeadPPPs;
+
+impl GCWork<Ruby> for RemoveDeadPPPs {
+    fn do_work(&mut self, _worker: &mut GCWorker<Ruby>, _mmtk: &'static MMTK<Ruby>) {
+        log::debug!("Removing dead PPPs...");
+
+        let registry = &crate::binding().ppp_registry;
+        {
+            let mut ppps = registry
+                .ppps
+                .try_lock()
+                .expect("PPPRegistry::ppps should not have races during GC.");
+
+            let num_ppps = ppps.len();
+            let mut num_no_longer_ppps = 0usize;
+            let mut num_dead_ppps = 0usize;
+
+            ppps.retain_mut(|obj| {
+                if obj.is_live() {
+                    let new_obj = obj.get_forwarded_object().unwrap_or(*obj);
+                    if (upcalls().is_no_longer_ppp)(new_obj) {
+                        num_no_longer_ppps += 1;
+                        log::trace!("  No longer PPP. Remove: {}", new_obj);
+                        false
+                    } else {
+                        *obj = new_obj;
+                        true
+                    }
+                } else {
+                    num_dead_ppps += 1;
+                    log::trace!("  Dead PPP removed: {}", *obj);
+                    false
+                }
+            });
+
+            probe!(
+                mmtk_ruby,
+                remove_dead_ppps,
+                num_ppps,
+                num_no_longer_ppps,
+                num_dead_ppps
+            );
+        }
+    }
+}
+
+struct UnpinPPPChildren {
+    children: Vec<ObjectReference>,
+}
+
+impl GCWork<Ruby> for UnpinPPPChildren {
+    fn do_work(&mut self, _worker: &mut GCWorker<Ruby>, _mmtk: &'static MMTK<Ruby>) {
+        log::debug!("Unpinning pinned PPP children...");
+
+        let num_children = self.children.len();
+
+        probe!(mmtk_ruby, unpin_ppp_children, num_children);
+
+        for obj in self.children.iter() {
+            let unpinned = memory_manager::unpin_object(*obj);
+            debug_assert!(unpinned);
+        }
+    }
+}

mmtk/src/scanning.rs

@@ -4,7 +4,7 @@ use crate::utils::ChunkedVecCollector;
 use crate::{extra_assert, is_mmtk_object_safe, upcalls, Ruby, RubySlot};
 use mmtk::scheduler::{GCWork, GCWorker, WorkBucketStage};
 use mmtk::util::{ObjectReference, VMWorkerThread};
-use mmtk::vm::{ObjectTracer, RootsWorkFactory, Scanning, SlotVisitor};
+use mmtk::vm::{ObjectTracer, ObjectTracerContext, RootsWorkFactory, Scanning, SlotVisitor};
 use mmtk::{Mutator, MutatorContext};
 
 pub struct VMScanning {}
@@ -135,18 +135,18 @@ impl Scanning<Ruby> for VMScanning {
 
     fn process_weak_refs(
         worker: &mut GCWorker<Ruby>,
-        tracer_context: impl mmtk::vm::ObjectTracerContext<Ruby>,
+        tracer_context: impl ObjectTracerContext<Ruby>,
     ) -> bool {
         crate::binding()
             .weak_proc
             .process_weak_stuff(worker, tracer_context);
-        crate::binding().ppp_registry.cleanup_ppps();
+        crate::binding().ppp_registry.cleanup_ppps(worker);
         false
     }
 
     fn forward_weak_refs(
         _worker: &mut GCWorker<Ruby>,
-        _tracer_context: impl mmtk::vm::ObjectTracerContext<Ruby>,
+        _tracer_context: impl ObjectTracerContext<Ruby>,
     ) {
         panic!("We can't use MarkCompact in Ruby.");
     }

tools/tracing/timeline/README.md

@@ -0,0 +1,32 @@
+# Ruby binding-specific timeline visualizer extensions
+
+Scripts in this directory extends the eBPF timeline visualizer tools in the
+[mmtk-core](https://github.com/mmtk/mmtk-core/) repository to add more information about work
+packets defined in the mmtk-ruby binding.
+
+Read `mmtk-core/tools/tracing/timeline/README.md` for the basic usage of the tools, and read
+`mmtk-core/tools/tracing/timeline/EXTENSION.md` for details about extensions.
+
+## Examples:
+
+To capture a trace with Ruby-specific information:
+
+```
+/path/to/mmtk-core/tools/tracing/timeline/capture.py \
+    -x /path/to/mmtk-ruby/tools/tracing/timeline/capture_ruby.bt \
+    -m /path/to/mmtk-ruby/mmtk/target/release/libmmtk_ruby.so \
+    > my-execution.log
+```
+
+To convert the log into the JSON format, with Ruby-specific information added to the timeline
+blocks:
+
+```
+/path/to/mmtk-core/tools/tracing/timeline/visualize.py \
+    -x /path/to/mmtk-ruby/tools/tracing/timeline/visualize_ruby.bt \
+    my-execution.log
+```
+
+It will generate `my-execution.log.json.gz` which can be loaded into [Perfetto UI].
+
+[Perfetto UI]: https://www.ui.perfetto.dev/

tools/tracing/timeline/capture_ruby.bt

@@ -0,0 +1,17 @@
+usdt:$MMTK:mmtk_ruby:pin_ppp_children {
+    if (@enable_print) {
+        printf("pin_ppp_children,meta,%d,%lu,%lu,%lu,%lu\n", tid, nsecs, arg0, arg1, arg2);
+    }
+}
+
+usdt:$MMTK:mmtk_ruby:remove_dead_ppps {
+    if (@enable_print) {
+        printf("remove_dead_ppps,meta,%d,%lu,%lu,%lu,%lu\n", tid, nsecs, arg0, arg1, arg2);
+    }
+}
+
+usdt:$MMTK:mmtk_ruby:unpin_ppp_children {
+    if (@enable_print) {
+        printf("unpin_ppp_children,meta,%d,%lu,%lu\n", tid, nsecs, arg0);
+    }
+}

tools/tracing/timeline/visualize_ruby.py

@@ -0,0 +1,30 @@
+#!/usr/bin/env python3
+
+def enrich_meta_extra(log_processor, name, tid, ts, gc, wp, args):
+    if wp is not None:
+        match name:
+            case "pin_ppp_children":
+                num_ppps, num_no_longer_ppps, num_pinned_children = [int(x) for x in args]
+                num_still_ppps = num_ppps - num_no_longer_ppps
+                wp["args"] |= {
+                    "num_ppps": num_ppps,
+                    "num_no_longer_ppps": num_no_longer_ppps,
+                    "num_still_ppps": num_still_ppps,
+                    "num_pinned_children": num_pinned_children,
+                }
+
+            case "remove_dead_ppps":
+                num_ppps, num_no_longer_ppps, num_dead_ppps = [int(x) for x in args]
+                num_retained_ppps = num_ppps - num_no_longer_ppps - num_dead_ppps
+                wp["args"] |= {
+                    "num_ppps": num_ppps,
+                    "num_no_longer_ppps": num_no_longer_ppps,
+                    "num_dead_ppps": num_dead_ppps,
+                    "num_retained_ppps": num_retained_ppps,
+                }
+
+            case "unpin_ppp_children":
+                num_children = int(args[0])
+                wp["args"] |= {
+                    "num_ppp_children": num_children,
+                }