Unregister PPPs when they are no longer PPPs

wks · wks · commit e9afc444a1da · 2024-10-16T15:31:19.000+08:00
This commit, together with a commit in the `ruby` repo, allows the GC to
remove objects from the list of registered PPPs when they are no longer
PPPs.  Specifically, iseq objects will stop being PPPs since
ISEQ_COMPILE_DATA_CLEAR.

This commit also adds eBPF tracing extensions to track the number of
PPPs and their children involved when pinning or unpinning PPP children,
and the number of PPPs removed when cleaning up the PPP list, and
visualize them on the generated timeline plot.
diff --git a/.gitignore b/.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__
diff --git a/mmtk/Cargo.toml b/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"
diff --git a/mmtk/src/abi.rs b/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),
diff --git a/mmtk/src/ppp.rs b/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);
+        }
+    }
+}
diff --git a/mmtk/src/scanning.rs b/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.");
     }
diff --git a/tools/tracing/timeline/README.md b/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/
diff --git a/tools/tracing/timeline/capture_ruby.bt b/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);
+    }
+}
diff --git a/tools/tracing/timeline/visualize_ruby.py b/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,
+                }
