From 4be3884d7a075376b68719c89b762c47e8640224 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Sat, 6 Jul 2024 08:04:07 +0800 Subject: [PATCH 1/2] WIP: Generational code cache roots --- .gitignore | 4 +- mmtk/Cargo.lock | 13 ++--- mmtk/Cargo.toml | 5 +- mmtk/src/api.rs | 35 +++++--------- mmtk/src/gc_work.rs | 53 +++++++++++++++++---- mmtk/src/lib.rs | 12 ++--- mmtk/src/scanning.rs | 2 +- tools/tracing/timeline/capture_openjdk.bt | 5 ++ tools/tracing/timeline/visualize_openjdk.py | 13 +++++ 9 files changed, 94 insertions(+), 48 deletions(-) create mode 100644 tools/tracing/timeline/capture_openjdk.bt create mode 100755 tools/tracing/timeline/visualize_openjdk.py diff --git a/.gitignore b/.gitignore index c081d423..a25b809e 100644 --- a/.gitignore +++ b/.gitignore @@ -15,4 +15,6 @@ target/ /*.dylib /*.so -/repos/mmtk-core \ No newline at end of file +/repos/mmtk-core + +__pycache__ diff --git a/mmtk/Cargo.lock b/mmtk/Cargo.lock index ff7149de..008b7327 100644 --- a/mmtk/Cargo.lock +++ b/mmtk/Cargo.lock @@ -108,9 +108,9 @@ dependencies = [ [[package]] name = "bytemuck" -version = "1.17.1" +version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "773d90827bc3feecfb67fab12e24de0749aad83c74b9504ecde46237b5cd24e2" +checksum = "94bbb0ad554ad961ddc5da507a12a29b14e4ae5bda06b19f575a3e6079d2e2ae" dependencies = [ "bytemuck_derive", ] @@ -128,9 +128,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.1.16" +version = "1.1.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e9d013ecb737093c0e86b151a7b837993cf9ec6c502946cfb44bedc392421e0b" +checksum = "b62ac837cdb5cb22e10a256099b4fc502b1dfe560cb282963a974d7abd80e476" dependencies = [ "jobserver", "libc", @@ -438,7 +438,7 @@ dependencies = [ [[package]] name = "mmtk" version = "0.27.0" -source = "git+https://github.com/mmtk/mmtk-core.git?rev=45cdf31055b1b6a629bdb8032adaa6dd5a8e32b9#45cdf31055b1b6a629bdb8032adaa6dd5a8e32b9" +source = "git+https://github.com/mmtk/mmtk-core.git?rev=a025c24104d8d456a865aa0122e6e0fb6d77e8f2#a025c24104d8d456a865aa0122e6e0fb6d77e8f2" dependencies = [ "atomic", "atomic-traits", @@ -474,7 +474,7 @@ dependencies = [ [[package]] name = "mmtk-macros" version = "0.27.0" -source = "git+https://github.com/mmtk/mmtk-core.git?rev=45cdf31055b1b6a629bdb8032adaa6dd5a8e32b9#45cdf31055b1b6a629bdb8032adaa6dd5a8e32b9" +source = "git+https://github.com/mmtk/mmtk-core.git?rev=a025c24104d8d456a865aa0122e6e0fb6d77e8f2#a025c24104d8d456a865aa0122e6e0fb6d77e8f2" dependencies = [ "proc-macro-error", "proc-macro2", @@ -494,6 +494,7 @@ dependencies = [ "memoffset", "mmtk", "once_cell", + "probe", ] [[package]] diff --git a/mmtk/Cargo.toml b/mmtk/Cargo.toml index f53eb549..9240d84b 100644 --- a/mmtk/Cargo.toml +++ b/mmtk/Cargo.toml @@ -2,7 +2,7 @@ name = "mmtk_openjdk" version = "0.27.0" authors = [" <>"] -rust-version = "1.71.1" +rust-version = "1.73.0" build = "build.rs" edition = "2021" @@ -27,6 +27,7 @@ once_cell = "1.10.0" atomic = "0.6.0" memoffset = "0.9.0" cfg-if = "1.0" +probe = "0.5" # Be very careful to commit any changes to the following mmtk dependency, as our CI scripts (including mmtk-core CI) # rely on matching these lines to modify them: e.g. comment out the git dependency and use the local path. @@ -34,7 +35,7 @@ cfg-if = "1.0" # - change branch # - change repo name # But other changes including adding/removing whitespaces in commented lines may break the CI. -mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "45cdf31055b1b6a629bdb8032adaa6dd5a8e32b9" } +mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "a025c24104d8d456a865aa0122e6e0fb6d77e8f2" } # Uncomment the following to build locally # mmtk = { path = "../repos/mmtk-core" } diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index 8ca52ae2..32fbdefb 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -497,7 +497,7 @@ thread_local! { /// Report a list of pointers in nmethod to mmtk. #[no_mangle] pub extern "C" fn mmtk_add_nmethod_oop(addr: Address) { - NMETHOD_SLOTS.with(|x| x.borrow_mut().push(addr)) + NMETHOD_SLOTS.with_borrow_mut(|x| x.push(addr)) } /// Register a nmethod. @@ -505,34 +505,23 @@ pub extern "C" fn mmtk_add_nmethod_oop(addr: Address) { /// This function will transfer all the locally cached pointers of this nmethod to the global storage. #[no_mangle] pub extern "C" fn mmtk_register_nmethod(nm: Address) { - let slots = NMETHOD_SLOTS.with(|x| { - if x.borrow().len() == 0 { - return None; + NMETHOD_SLOTS.with_borrow_mut(|slots| { + if !slots.is_empty() { + let mut roots = crate::NURSERY_CODE_CACHE_ROOTS.lock().unwrap(); + roots.insert(nm, std::mem::take(slots)); } - Some(x.replace(vec![])) }); - let slots = match slots { - Some(slots) => slots, - _ => return, - }; - let mut roots = crate::CODE_CACHE_ROOTS.lock().unwrap(); - // Relaxed add instead of `fetch_add`, since we've already acquired the lock. - crate::CODE_CACHE_ROOTS_SIZE.store( - crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed) + slots.len(), - Ordering::Relaxed, - ); - roots.insert(nm, slots); } /// Unregister a nmethod. #[no_mangle] pub extern "C" fn mmtk_unregister_nmethod(nm: Address) { - let mut roots = crate::CODE_CACHE_ROOTS.lock().unwrap(); - if let Some(slots) = roots.remove(&nm) { - // Relaxed sub instead of `fetch_sub`, since we've already acquired the lock. - crate::CODE_CACHE_ROOTS_SIZE.store( - crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed) - slots.len(), - Ordering::Relaxed, - ); + { + let mut roots = crate::NURSERY_CODE_CACHE_ROOTS.lock().unwrap(); + roots.remove(&nm); + } + { + let mut roots = crate::MATURE_CODE_CACHE_ROOTS.lock().unwrap(); + roots.remove(&nm); } } diff --git a/mmtk/src/gc_work.rs b/mmtk/src/gc_work.rs index 239e1ae5..01b9bc76 100644 --- a/mmtk/src/gc_work.rs +++ b/mmtk/src/gc_work.rs @@ -1,10 +1,10 @@ -use std::sync::atomic::Ordering; - +use crate::scanning; use crate::scanning::to_slots_closure; use crate::OpenJDK; use crate::OpenJDKSlot; use crate::UPCALLS; use mmtk::scheduler::*; +use mmtk::util::Address; use mmtk::vm::RootsWorkFactory; use mmtk::vm::*; use mmtk::MMTK; @@ -69,16 +69,51 @@ impl>> fn do_work( &mut self, _worker: &mut GCWorker>, - _mmtk: &'static MMTK>, + mmtk: &'static MMTK>, ) { - // Collect all the cached roots - let mut slots = Vec::with_capacity(crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed)); - for roots in (*crate::CODE_CACHE_ROOTS.lock().unwrap()).values() { - for r in roots { - slots.push((*r).into()) + let is_current_gc_nursery = mmtk + .get_plan() + .generational() + .is_some_and(|gen| gen.is_current_gc_nursery()); + + let mut slots = Vec::with_capacity(scanning::WORK_PACKET_CAPACITY); + + let mut nursery_slots = 0; + let mut mature_slots = 0; + + let mut add_roots = |roots: &[Address]| { + for root in roots { + slots.push(OpenJDKSlot::::from(*root)); + if slots.len() >= scanning::WORK_PACKET_CAPACITY { + self.factory + .create_process_roots_work(std::mem::take(&mut slots)); + } + } + }; + + { + let mut mature = crate::MATURE_CODE_CACHE_ROOTS.lock().unwrap(); + + // Only scan mature roots in full-heap collections. + if !is_current_gc_nursery { + for roots in mature.values() { + mature_slots += roots.len(); + add_roots(roots); + } + } + + { + let mut nursery = crate::NURSERY_CODE_CACHE_ROOTS.lock().unwrap(); + for (key, roots) in nursery.drain() { + nursery_slots += roots.len(); + add_roots(&roots); + mature.insert(key, roots); + } } } - // Create work packet + + probe!(mmtk_openjdk, code_cache_roots, nursery_slots, mature_slots); + if !slots.is_empty() { self.factory.create_process_roots_work(slots); } diff --git a/mmtk/src/lib.rs b/mmtk/src/lib.rs index e4259da9..4bb8b194 100644 --- a/mmtk/src/lib.rs +++ b/mmtk/src/lib.rs @@ -1,9 +1,10 @@ #[macro_use] extern crate lazy_static; +#[macro_use] +extern crate probe; use std::collections::HashMap; use std::ptr::null_mut; -use std::sync::atomic::AtomicUsize; use std::sync::Mutex; use libc::{c_char, c_void, uintptr_t}; @@ -202,13 +203,12 @@ pub static MMTK_MARK_COMPACT_HEADER_RESERVED_IN_BYTES: usize = mmtk::util::alloc::MarkCompactAllocator::>::HEADER_RESERVED_IN_BYTES; lazy_static! { - /// A global storage for all the cached CodeCache root pointers - static ref CODE_CACHE_ROOTS: Mutex>> = Mutex::new(HashMap::new()); + /// A global storage for all the cached CodeCache root pointers added since the last GC. + static ref NURSERY_CODE_CACHE_ROOTS: Mutex>> = Mutex::new(HashMap::new()); + /// A global storage for all the cached CodeCache root pointers added before the last GC. + static ref MATURE_CODE_CACHE_ROOTS: Mutex>> = Mutex::new(HashMap::new()); } -/// A counter tracking the total size of the `CODE_CACHE_ROOTS`. -static CODE_CACHE_ROOTS_SIZE: AtomicUsize = AtomicUsize::new(0); - fn set_compressed_pointer_vm_layout(builder: &mut MMTKBuilder) { let max_heap_size = builder.options.gc_trigger.max_heap_size(); assert!( diff --git a/mmtk/src/scanning.rs b/mmtk/src/scanning.rs index dc94cf87..528b940d 100644 --- a/mmtk/src/scanning.rs +++ b/mmtk/src/scanning.rs @@ -12,7 +12,7 @@ use mmtk::MutatorContext; pub struct VMScanning {} -const WORK_PACKET_CAPACITY: usize = 4096; +pub(crate) const WORK_PACKET_CAPACITY: usize = 4096; extern "C" fn report_slots_and_renew_buffer>( ptr: *mut Address, diff --git a/tools/tracing/timeline/capture_openjdk.bt b/tools/tracing/timeline/capture_openjdk.bt new file mode 100644 index 00000000..0398f815 --- /dev/null +++ b/tools/tracing/timeline/capture_openjdk.bt @@ -0,0 +1,5 @@ +usdt:$MMTK:mmtk_openjdk:code_cache_roots { + if (@enable_print) { + printf("code_cache_roots,meta,%d,%lu,%lu,%lu\n", tid, nsecs, arg0, arg1); + } +} diff --git a/tools/tracing/timeline/visualize_openjdk.py b/tools/tracing/timeline/visualize_openjdk.py new file mode 100755 index 00000000..cef1ff73 --- /dev/null +++ b/tools/tracing/timeline/visualize_openjdk.py @@ -0,0 +1,13 @@ +#!/usr/bin/env python3 + +def enrich_meta_extra(log_processor, name, tid, ts, gc, wp, args): + if wp is not None: + match name: + case "code_cache_roots": + nursery, mature = int(args[0]), int(args[1]) + total = nursery + mature + wp["args"] |= { + "nursery_slots": nursery, + "mature_slots": mature, + "total_slots": total, + } From a8a5fd60fd9fbc60eee8a06999a97617e74e33a0 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 9 Sep 2024 16:27:25 +0800 Subject: [PATCH 2/2] Adjust comments --- mmtk/src/api.rs | 15 +++++++++------ mmtk/src/gc_work.rs | 2 +- mmtk/src/lib.rs | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index 32fbdefb..4e619e2b 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -490,19 +490,22 @@ pub extern "C" fn get_finalized_object() -> NullableObjectReference { } thread_local! { - /// Cache all the pointers reported by the current thread. + /// Cache reference slots of an nmethod while the current thread is executing + /// `MMTkRegisterNMethodOopClosure`. static NMETHOD_SLOTS: RefCell> = const { RefCell::new(vec![]) }; } -/// Report a list of pointers in nmethod to mmtk. +/// Report one reference slot in an nmethod to MMTk. #[no_mangle] pub extern "C" fn mmtk_add_nmethod_oop(addr: Address) { NMETHOD_SLOTS.with_borrow_mut(|x| x.push(addr)) } -/// Register a nmethod. -/// The c++ part of the binding should scan the nmethod and report all the pointers to mmtk first, before calling this function. -/// This function will transfer all the locally cached pointers of this nmethod to the global storage. +/// Register an nmethod. +/// +/// The C++ part of the binding should have scanned the nmethod and reported all the reference slots +/// using `mmtk_add_nmethod_oop` before calling this function. This function will transfer all the +/// locally cached slots of this nmethod to the global storage. #[no_mangle] pub extern "C" fn mmtk_register_nmethod(nm: Address) { NMETHOD_SLOTS.with_borrow_mut(|slots| { @@ -513,7 +516,7 @@ pub extern "C" fn mmtk_register_nmethod(nm: Address) { }); } -/// Unregister a nmethod. +/// Unregister an nmethod. #[no_mangle] pub extern "C" fn mmtk_unregister_nmethod(nm: Address) { { diff --git a/mmtk/src/gc_work.rs b/mmtk/src/gc_work.rs index 01b9bc76..bde3aff4 100644 --- a/mmtk/src/gc_work.rs +++ b/mmtk/src/gc_work.rs @@ -76,7 +76,7 @@ impl>> .generational() .is_some_and(|gen| gen.is_current_gc_nursery()); - let mut slots = Vec::with_capacity(scanning::WORK_PACKET_CAPACITY); + let mut slots = Vec::with_capacity(scanning::WORK_PACKET_CAPACITY); let mut nursery_slots = 0; let mut mature_slots = 0; diff --git a/mmtk/src/lib.rs b/mmtk/src/lib.rs index 4bb8b194..3b1f54a2 100644 --- a/mmtk/src/lib.rs +++ b/mmtk/src/lib.rs @@ -203,9 +203,9 @@ pub static MMTK_MARK_COMPACT_HEADER_RESERVED_IN_BYTES: usize = mmtk::util::alloc::MarkCompactAllocator::>::HEADER_RESERVED_IN_BYTES; lazy_static! { - /// A global storage for all the cached CodeCache root pointers added since the last GC. + /// A global storage for all the cached CodeCache roots added since the last GC. static ref NURSERY_CODE_CACHE_ROOTS: Mutex>> = Mutex::new(HashMap::new()); - /// A global storage for all the cached CodeCache root pointers added before the last GC. + /// A global storage for all the cached CodeCache roots added before the last GC. static ref MATURE_CODE_CACHE_ROOTS: Mutex>> = Mutex::new(HashMap::new()); }