From 544770d566ae6b9cbdb79b24a555f10f60b3e5e3 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 30 Dec 2025 13:42:21 -0500 Subject: [PATCH 1/4] [ruby/mmtk] Process obj_free candidates in parallel This commit allows objects that are safe to be freed in parallel to do so. A decrease in object freeing time can be seen in profiles. The benchmarks don't show much difference. Before: -------------- -------------------- ---------- --------- bench sequential free (ms) stddev (%) RSS (MiB) activerecord 242.3 7.4 84.3 chunky-png 439.1 0.6 75.6 erubi-rails 1221.2 4.2 132.7 hexapdf 1544.8 1.8 429.1 liquid-c 42.7 7.4 48.5 liquid-compile 41.4 8.3 52.2 liquid-render 100.6 3.0 56.8 mail 108.9 2.1 65.1 psych-load 1536.9 0.6 43.4 railsbench 1633.5 2.6 146.2 rubocop 126.5 15.8 142.1 ruby-lsp 129.6 9.7 112.2 sequel 47.9 6.5 44.6 shipit 1152.0 2.7 315.2 -------------- -------------------- ---------- --------- After: -------------- ------------------ ---------- --------- bench parallel free (ms) stddev (%) RSS (MiB) activerecord 235.1 5.5 87.4 chunky-png 440.8 0.8 68.1 erubi-rails 1105.3 0.8 128.0 hexapdf 1578.3 4.1 405.1 liquid-c 42.6 7.1 48.4 liquid-compile 41.5 8.1 52.1 liquid-render 101.2 2.8 53.3 mail 109.7 2.7 64.8 psych-load 1567.7 1.1 44.4 railsbench 1644.9 1.9 150.9 rubocop 125.6 15.4 148.5 ruby-lsp 127.9 5.8 104.6 sequel 48.2 6.1 44.1 shipit 1215.3 4.7 320.5 -------------- ------------------ ---------- --------- https://github.com/ruby/mmtk/commit/4f0b5fd2eb --- gc/mmtk/mmtk.c | 23 ++++++++- gc/mmtk/mmtk.h | 2 +- gc/mmtk/src/api.rs | 11 ++-- gc/mmtk/src/weak_proc.rs | 106 ++++++++++++++++++++++++++++++--------- 4 files changed, 114 insertions(+), 28 deletions(-) diff --git a/gc/mmtk/mmtk.c b/gc/mmtk/mmtk.c index b532d3a774cca1..f61130b4f62350 100644 --- a/gc/mmtk/mmtk.c +++ b/gc/mmtk/mmtk.c @@ -696,6 +696,27 @@ rb_mmtk_alloc_fast_path(struct objspace *objspace, struct MMTk_ractor_cache *rac } } +static bool +obj_can_parallel_free_p(VALUE obj) +{ + switch (RB_BUILTIN_TYPE(obj)) { + case T_ARRAY: + case T_BIGNUM: + case T_COMPLEX: + case T_FLOAT: + case T_HASH: + case T_OBJECT: + case T_RATIONAL: + case T_REGEXP: + case T_STRING: + case T_STRUCT: + case T_SYMBOL: + return true; + default: + return false; + } +} + VALUE rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size) { @@ -732,7 +753,7 @@ rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags mmtk_post_alloc(ractor_cache->mutator, (void*)alloc_obj, alloc_size, MMTK_ALLOCATION_SEMANTICS_DEFAULT); // TODO: only add when object needs obj_free to be called - mmtk_add_obj_free_candidate(alloc_obj); + mmtk_add_obj_free_candidate(alloc_obj, obj_can_parallel_free_p((VALUE)alloc_obj)); objspace->total_allocated_objects++; diff --git a/gc/mmtk/mmtk.h b/gc/mmtk/mmtk.h index f7da0f95f0214d..51798c4240f1d4 100644 --- a/gc/mmtk/mmtk.h +++ b/gc/mmtk/mmtk.h @@ -122,7 +122,7 @@ void mmtk_post_alloc(MMTk_Mutator *mutator, size_t bytes, MMTk_AllocationSemantics semantics); -void mmtk_add_obj_free_candidate(MMTk_ObjectReference object); +void mmtk_add_obj_free_candidate(MMTk_ObjectReference object, bool can_parallel_free); void mmtk_declare_weak_references(MMTk_ObjectReference object); diff --git a/gc/mmtk/src/api.rs b/gc/mmtk/src/api.rs index ec0e5bafe2289a..3515a2408b3714 100644 --- a/gc/mmtk/src/api.rs +++ b/gc/mmtk/src/api.rs @@ -198,7 +198,10 @@ pub unsafe extern "C" fn mmtk_init_binding( let mmtk_boxed = mmtk_init(&builder); let mmtk_static = Box::leak(Box::new(mmtk_boxed)); - let binding = RubyBinding::new(mmtk_static, &binding_options, upcalls); + let mut binding = RubyBinding::new(mmtk_static, &binding_options, upcalls); + binding + .weak_proc + .init_parallel_obj_free_candidates(memory_manager::num_of_workers(binding.mmtk)); crate::BINDING .set(binding) @@ -296,8 +299,10 @@ pub unsafe extern "C" fn mmtk_post_alloc( // TODO: Replace with buffered mmtk_add_obj_free_candidates #[no_mangle] -pub extern "C" fn mmtk_add_obj_free_candidate(object: ObjectReference) { - binding().weak_proc.add_obj_free_candidate(object) +pub extern "C" fn mmtk_add_obj_free_candidate(object: ObjectReference, can_parallel_free: bool) { + binding() + .weak_proc + .add_obj_free_candidate(object, can_parallel_free) } // =============== Weak references =============== diff --git a/gc/mmtk/src/weak_proc.rs b/gc/mmtk/src/weak_proc.rs index 3184c4f6d183f2..8bb82625446c85 100644 --- a/gc/mmtk/src/weak_proc.rs +++ b/gc/mmtk/src/weak_proc.rs @@ -1,3 +1,5 @@ +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering; use std::sync::Mutex; use mmtk::{ @@ -9,10 +11,13 @@ use mmtk::{ use crate::{abi::GCThreadTLS, upcalls, Ruby}; pub struct WeakProcessor { + non_parallel_obj_free_candidates: Mutex>, + parallel_obj_free_candidates: Vec>>, + parallel_obj_free_candidates_counter: AtomicUsize, + /// Objects that needs `obj_free` called when dying. /// If it is a bottleneck, replace it with a lock-free data structure, /// or add candidates in batch. - obj_free_candidates: Mutex>, weak_references: Mutex>, } @@ -25,32 +30,59 @@ impl Default for WeakProcessor { impl WeakProcessor { pub fn new() -> Self { Self { - obj_free_candidates: Mutex::new(Vec::new()), + non_parallel_obj_free_candidates: Mutex::new(Vec::new()), + parallel_obj_free_candidates: vec![Mutex::new(Vec::new())], + parallel_obj_free_candidates_counter: AtomicUsize::new(0), weak_references: Mutex::new(Vec::new()), } } - /// Add an object as a candidate for `obj_free`. - /// - /// Multiple mutators can call it concurrently, so it has `&self`. - pub fn add_obj_free_candidate(&self, object: ObjectReference) { - let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); - obj_free_candidates.push(object); + pub fn init_parallel_obj_free_candidates(&mut self, num_workers: usize) { + debug_assert_eq!(self.parallel_obj_free_candidates.len(), 1); + + for _ in 1..num_workers { + self.parallel_obj_free_candidates + .push(Mutex::new(Vec::new())); + } } - /// Add many objects as candidates for `obj_free`. + /// Add an object as a candidate for `obj_free`. /// /// Multiple mutators can call it concurrently, so it has `&self`. - pub fn add_obj_free_candidates(&self, objects: &[ObjectReference]) { - let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); - for object in objects.iter().copied() { - obj_free_candidates.push(object); + pub fn add_obj_free_candidate(&self, object: ObjectReference, can_parallel_free: bool) { + if can_parallel_free { + // Newly allocated objects are placed in parallel_obj_free_candidates using + // round-robin. This may not be ideal for load balancing. + let idx = self + .parallel_obj_free_candidates_counter + .fetch_add(1, Ordering::Relaxed) + % self.parallel_obj_free_candidates.len(); + + self.parallel_obj_free_candidates[idx] + .lock() + .unwrap() + .push(object); + } else { + self.non_parallel_obj_free_candidates + .lock() + .unwrap() + .push(object); } } pub fn get_all_obj_free_candidates(&self) -> Vec { - let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); - std::mem::take(obj_free_candidates.as_mut()) + // let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); + let mut all_obj_free_candidates = self + .non_parallel_obj_free_candidates + .lock() + .unwrap() + .to_vec(); + + for candidates_mutex in &self.parallel_obj_free_candidates { + all_obj_free_candidates.extend(candidates_mutex.lock().unwrap().to_vec()); + } + + std::mem::take(all_obj_free_candidates.as_mut()) } pub fn add_weak_reference(&self, object: ObjectReference) { @@ -63,7 +95,22 @@ impl WeakProcessor { worker: &mut GCWorker, _tracer_context: impl ObjectTracerContext, ) { - worker.add_work(WorkBucketStage::VMRefClosure, ProcessObjFreeCandidates); + worker.add_work( + WorkBucketStage::VMRefClosure, + ProcessObjFreeCandidates { + process_type: ProcessObjFreeCandidatesType::NonParallel, + }, + ); + + for i in 0..self.parallel_obj_free_candidates.len() { + worker.add_work( + WorkBucketStage::VMRefClosure, + ProcessObjFreeCandidates { + process_type: ProcessObjFreeCandidatesType::Parallel(i), + }, + ); + } + worker.add_work(WorkBucketStage::VMRefClosure, ProcessWeakReferences); worker.add_work(WorkBucketStage::Prepare, UpdateFinalizerObjIdTables); @@ -80,16 +127,29 @@ impl WeakProcessor { } } -struct ProcessObjFreeCandidates; +enum ProcessObjFreeCandidatesType { + NonParallel, + Parallel(usize), +} + +struct ProcessObjFreeCandidates { + process_type: ProcessObjFreeCandidatesType, +} impl GCWork for ProcessObjFreeCandidates { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static mmtk::MMTK) { - // If it blocks, it is a bug. - let mut obj_free_candidates = crate::binding() - .weak_proc - .obj_free_candidates - .try_lock() - .expect("It's GC time. No mutators should hold this lock at this time."); + let mut obj_free_candidates = match self.process_type { + ProcessObjFreeCandidatesType::NonParallel => crate::binding() + .weak_proc + .non_parallel_obj_free_candidates + .try_lock() + .expect("Lock for non_parallel_obj_free_candidates should not be held"), + ProcessObjFreeCandidatesType::Parallel(idx) => { + crate::binding().weak_proc.parallel_obj_free_candidates[idx] + .try_lock() + .expect("Lock for parallel_obj_free_candidates should not be held") + } + }; let n_cands = obj_free_candidates.len(); From dbfedeb3a30be711fa17bf6ab9e1f4a83338f7d9 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 30 Dec 2025 14:03:40 -0500 Subject: [PATCH 2/4] [ruby/mmtk] Split ProcessObjFreeCandidates to parallel and non-parallel This makes it easier to visualize in profilers which one is non-parallel. https://github.com/ruby/mmtk/commit/ba68b2ef3b --- gc/mmtk/src/weak_proc.rs | 79 +++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/gc/mmtk/src/weak_proc.rs b/gc/mmtk/src/weak_proc.rs index 8bb82625446c85..41b133a4945506 100644 --- a/gc/mmtk/src/weak_proc.rs +++ b/gc/mmtk/src/weak_proc.rs @@ -97,17 +97,13 @@ impl WeakProcessor { ) { worker.add_work( WorkBucketStage::VMRefClosure, - ProcessObjFreeCandidates { - process_type: ProcessObjFreeCandidatesType::NonParallel, - }, + ProcessNonParallelObjFreeCanadidates {}, ); - for i in 0..self.parallel_obj_free_candidates.len() { + for index in 0..self.parallel_obj_free_candidates.len() { worker.add_work( WorkBucketStage::VMRefClosure, - ProcessObjFreeCandidates { - process_type: ProcessObjFreeCandidatesType::Parallel(i), - }, + ProcessParallelObjFreeCandidates { index }, ); } @@ -127,49 +123,50 @@ impl WeakProcessor { } } -enum ProcessObjFreeCandidatesType { - NonParallel, - Parallel(usize), +fn process_obj_free_candidates(obj_free_candidates: &mut Vec) { + // Process obj_free + let mut new_candidates = Vec::new(); + + for object in obj_free_candidates.iter().copied() { + if object.is_reachable() { + // Forward and add back to the candidate list. + let new_object = object.forward(); + trace!("Forwarding obj_free candidate: {object} -> {new_object}"); + new_candidates.push(new_object); + } else { + (upcalls().call_obj_free)(object); + } + } + + *obj_free_candidates = new_candidates; } -struct ProcessObjFreeCandidates { - process_type: ProcessObjFreeCandidatesType, +struct ProcessParallelObjFreeCandidates { + index: usize, } -impl GCWork for ProcessObjFreeCandidates { +impl GCWork for ProcessParallelObjFreeCandidates { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static mmtk::MMTK) { - let mut obj_free_candidates = match self.process_type { - ProcessObjFreeCandidatesType::NonParallel => crate::binding() - .weak_proc - .non_parallel_obj_free_candidates - .try_lock() - .expect("Lock for non_parallel_obj_free_candidates should not be held"), - ProcessObjFreeCandidatesType::Parallel(idx) => { - crate::binding().weak_proc.parallel_obj_free_candidates[idx] - .try_lock() - .expect("Lock for parallel_obj_free_candidates should not be held") - } - }; - - let n_cands = obj_free_candidates.len(); + let mut obj_free_candidates = crate::binding().weak_proc.parallel_obj_free_candidates + [self.index] + .try_lock() + .expect("Lock for parallel_obj_free_candidates should not be held"); - debug!("Total: {n_cands} candidates"); + process_obj_free_candidates(&mut obj_free_candidates); + } +} - // Process obj_free - let mut new_candidates = Vec::new(); +struct ProcessNonParallelObjFreeCanadidates; - for object in obj_free_candidates.iter().copied() { - if object.is_reachable() { - // Forward and add back to the candidate list. - let new_object = object.forward(); - trace!("Forwarding obj_free candidate: {object} -> {new_object}"); - new_candidates.push(new_object); - } else { - (upcalls().call_obj_free)(object); - } - } +impl GCWork for ProcessNonParallelObjFreeCanadidates { + fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static mmtk::MMTK) { + let mut obj_free_candidates = crate::binding() + .weak_proc + .non_parallel_obj_free_candidates + .try_lock() + .expect("Lock for non_parallel_obj_free_candidates should not be held"); - *obj_free_candidates = new_candidates; + process_obj_free_candidates(&mut obj_free_candidates); } } From b27d9353a73179f1f8e582d651382b1b4404cab2 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 31 Dec 2025 23:56:58 +0900 Subject: [PATCH 3/4] Use `is_obj_encoding` instead of `is_data_encoding` The argument to `is_data_encoding` is assumed to be `T_DATA`. --- encoding.c | 7 +- gc/mmtk/mmtk.c | 23 +------ gc/mmtk/mmtk.h | 2 +- gc/mmtk/src/api.rs | 11 +--- gc/mmtk/src/weak_proc.rs | 135 +++++++++++---------------------------- 5 files changed, 48 insertions(+), 130 deletions(-) diff --git a/encoding.c b/encoding.c index 749cbd586d00be..8bb393b471ed54 100644 --- a/encoding.c +++ b/encoding.c @@ -125,8 +125,9 @@ static const rb_data_type_t encoding_data_type = { 0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED }; -#define is_data_encoding(obj) (RTYPEDDATA_P(obj) && RTYPEDDATA_TYPE(obj) == &encoding_data_type) -#define is_obj_encoding(obj) (RB_TYPE_P((obj), T_DATA) && is_data_encoding(obj)) +#define is_encoding_type(obj) (RTYPEDDATA_TYPE(obj) == &encoding_data_type) +#define is_data_encoding(obj) (rbimpl_rtypeddata_p(obj) && is_encoding_type(obj)) +#define is_obj_encoding(obj) (rbimpl_obj_typeddata_p(obj) && is_encoding_type(obj)) int rb_data_is_encoding(VALUE obj) @@ -1345,7 +1346,7 @@ enc_inspect(VALUE self) { rb_encoding *enc; - if (!is_data_encoding(self)) { + if (!is_obj_encoding(self)) { /* do not resolve autoload */ not_encoding(self); } if (!(enc = RTYPEDDATA_GET_DATA(self)) || rb_enc_from_index(rb_enc_to_index(enc)) != enc) { diff --git a/gc/mmtk/mmtk.c b/gc/mmtk/mmtk.c index f61130b4f62350..b532d3a774cca1 100644 --- a/gc/mmtk/mmtk.c +++ b/gc/mmtk/mmtk.c @@ -696,27 +696,6 @@ rb_mmtk_alloc_fast_path(struct objspace *objspace, struct MMTk_ractor_cache *rac } } -static bool -obj_can_parallel_free_p(VALUE obj) -{ - switch (RB_BUILTIN_TYPE(obj)) { - case T_ARRAY: - case T_BIGNUM: - case T_COMPLEX: - case T_FLOAT: - case T_HASH: - case T_OBJECT: - case T_RATIONAL: - case T_REGEXP: - case T_STRING: - case T_STRUCT: - case T_SYMBOL: - return true; - default: - return false; - } -} - VALUE rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size) { @@ -753,7 +732,7 @@ rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags mmtk_post_alloc(ractor_cache->mutator, (void*)alloc_obj, alloc_size, MMTK_ALLOCATION_SEMANTICS_DEFAULT); // TODO: only add when object needs obj_free to be called - mmtk_add_obj_free_candidate(alloc_obj, obj_can_parallel_free_p((VALUE)alloc_obj)); + mmtk_add_obj_free_candidate(alloc_obj); objspace->total_allocated_objects++; diff --git a/gc/mmtk/mmtk.h b/gc/mmtk/mmtk.h index 51798c4240f1d4..f7da0f95f0214d 100644 --- a/gc/mmtk/mmtk.h +++ b/gc/mmtk/mmtk.h @@ -122,7 +122,7 @@ void mmtk_post_alloc(MMTk_Mutator *mutator, size_t bytes, MMTk_AllocationSemantics semantics); -void mmtk_add_obj_free_candidate(MMTk_ObjectReference object, bool can_parallel_free); +void mmtk_add_obj_free_candidate(MMTk_ObjectReference object); void mmtk_declare_weak_references(MMTk_ObjectReference object); diff --git a/gc/mmtk/src/api.rs b/gc/mmtk/src/api.rs index 3515a2408b3714..ec0e5bafe2289a 100644 --- a/gc/mmtk/src/api.rs +++ b/gc/mmtk/src/api.rs @@ -198,10 +198,7 @@ pub unsafe extern "C" fn mmtk_init_binding( let mmtk_boxed = mmtk_init(&builder); let mmtk_static = Box::leak(Box::new(mmtk_boxed)); - let mut binding = RubyBinding::new(mmtk_static, &binding_options, upcalls); - binding - .weak_proc - .init_parallel_obj_free_candidates(memory_manager::num_of_workers(binding.mmtk)); + let binding = RubyBinding::new(mmtk_static, &binding_options, upcalls); crate::BINDING .set(binding) @@ -299,10 +296,8 @@ pub unsafe extern "C" fn mmtk_post_alloc( // TODO: Replace with buffered mmtk_add_obj_free_candidates #[no_mangle] -pub extern "C" fn mmtk_add_obj_free_candidate(object: ObjectReference, can_parallel_free: bool) { - binding() - .weak_proc - .add_obj_free_candidate(object, can_parallel_free) +pub extern "C" fn mmtk_add_obj_free_candidate(object: ObjectReference) { + binding().weak_proc.add_obj_free_candidate(object) } // =============== Weak references =============== diff --git a/gc/mmtk/src/weak_proc.rs b/gc/mmtk/src/weak_proc.rs index 41b133a4945506..3184c4f6d183f2 100644 --- a/gc/mmtk/src/weak_proc.rs +++ b/gc/mmtk/src/weak_proc.rs @@ -1,5 +1,3 @@ -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::Ordering; use std::sync::Mutex; use mmtk::{ @@ -11,13 +9,10 @@ use mmtk::{ use crate::{abi::GCThreadTLS, upcalls, Ruby}; pub struct WeakProcessor { - non_parallel_obj_free_candidates: Mutex>, - parallel_obj_free_candidates: Vec>>, - parallel_obj_free_candidates_counter: AtomicUsize, - /// Objects that needs `obj_free` called when dying. /// If it is a bottleneck, replace it with a lock-free data structure, /// or add candidates in batch. + obj_free_candidates: Mutex>, weak_references: Mutex>, } @@ -30,59 +25,32 @@ impl Default for WeakProcessor { impl WeakProcessor { pub fn new() -> Self { Self { - non_parallel_obj_free_candidates: Mutex::new(Vec::new()), - parallel_obj_free_candidates: vec![Mutex::new(Vec::new())], - parallel_obj_free_candidates_counter: AtomicUsize::new(0), + obj_free_candidates: Mutex::new(Vec::new()), weak_references: Mutex::new(Vec::new()), } } - pub fn init_parallel_obj_free_candidates(&mut self, num_workers: usize) { - debug_assert_eq!(self.parallel_obj_free_candidates.len(), 1); - - for _ in 1..num_workers { - self.parallel_obj_free_candidates - .push(Mutex::new(Vec::new())); - } + /// Add an object as a candidate for `obj_free`. + /// + /// Multiple mutators can call it concurrently, so it has `&self`. + pub fn add_obj_free_candidate(&self, object: ObjectReference) { + let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); + obj_free_candidates.push(object); } - /// Add an object as a candidate for `obj_free`. + /// Add many objects as candidates for `obj_free`. /// /// Multiple mutators can call it concurrently, so it has `&self`. - pub fn add_obj_free_candidate(&self, object: ObjectReference, can_parallel_free: bool) { - if can_parallel_free { - // Newly allocated objects are placed in parallel_obj_free_candidates using - // round-robin. This may not be ideal for load balancing. - let idx = self - .parallel_obj_free_candidates_counter - .fetch_add(1, Ordering::Relaxed) - % self.parallel_obj_free_candidates.len(); - - self.parallel_obj_free_candidates[idx] - .lock() - .unwrap() - .push(object); - } else { - self.non_parallel_obj_free_candidates - .lock() - .unwrap() - .push(object); + pub fn add_obj_free_candidates(&self, objects: &[ObjectReference]) { + let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); + for object in objects.iter().copied() { + obj_free_candidates.push(object); } } pub fn get_all_obj_free_candidates(&self) -> Vec { - // let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); - let mut all_obj_free_candidates = self - .non_parallel_obj_free_candidates - .lock() - .unwrap() - .to_vec(); - - for candidates_mutex in &self.parallel_obj_free_candidates { - all_obj_free_candidates.extend(candidates_mutex.lock().unwrap().to_vec()); - } - - std::mem::take(all_obj_free_candidates.as_mut()) + let mut obj_free_candidates = self.obj_free_candidates.lock().unwrap(); + std::mem::take(obj_free_candidates.as_mut()) } pub fn add_weak_reference(&self, object: ObjectReference) { @@ -95,18 +63,7 @@ impl WeakProcessor { worker: &mut GCWorker, _tracer_context: impl ObjectTracerContext, ) { - worker.add_work( - WorkBucketStage::VMRefClosure, - ProcessNonParallelObjFreeCanadidates {}, - ); - - for index in 0..self.parallel_obj_free_candidates.len() { - worker.add_work( - WorkBucketStage::VMRefClosure, - ProcessParallelObjFreeCandidates { index }, - ); - } - + worker.add_work(WorkBucketStage::VMRefClosure, ProcessObjFreeCandidates); worker.add_work(WorkBucketStage::VMRefClosure, ProcessWeakReferences); worker.add_work(WorkBucketStage::Prepare, UpdateFinalizerObjIdTables); @@ -123,50 +80,36 @@ impl WeakProcessor { } } -fn process_obj_free_candidates(obj_free_candidates: &mut Vec) { - // Process obj_free - let mut new_candidates = Vec::new(); - - for object in obj_free_candidates.iter().copied() { - if object.is_reachable() { - // Forward and add back to the candidate list. - let new_object = object.forward(); - trace!("Forwarding obj_free candidate: {object} -> {new_object}"); - new_candidates.push(new_object); - } else { - (upcalls().call_obj_free)(object); - } - } - - *obj_free_candidates = new_candidates; -} - -struct ProcessParallelObjFreeCandidates { - index: usize, -} +struct ProcessObjFreeCandidates; -impl GCWork for ProcessParallelObjFreeCandidates { +impl GCWork for ProcessObjFreeCandidates { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static mmtk::MMTK) { - let mut obj_free_candidates = crate::binding().weak_proc.parallel_obj_free_candidates - [self.index] + // If it blocks, it is a bug. + let mut obj_free_candidates = crate::binding() + .weak_proc + .obj_free_candidates .try_lock() - .expect("Lock for parallel_obj_free_candidates should not be held"); + .expect("It's GC time. No mutators should hold this lock at this time."); - process_obj_free_candidates(&mut obj_free_candidates); - } -} + let n_cands = obj_free_candidates.len(); -struct ProcessNonParallelObjFreeCanadidates; + debug!("Total: {n_cands} candidates"); -impl GCWork for ProcessNonParallelObjFreeCanadidates { - fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static mmtk::MMTK) { - let mut obj_free_candidates = crate::binding() - .weak_proc - .non_parallel_obj_free_candidates - .try_lock() - .expect("Lock for non_parallel_obj_free_candidates should not be held"); + // Process obj_free + let mut new_candidates = Vec::new(); + + for object in obj_free_candidates.iter().copied() { + if object.is_reachable() { + // Forward and add back to the candidate list. + let new_object = object.forward(); + trace!("Forwarding obj_free candidate: {object} -> {new_object}"); + new_candidates.push(new_object); + } else { + (upcalls().call_obj_free)(object); + } + } - process_obj_free_candidates(&mut obj_free_candidates); + *obj_free_candidates = new_candidates; } } From 7cf6cc83f3d34dbff6a30457fb008d87106f5e0c Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 31 Dec 2025 11:36:19 -0500 Subject: [PATCH 4/4] Register imemo_ment as a pinning object It sometimes pins itself when it is in the overloaded_cme table. --- vm_method.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vm_method.c b/vm_method.c index 26dbe4cae8b416..dcf35527f7b956 100644 --- a/vm_method.c +++ b/vm_method.c @@ -1119,6 +1119,10 @@ rb_method_entry_alloc(ID called_id, VALUE owner, VALUE defined_class, rb_method_ VM_ASSERT_TYPE2(defined_class, T_CLASS, T_ICLASS); } rb_method_entry_t *me = SHAREABLE_IMEMO_NEW(rb_method_entry_t, imemo_ment, defined_class); + + // mark_and_move_method_entry pins itself when it is in the overloaded_cme table + rb_gc_register_pinning_obj((VALUE)me); + *((rb_method_definition_t **)&me->def) = def; me->called_id = called_id; me->owner = owner;