From ac8315476e11cee4c61474990c40bd432bfc9d42 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 10 Jan 2024 20:35:28 +0800 Subject: [PATCH 1/5] Remove coordinator (controller) thread. --- mmtk/Cargo.lock | 2 -- mmtk/Cargo.toml | 6 +++--- mmtk/src/abi.rs | 9 ++------- mmtk/src/collection.rs | 23 ----------------------- 4 files changed, 5 insertions(+), 35 deletions(-) diff --git a/mmtk/Cargo.lock b/mmtk/Cargo.lock index 1e923e5..c280806 100644 --- a/mmtk/Cargo.lock +++ b/mmtk/Cargo.lock @@ -376,7 +376,6 @@ dependencies = [ [[package]] name = "mmtk" version = "0.22.0" -source = "git+https://github.com/mmtk/mmtk-core.git?rev=b66fa351c6e37e4a0672070aab5cd5864d150890#b66fa351c6e37e4a0672070aab5cd5864d150890" dependencies = [ "atomic", "atomic-traits", @@ -411,7 +410,6 @@ dependencies = [ [[package]] name = "mmtk-macros" version = "0.22.0" -source = "git+https://github.com/mmtk/mmtk-core.git?rev=b66fa351c6e37e4a0672070aab5cd5864d150890#b66fa351c6e37e4a0672070aab5cd5864d150890" dependencies = [ "proc-macro-error", "proc-macro2", diff --git a/mmtk/Cargo.toml b/mmtk/Cargo.toml index 54dfd0b..74382fe 100644 --- a/mmtk/Cargo.toml +++ b/mmtk/Cargo.toml @@ -36,11 +36,11 @@ probe = "0.5" features = ["is_mmtk_object", "object_pinning"] # Uncomment the following lines to use mmtk-core from the official repository. -git = "https://github.com/mmtk/mmtk-core.git" -rev = "b66fa351c6e37e4a0672070aab5cd5864d150890" +#git = "https://github.com/mmtk/mmtk-core.git" +#rev = "b66fa351c6e37e4a0672070aab5cd5864d150890" # Uncomment the following line to use mmtk-core from a local repository. -#path = "../../mmtk-core" +path = "../../mmtk-core" [features] default = [] diff --git a/mmtk/src/abi.rs b/mmtk/src/abi.rs index a67faaf..9451e1c 100644 --- a/mmtk/src/abi.rs +++ b/mmtk/src/abi.rs @@ -1,13 +1,12 @@ use crate::api::RubyMutator; use crate::{upcalls, Ruby}; -use mmtk::scheduler::{GCController, GCWorker}; +use mmtk::scheduler::GCWorker; use mmtk::util::{Address, ObjectReference, VMMutatorThread, VMWorkerThread}; // For the C binding pub const OBJREF_OFFSET: usize = 8; pub const MIN_OBJ_ALIGN: usize = 8; // Even on 32-bit machine. A Ruby object is at least 40 bytes large. -pub const GC_THREAD_KIND_CONTROLLER: libc::c_int = 0; pub const GC_THREAD_KIND_WORKER: libc::c_int = 1; const HAS_MOVED_GIVTBL: usize = 1 << 63; @@ -244,10 +243,6 @@ impl GCThreadTLS { } } - pub fn for_controller(gc_context: *mut GCController) -> Self { - Self::new(GC_THREAD_KIND_CONTROLLER, gc_context as *mut libc::c_void) - } - pub fn for_worker(gc_context: *mut GCWorker) -> Self { Self::new(GC_THREAD_KIND_WORKER, gc_context as *mut libc::c_void) } @@ -266,7 +261,7 @@ impl GCThreadTLS { let result = &mut *ptr; debug_assert!({ let kind = result.kind; - kind == GC_THREAD_KIND_CONTROLLER || kind == GC_THREAD_KIND_WORKER + kind == GC_THREAD_KIND_WORKER }); result } diff --git a/mmtk/src/collection.rs b/mmtk/src/collection.rs index 2b788bb..d6a970c 100644 --- a/mmtk/src/collection.rs +++ b/mmtk/src/collection.rs @@ -33,29 +33,6 @@ impl Collection for VMCollection { fn spawn_gc_thread(_tls: VMThread, ctx: GCThreadContext) { match ctx { - GCThreadContext::Controller(mut controller) => { - thread::Builder::new() - .name("MMTk Controller Thread".to_string()) - .spawn(move || { - debug!("Hello! This is MMTk Controller Thread running!"); - crate::register_gc_thread(thread::current().id()); - let ptr_controller = &mut *controller as *mut GCController; - let gc_thread_tls = - Box::into_raw(Box::new(GCThreadTLS::for_controller(ptr_controller))); - (upcalls().init_gc_worker_thread)(gc_thread_tls); - memory_manager::start_control_collector( - mmtk(), - GCThreadTLS::to_vwt(gc_thread_tls), - &mut controller, - ); - - // Currently the MMTk controller thread should run forever. - // This is an unlikely event, but we log it anyway. - warn!("The MMTk Controller Thread is quitting!"); - crate::unregister_gc_thread(thread::current().id()); - }) - .unwrap(); - } GCThreadContext::Worker(mut worker) => { thread::Builder::new() .name("MMTk Worker Thread".to_string()) From 9b158ec40f26297c1b79908f16c013acecf0c131 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 18 Jan 2024 16:18:44 +0800 Subject: [PATCH 2/5] Upstream API change --- mmtk/Cargo.lock | 4 ++-- mmtk/src/api.rs | 5 +++++ mmtk/src/collection.rs | 16 ++++++++++------ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/mmtk/Cargo.lock b/mmtk/Cargo.lock index c280806..25c48a2 100644 --- a/mmtk/Cargo.lock +++ b/mmtk/Cargo.lock @@ -375,7 +375,7 @@ dependencies = [ [[package]] name = "mmtk" -version = "0.22.0" +version = "0.22.1" dependencies = [ "atomic", "atomic-traits", @@ -409,7 +409,7 @@ dependencies = [ [[package]] name = "mmtk-macros" -version = "0.22.0" +version = "0.22.1" dependencies = [ "proc-macro-error", "proc-macro2", diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index 601326f..ed337c7 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -154,6 +154,11 @@ pub extern "C" fn mmtk_initialize_collection(tls: VMThread) { memory_manager::initialize_collection(mmtk(), tls) } +#[no_mangle] +pub extern "C" fn mmtk_uninitialize_collection() { + memory_manager::uninitialize_collection(mmtk()) +} + #[no_mangle] pub extern "C" fn mmtk_enable_collection() { memory_manager::enable_collection(mmtk()) diff --git a/mmtk/src/collection.rs b/mmtk/src/collection.rs index d6a970c..584c0d7 100644 --- a/mmtk/src/collection.rs +++ b/mmtk/src/collection.rs @@ -37,7 +37,11 @@ impl Collection for VMCollection { thread::Builder::new() .name("MMTk Worker Thread".to_string()) .spawn(move || { - debug!("Hello! This is MMTk Worker Thread running!"); + let ordinal = worker.ordinal; + debug!( + "Hello! This is MMTk Worker Thread running! ordinal: {}", + ordinal + ); crate::register_gc_thread(thread::current().id()); let ptr_worker = &mut *worker as *mut GCWorker; let gc_thread_tls = @@ -46,12 +50,12 @@ impl Collection for VMCollection { memory_manager::start_worker( mmtk(), GCThreadTLS::to_vwt(gc_thread_tls), - &mut worker, + worker, + ); + debug!( + "An MMTk Worker Thread is quitting. Good bye! ordinal: {}", + ordinal ); - - // Currently all MMTk worker threads should run forever. - // This is an unlikely event, but we log it anyway. - warn!("An MMTk Worker Thread is quitting!"); crate::unregister_gc_thread(thread::current().id()); }) .unwrap(); From 411208aa7ae7e42b0811d8f781603999bc00d28c Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 1 Feb 2024 13:45:57 +0800 Subject: [PATCH 3/5] Use git rev for mmtk-core --- mmtk/Cargo.lock | 2 ++ mmtk/Cargo.toml | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/mmtk/Cargo.lock b/mmtk/Cargo.lock index b52b0a7..25fe2ab 100644 --- a/mmtk/Cargo.lock +++ b/mmtk/Cargo.lock @@ -376,6 +376,7 @@ dependencies = [ [[package]] name = "mmtk" version = "0.22.1" +source = "git+https://github.com/wks/mmtk-core.git?rev=7cb0b2c12be341c084e03f2cd943fad8ac088f83#7cb0b2c12be341c084e03f2cd943fad8ac088f83" dependencies = [ "atomic", "atomic-traits", @@ -410,6 +411,7 @@ dependencies = [ [[package]] name = "mmtk-macros" version = "0.22.1" +source = "git+https://github.com/wks/mmtk-core.git?rev=7cb0b2c12be341c084e03f2cd943fad8ac088f83#7cb0b2c12be341c084e03f2cd943fad8ac088f83" dependencies = [ "proc-macro-error", "proc-macro2", diff --git a/mmtk/Cargo.toml b/mmtk/Cargo.toml index 547bfe1..4394759 100644 --- a/mmtk/Cargo.toml +++ b/mmtk/Cargo.toml @@ -36,11 +36,11 @@ probe = "0.5" features = ["is_mmtk_object", "object_pinning"] # Uncomment the following lines to use mmtk-core from the official repository. -#git = "https://github.com/mmtk/mmtk-core.git" -#rev = "42754a5f23fdb513039d7f07e52014ded42c98bf" +git = "https://github.com/wks/mmtk-core.git" +rev = "7cb0b2c12be341c084e03f2cd943fad8ac088f83" # Uncomment the following line to use mmtk-core from a local repository. -path = "../../mmtk-core" +#path = "../../mmtk-core" [features] default = [] From 5d6b0fa3c181f412c1a7427bc59b96f15395f38f Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 5 Feb 2024 17:28:48 +0800 Subject: [PATCH 4/5] Replace uninitialize_collection with prepare/after fork. --- mmtk/Cargo.lock | 2 -- mmtk/Cargo.toml | 6 +++--- mmtk/src/api.rs | 9 +++++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/mmtk/Cargo.lock b/mmtk/Cargo.lock index 25fe2ab..b52b0a7 100644 --- a/mmtk/Cargo.lock +++ b/mmtk/Cargo.lock @@ -376,7 +376,6 @@ dependencies = [ [[package]] name = "mmtk" version = "0.22.1" -source = "git+https://github.com/wks/mmtk-core.git?rev=7cb0b2c12be341c084e03f2cd943fad8ac088f83#7cb0b2c12be341c084e03f2cd943fad8ac088f83" dependencies = [ "atomic", "atomic-traits", @@ -411,7 +410,6 @@ dependencies = [ [[package]] name = "mmtk-macros" version = "0.22.1" -source = "git+https://github.com/wks/mmtk-core.git?rev=7cb0b2c12be341c084e03f2cd943fad8ac088f83#7cb0b2c12be341c084e03f2cd943fad8ac088f83" dependencies = [ "proc-macro-error", "proc-macro2", diff --git a/mmtk/Cargo.toml b/mmtk/Cargo.toml index 4394759..1cdc999 100644 --- a/mmtk/Cargo.toml +++ b/mmtk/Cargo.toml @@ -36,11 +36,11 @@ probe = "0.5" features = ["is_mmtk_object", "object_pinning"] # Uncomment the following lines to use mmtk-core from the official repository. -git = "https://github.com/wks/mmtk-core.git" -rev = "7cb0b2c12be341c084e03f2cd943fad8ac088f83" +#git = "https://github.com/wks/mmtk-core.git" +#rev = "7cb0b2c12be341c084e03f2cd943fad8ac088f83" # Uncomment the following line to use mmtk-core from a local repository. -#path = "../../mmtk-core" +path = "../../mmtk-core" [features] default = [] diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index ed337c7..f51e1da 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -155,8 +155,13 @@ pub extern "C" fn mmtk_initialize_collection(tls: VMThread) { } #[no_mangle] -pub extern "C" fn mmtk_uninitialize_collection() { - memory_manager::uninitialize_collection(mmtk()) +pub extern "C" fn mmtk_prepare_to_fork() { + mmtk().prepare_to_fork(); +} + +#[no_mangle] +pub extern "C" fn mmtk_after_fork(tls: VMThread) { + mmtk().after_fork(tls); } #[no_mangle] From 06753f0309a785bcb5d22ffd41715677eb1ef38e Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 6 Feb 2024 17:54:49 +0800 Subject: [PATCH 5/5] Join GC threads --- mmtk/src/api.rs | 1 + mmtk/src/binding.rs | 19 ++++++++++++++ mmtk/src/collection.rs | 59 ++++++++++++++++++++++-------------------- 3 files changed, 51 insertions(+), 28 deletions(-) diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index f51e1da..150293e 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -157,6 +157,7 @@ pub extern "C" fn mmtk_initialize_collection(tls: VMThread) { #[no_mangle] pub extern "C" fn mmtk_prepare_to_fork() { mmtk().prepare_to_fork(); + binding().join_all_gc_threads(); } #[no_mangle] diff --git a/mmtk/src/binding.rs b/mmtk/src/binding.rs index 8acc9a1..af0969a 100644 --- a/mmtk/src/binding.rs +++ b/mmtk/src/binding.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::ffi::CString; use std::sync::Mutex; +use std::thread::JoinHandle; use libc::c_void; use mmtk::util::ObjectReference; @@ -36,6 +37,7 @@ pub struct RubyBinding { pub weak_proc: WeakProcessor, pub ppp_registry: PPPRegistry, pub(crate) moved_givtbl: Mutex>, + pub gc_thread_join_handles: Mutex>> } unsafe impl Sync for RubyBinding {} @@ -58,6 +60,7 @@ impl RubyBinding { weak_proc: WeakProcessor::new(), ppp_registry: PPPRegistry::new(), moved_givtbl: Default::default(), + gc_thread_join_handles: Default::default(), } } @@ -75,4 +78,20 @@ impl RubyBinding { } plan_name.as_deref().unwrap().as_ptr() } + + pub fn join_all_gc_threads(&self) { + let handles = { + let mut guard = self.gc_thread_join_handles.lock().unwrap(); + std::mem::take(&mut *guard) + }; + + debug!("Joining GC threads..."); + let total = handles.len(); + let mut joined = 0; + for handle in handles { + handle.join().unwrap(); + joined += 1; + debug!("{joined}/{total} GC threads joined."); + } + } } diff --git a/mmtk/src/collection.rs b/mmtk/src/collection.rs index 584c0d7..42d5403 100644 --- a/mmtk/src/collection.rs +++ b/mmtk/src/collection.rs @@ -32,34 +32,37 @@ impl Collection for VMCollection { } fn spawn_gc_thread(_tls: VMThread, ctx: GCThreadContext) { - match ctx { - GCThreadContext::Worker(mut worker) => { - thread::Builder::new() - .name("MMTk Worker Thread".to_string()) - .spawn(move || { - let ordinal = worker.ordinal; - debug!( - "Hello! This is MMTk Worker Thread running! ordinal: {}", - ordinal - ); - crate::register_gc_thread(thread::current().id()); - let ptr_worker = &mut *worker as *mut GCWorker; - let gc_thread_tls = - Box::into_raw(Box::new(GCThreadTLS::for_worker(ptr_worker))); - (upcalls().init_gc_worker_thread)(gc_thread_tls); - memory_manager::start_worker( - mmtk(), - GCThreadTLS::to_vwt(gc_thread_tls), - worker, - ); - debug!( - "An MMTk Worker Thread is quitting. Good bye! ordinal: {}", - ordinal - ); - crate::unregister_gc_thread(thread::current().id()); - }) - .unwrap(); - } + let join_handle = match ctx { + GCThreadContext::Worker(mut worker) => thread::Builder::new() + .name("MMTk Worker Thread".to_string()) + .spawn(move || { + let ordinal = worker.ordinal; + debug!( + "Hello! This is MMTk Worker Thread running! ordinal: {}", + ordinal + ); + crate::register_gc_thread(thread::current().id()); + let ptr_worker = &mut *worker as *mut GCWorker; + let gc_thread_tls = + Box::into_raw(Box::new(GCThreadTLS::for_worker(ptr_worker))); + (upcalls().init_gc_worker_thread)(gc_thread_tls); + memory_manager::start_worker( + mmtk(), + GCThreadTLS::to_vwt(gc_thread_tls), + worker, + ); + debug!( + "An MMTk Worker Thread is quitting. Good bye! ordinal: {}", + ordinal + ); + crate::unregister_gc_thread(thread::current().id()); + }) + .unwrap(), + }; + + { + let mut handles = crate::binding().gc_thread_join_handles.lock().unwrap(); + handles.push(join_handle); } }