From e9afc444a1da1121e0ea7374cb71286aaaf8a42d Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 16 Oct 2024 15:31:19 +0800 Subject: [PATCH] Unregister PPPs when they are no longer PPPs 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. --- .gitignore | 3 + mmtk/Cargo.toml | 2 +- mmtk/src/abi.rs | 1 + mmtk/src/ppp.rs | 147 +++++++++++++++++------ mmtk/src/scanning.rs | 8 +- tools/tracing/timeline/README.md | 32 +++++ tools/tracing/timeline/capture_ruby.bt | 17 +++ tools/tracing/timeline/visualize_ruby.py | 30 +++++ 8 files changed, 196 insertions(+), 44 deletions(-) create mode 100644 tools/tracing/timeline/README.md create mode 100644 tools/tracing/timeline/capture_ruby.bt create mode 100755 tools/tracing/timeline/visualize_ruby.py diff --git a/.gitignore b/.gitignore index 8c8211f..1d58d58 100644 --- 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 index 7ddd662..556523f 100644 --- 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 index b50c472..2dfed17 100644 --- 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 index 526089c..7b911c7 100644 --- 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) { + 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 for PinPPPChildren { - fn do_work( - &mut self, - worker: &mut mmtk::scheduler::GCWorker, - _mmtk: &'static mmtk::MMTK, - ) { + fn do_work(&mut self, worker: &mut GCWorker, _mmtk: &'static MMTK) { 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 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 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 for PinPPPChildren { } } } + +struct RemoveDeadPPPs; + +impl GCWork for RemoveDeadPPPs { + fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { + 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, +} + +impl GCWork for UnpinPPPChildren { + fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { + 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 index 48f7b6e..40d771f 100644 --- 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 for VMScanning { fn process_weak_refs( worker: &mut GCWorker, - tracer_context: impl mmtk::vm::ObjectTracerContext, + tracer_context: impl ObjectTracerContext, ) -> 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, - _tracer_context: impl mmtk::vm::ObjectTracerContext, + _tracer_context: impl ObjectTracerContext, ) { panic!("We can't use MarkCompact in Ruby."); } diff --git a/tools/tracing/timeline/README.md b/tools/tracing/timeline/README.md new file mode 100644 index 0000000..b4ccaa7 --- /dev/null +++ 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 new file mode 100644 index 0000000..0883e2c --- /dev/null +++ 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 new file mode 100755 index 0000000..5c2e357 --- /dev/null +++ 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, + }