From fa1a26e101f2cbd289a6068892a52a37a9c68318 Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Tue, 28 Jun 2022 10:24:40 +1000 Subject: [PATCH 1/8] Fix root scanning --- mmtk/Cargo.toml | 1 + mmtk/src/api.rs | 34 ++++++++++++++ mmtk/src/gc_work.rs | 11 +++-- mmtk/src/lib.rs | 15 ++++--- mmtk/src/scanning.rs | 11 ++++- openjdk/mmtk.h | 11 ++--- openjdk/mmtkHeap.cpp | 99 ++++++++--------------------------------- openjdk/mmtkHeap.hpp | 6 +-- openjdk/mmtkUpcalls.cpp | 32 ++++--------- 9 files changed, 98 insertions(+), 122 deletions(-) diff --git a/mmtk/Cargo.toml b/mmtk/Cargo.toml index f20f082e..9b225341 100644 --- a/mmtk/Cargo.toml +++ b/mmtk/Cargo.toml @@ -22,6 +22,7 @@ openjdk_version = "ca90b43f0f51d9ddf754e6ab134c5030cf54118b" libc = "0.2" lazy_static = "1.1" once_cell = "1.10.0" +spin = "0.9.2" # 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. # These changes are safe: diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index 5d6b0fe9..edbe5537 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -15,7 +15,9 @@ use mmtk::Mutator; use mmtk::MutatorContext; use mmtk::MMTK; use once_cell::sync; +use std::cell::RefCell; use std::ffi::{CStr, CString}; +use std::sync::atomic::Ordering; // Supported barriers: static NO_BARRIER: sync::Lazy = sync::Lazy::new(|| CString::new("NoBarrier").unwrap()); @@ -282,3 +284,35 @@ pub extern "C" fn get_finalized_object() -> ObjectReference { None => unsafe { Address::ZERO.to_object_reference() }, } } + +thread_local! { + static NMETHOD_SLOTS: RefCell> = RefCell::new(vec![]); +} + +#[no_mangle] +pub unsafe extern "C" fn mmtk_add_nmethod_oop(addr: Address) { + NMETHOD_SLOTS.with(|x| x.borrow_mut().push(addr)) +} + +#[no_mangle] +pub unsafe extern "C" fn mmtk_register_nmethod(nm: Address) { + let slots = NMETHOD_SLOTS.with(|x| { + if x.borrow().len() == 0 { + return None; + } + Some(x.replace(vec![])) + }); + if slots.is_none() { + return; + } + let slots = slots.unwrap(); + crate::CODE_CACHE_ROOTS_SIZE.fetch_add(slots.len(), Ordering::SeqCst); + crate::CODE_CACHE_ROOTS.lock().unwrap().insert(nm, slots); +} + +#[no_mangle] +pub unsafe extern "C" fn mmtk_unregister_nmethod(nm: Address) { + if let Some(slots) = crate::CODE_CACHE_ROOTS.lock().unwrap().remove(&nm) { + crate::CODE_CACHE_ROOTS_SIZE.fetch_sub(slots.len(), Ordering::SeqCst); + } +} diff --git a/mmtk/src/gc_work.rs b/mmtk/src/gc_work.rs index ff75e4f2..ff7018ef 100644 --- a/mmtk/src/gc_work.rs +++ b/mmtk/src/gc_work.rs @@ -1,8 +1,9 @@ use super::{OpenJDK, UPCALLS}; -use crate::scanning::create_process_edges_work; +use crate::scanning::{create_process_edges_work, create_process_edges_work_vec}; use mmtk::scheduler::*; use mmtk::MMTK; use std::marker::PhantomData; +use std::sync::atomic::Ordering; pub struct ScanUniverseRoots>(PhantomData); @@ -126,9 +127,13 @@ impl> ScanCodeCacheRoots { impl> GCWork for ScanCodeCacheRoots { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { - unsafe { - ((*UPCALLS).scan_code_cache_roots)(create_process_edges_work:: as _); + let mut vec = Vec::with_capacity(crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed)); + for (_, roots) in &*crate::CODE_CACHE_ROOTS.lock().unwrap() { + for r in roots { + vec.push(*r) + } } + create_process_edges_work_vec::(vec) } } diff --git a/mmtk/src/lib.rs b/mmtk/src/lib.rs index 2174cb50..3902255e 100644 --- a/mmtk/src/lib.rs +++ b/mmtk/src/lib.rs @@ -3,8 +3,12 @@ extern crate mmtk; #[macro_use] extern crate lazy_static; extern crate once_cell; +extern crate spin; +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}; use mmtk::util::alloc::AllocationError; @@ -44,9 +48,6 @@ pub struct OpenJDK_Upcalls { pub out_of_memory: extern "C" fn(tls: VMThread, err_kind: AllocationError), pub get_next_mutator: extern "C" fn() -> *mut Mutator, pub reset_mutator_iterator: extern "C" fn(), - pub compute_static_roots: extern "C" fn(trace: *mut c_void, tls: OpaquePointer), - pub compute_global_roots: extern "C" fn(trace: *mut c_void, tls: OpaquePointer), - pub compute_thread_roots: extern "C" fn(trace: *mut c_void, tls: OpaquePointer), pub scan_object: extern "C" fn(trace: *mut c_void, object: ObjectReference, tls: OpaquePointer), pub dump_object: extern "C" fn(object: ObjectReference), pub get_object_size: extern "C" fn(object: ObjectReference) -> usize, @@ -60,8 +61,8 @@ pub struct OpenJDK_Upcalls { pub referent_offset: extern "C" fn() -> i32, pub discovered_offset: extern "C" fn() -> i32, pub dump_object_string: extern "C" fn(object: ObjectReference) -> *const c_char, - pub scan_thread_roots: extern "C" fn(process_edges: ProcessEdgesFn), - pub scan_thread_root: extern "C" fn(process_edges: ProcessEdgesFn, tls: VMMutatorThread), + pub scan_all_thread_roots: extern "C" fn(process_edges: ProcessEdgesFn), + pub scan_thread_roots: extern "C" fn(process_edges: ProcessEdgesFn, tls: VMMutatorThread), pub scan_universe_roots: extern "C" fn(process_edges: ProcessEdgesFn), pub scan_jni_handle_roots: extern "C" fn(process_edges: ProcessEdgesFn), pub scan_object_synchronizer_roots: extern "C" fn(process_edges: ProcessEdgesFn), @@ -128,3 +129,7 @@ lazy_static! { #[no_mangle] pub static MMTK_MARK_COMPACT_HEADER_RESERVED_IN_BYTES: usize = mmtk::util::alloc::MarkCompactAllocator::::HEADER_RESERVED_IN_BYTES; + +static CODE_CACHE_ROOTS: spin::Lazy>>> = + spin::Lazy::new(|| Mutex::new(HashMap::new())); +static CODE_CACHE_ROOTS_SIZE: AtomicUsize = AtomicUsize::new(0); diff --git a/mmtk/src/scanning.rs b/mmtk/src/scanning.rs index ec461ca4..93f298a2 100644 --- a/mmtk/src/scanning.rs +++ b/mmtk/src/scanning.rs @@ -10,6 +10,13 @@ use mmtk::vm::{EdgeVisitor, Scanning}; use mmtk::Mutator; use mmtk::MutatorContext; +pub(crate) fn create_process_edges_work_vec>(buf: Vec
) { + if !buf.is_empty() { + let w = W::new(buf, true, &SINGLETON); + memory_manager::add_work_packet(&SINGLETON, WorkBucketStage::Closure, w); + } +} + pub struct VMScanning {} pub(crate) extern "C" fn create_process_edges_work>( @@ -55,7 +62,7 @@ impl Scanning for VMScanning { fn scan_thread_roots>() { let process_edges = create_process_edges_work::; unsafe { - ((*UPCALLS).scan_thread_roots)(process_edges as _); + ((*UPCALLS).scan_all_thread_roots)(process_edges as _); } } @@ -66,7 +73,7 @@ impl Scanning for VMScanning { let tls = mutator.get_tls(); let process_edges = create_process_edges_work::; unsafe { - ((*UPCALLS).scan_thread_root)(process_edges as _, tls); + ((*UPCALLS).scan_thread_roots)(process_edges as _, tls); } } diff --git a/openjdk/mmtk.h b/openjdk/mmtk.h index b475871f..e51966b2 100644 --- a/openjdk/mmtk.h +++ b/openjdk/mmtk.h @@ -87,6 +87,10 @@ extern void handle_user_collection_request(void *tls); extern void start_control_collector(void *tls, void *context); extern void start_worker(void *tls, void* worker); +extern size_t mmtk_add_nmethod_oop(void* object); +extern size_t mmtk_register_nmethod(void* nm); +extern size_t mmtk_unregister_nmethod(void* nm); + /** * VM Accounting */ @@ -111,9 +115,6 @@ typedef struct { void (*out_of_memory) (void *tls, MMTkAllocationError err_kind); void* (*get_next_mutator) (); void (*reset_mutator_iterator) (); - void (*compute_static_roots) (void* trace, void* tls); - void (*compute_global_roots) (void* trace, void* tls); - void (*compute_thread_roots) (void* trace, void* tls); void (*scan_object) (void* trace, void* object, void* tls); void (*dump_object) (void* object); size_t (*get_object_size) (void* object); @@ -127,8 +128,8 @@ typedef struct { int (*referent_offset) (); int (*discovered_offset) (); char* (*dump_object_string) (void* object); - void (*scan_thread_roots)(ProcessEdgesFn process_edges); - void (*scan_thread_root)(ProcessEdgesFn process_edges, void* tls); + void (*scan_all_thread_roots)(ProcessEdgesFn process_edges); + void (*scan_thread_roots)(ProcessEdgesFn process_edges, void* tls); void (*scan_universe_roots) (ProcessEdgesFn process_edges); void (*scan_jni_handle_roots) (ProcessEdgesFn process_edges); void (*scan_object_synchronizer_roots) (ProcessEdgesFn process_edges); diff --git a/openjdk/mmtkHeap.cpp b/openjdk/mmtkHeap.cpp index 5da85d9e..1179cee4 100644 --- a/openjdk/mmtkHeap.cpp +++ b/openjdk/mmtkHeap.cpp @@ -335,135 +335,74 @@ void MMTkHeap::print_tracing_info() const { } -// An object is scavengable if its location may move during a scavenge. -// (A scavenge is a GC which is not a full GC.) -bool MMTkHeap::is_scavengable(oop obj) {return false;} // Registering and unregistering an nmethod (compiled code) with the heap. // Override with specific mechanism for each specialized heap type. - -// Heap verification -void MMTkHeap::verify(VerifyOption option) {} - -template -struct MMTkRootScanWorkScope { - int* _num_root_scan_tasks; - int _current_task_ordinal; - MMTkRootScanWorkScope(int* num_root_scan_tasks): _num_root_scan_tasks(num_root_scan_tasks), _current_task_ordinal(0) { - _current_task_ordinal = Atomic::add(1, _num_root_scan_tasks); - if (_current_task_ordinal == 1) { - nmethod::oops_do_marking_prologue(); - } - } - ~MMTkRootScanWorkScope() { - if (_current_task_ordinal == MAX_TASKS) { - _current_task_ordinal = 0; - nmethod::oops_do_marking_epilogue(); - } +class MMTkRegisterNMethodOopClosure: public OopClosure { + template void do_oop_work(T* p) { + mmtk_add_nmethod_oop((void*) p); } +public: + void do_oop(oop* p) { do_oop_work(p); } + void do_oop(narrowOop* p) { do_oop_work(p); } }; -void MMTkHeap::scan_static_roots(OopClosure& cl) { + +void MMTkHeap::register_nmethod(nmethod* nm) { + MMTkRegisterNMethodOopClosure reg_cl; + nm->oops_do(®_cl); + mmtk_register_nmethod((void*) nm); } +void MMTkHeap::unregister_nmethod(nmethod* nm) { + mmtk_unregister_nmethod((void*) nm); +} + +// Heap verification +void MMTkHeap::verify(VerifyOption option) {} void MMTkHeap::scan_universe_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); Universe::oops_do(&cl); } void MMTkHeap::scan_jni_handle_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); JNIHandles::oops_do(&cl); } void MMTkHeap::scan_object_synchronizer_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); ObjectSynchronizer::oops_do(&cl); } void MMTkHeap::scan_management_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); Management::oops_do(&cl); } void MMTkHeap::scan_jvmti_export_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); JvmtiExport::oops_do(&cl); } void MMTkHeap::scan_aot_loader_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); AOTLoader::oops_do(&cl); } void MMTkHeap::scan_system_dictionary_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); SystemDictionary::oops_do(&cl); } void MMTkHeap::scan_code_cache_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); - CodeBlobToOopClosure cb_cl(&cl, true); - { - MutexLockerEx lock(CodeCache_lock, Mutex::_no_safepoint_check_flag); - CodeCache::scavenge_root_nmethods_do(&cb_cl); - CodeCache::blobs_do(&cb_cl); - } + MarkingCodeBlobClosure cb_cl(&cl, false); + CodeCache::scavenge_root_nmethods_do(&cb_cl); } void MMTkHeap::scan_string_table_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); StringTable::oops_do(&cl); } void MMTkHeap::scan_class_loader_data_graph_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); CLDToOopClosure cld_cl(&cl, false); ClassLoaderDataGraph::cld_do(&cld_cl); } void MMTkHeap::scan_weak_processor_roots(OopClosure& cl) { ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); WeakProcessor::oops_do(&cl); // (really needed???) } void MMTkHeap::scan_vm_thread_roots(OopClosure& cl) { ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); VMThread::vm_thread()->oops_do(&cl, NULL); } -void MMTkHeap::scan_global_roots(OopClosure& cl) { - ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); - - CodeBlobToOopClosure cb_cl(&cl, true); - CLDToOopClosure cld_cl(&cl, false); - - Universe::oops_do(&cl); - JNIHandles::oops_do(&cl); - ObjectSynchronizer::oops_do(&cl); - Management::oops_do(&cl); - JvmtiExport::oops_do(&cl); - AOTLoader::oops_do(&cl); - SystemDictionary::oops_do(&cl); - { - MutexLockerEx lock(CodeCache_lock, Mutex::_no_safepoint_check_flag); - CodeCache::blobs_do(&cb_cl); - } - - OopStorage::ParState _par_state_string(StringTable::weak_storage()); - StringTable::possibly_parallel_oops_do(&_par_state_string, &cl); - - // if (!_root_tasks->is_task_claimed(MMTk_ClassLoaderDataGraph_oops_do)) ClassLoaderDataGraph::roots_cld_do(&cld_cl, &cld_cl); - ClassLoaderDataGraph::cld_do(&cld_cl); - - WeakProcessor::oops_do(&cl); // (really needed???) -} - void MMTkHeap::scan_thread_roots(OopClosure& cl) { ResourceMark rm; - MMTkRootScanWorkScope<> root_scan_work(&_num_root_scan_tasks); Threads::possibly_parallel_oops_do(false, &cl, NULL); } diff --git a/openjdk/mmtkHeap.hpp b/openjdk/mmtkHeap.hpp index 677638fc..c5fe037f 100644 --- a/openjdk/mmtkHeap.hpp +++ b/openjdk/mmtkHeap.hpp @@ -181,9 +181,11 @@ class MMTkHeap : public CollectedHeap { // An object is scavengable if its location may move during a scavenge. // (A scavenge is a GC which is not a full GC.) - bool is_scavengable(oop obj); + inline bool is_scavengable(oop obj) { return true; } // Registering and unregistering an nmethod (compiled code) with the heap. // Override with specific mechanism for each specialized heap type. + virtual void register_nmethod(nmethod* nm); + virtual void unregister_nmethod(nmethod* nm); // Heap verification void verify(VerifyOption option); @@ -192,8 +194,6 @@ class MMTkHeap : public CollectedHeap { void scan_roots(OopClosure& cl); - void scan_static_roots(OopClosure& cl); - void scan_global_roots(OopClosure& cl); void scan_thread_roots(OopClosure& cl); void scan_universe_roots(OopClosure& cl); diff --git a/openjdk/mmtkUpcalls.cpp b/openjdk/mmtkUpcalls.cpp index 50355a7c..d5c145a1 100644 --- a/openjdk/mmtkUpcalls.cpp +++ b/openjdk/mmtkUpcalls.cpp @@ -65,10 +65,12 @@ static void mmtk_stop_all_mutators(void *tls, void (*create_stack_scan_work)(voi } } log_debug(gc)("Finished enumerating threads."); + nmethod::oops_do_marking_prologue(); } static void mmtk_resume_mutators(void *tls) { - ClassLoaderDataGraph::purge(); + nmethod::oops_do_marking_epilogue(); + // ClassLoaderDataGraph::purge(); CodeCache::gc_epilogue(); JvmtiExport::gc_epilogue(); #if COMPILER2_OR_JVMCI @@ -211,31 +213,17 @@ static void mmtk_reset_mutator_iterator() { } -static void mmtk_compute_global_roots(void* trace, void* tls) { - MMTkRootsClosure cl(trace); - MMTkHeap::heap()->scan_global_roots(cl); -} - -static void mmtk_compute_static_roots(void* trace, void* tls) { - MMTkRootsClosure cl(trace); - MMTkHeap::heap()->scan_static_roots(cl); -} - -static void mmtk_compute_thread_roots(void* trace, void* tls) { - MMTkRootsClosure cl(trace); - MMTkHeap::heap()->scan_thread_roots(cl); -} - -static void mmtk_scan_thread_roots(ProcessEdgesFn process_edges) { +static void mmtk_scan_all_thread_roots(ProcessEdgesFn process_edges) { MMTkRootsClosure2 cl(process_edges); MMTkHeap::heap()->scan_thread_roots(cl); } -static void mmtk_scan_thread_root(ProcessEdgesFn process_edges, void* tls) { +static void mmtk_scan_thread_roots(ProcessEdgesFn process_edges, void* tls) { ResourceMark rm; JavaThread* thread = (JavaThread*) tls; MMTkRootsClosure2 cl(process_edges); - thread->oops_do(&cl, NULL); + MarkingCodeBlobClosure cb_cl(&cl, false); + thread->oops_do(&cl, &cb_cl); } static void mmtk_scan_object(void* trace, void* object, void* tls) { @@ -266,7 +254,6 @@ static void mmtk_harness_begin() { JavaThread* current = ((JavaThread*) Thread::current()); ThreadInVMfromNative tiv(current); mmtk_harness_begin_impl(); - } static void mmtk_harness_end() { @@ -365,9 +352,6 @@ OpenJDK_Upcalls mmtk_upcalls = { mmtk_out_of_memory, mmtk_get_next_mutator, mmtk_reset_mutator_iterator, - mmtk_compute_static_roots, - mmtk_compute_global_roots, - mmtk_compute_thread_roots, mmtk_scan_object, mmtk_dump_object, mmtk_get_object_size, @@ -381,8 +365,8 @@ OpenJDK_Upcalls mmtk_upcalls = { referent_offset, discovered_offset, dump_object_string, + mmtk_scan_all_thread_roots, mmtk_scan_thread_roots, - mmtk_scan_thread_root, mmtk_scan_universe_roots, mmtk_scan_jni_handle_roots, mmtk_scan_object_synchronizer_roots, From 084a127f658507d69fb39e23ed592533ec07ea72 Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Tue, 28 Jun 2022 10:36:44 +1000 Subject: [PATCH 2/8] minor --- mmtk/src/gc_work.rs | 6 +++++- openjdk/mmtkHeap.cpp | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/mmtk/src/gc_work.rs b/mmtk/src/gc_work.rs index ff7018ef..9d856519 100644 --- a/mmtk/src/gc_work.rs +++ b/mmtk/src/gc_work.rs @@ -133,7 +133,11 @@ impl> GCWork for ScanCodeCacheRoots(vec) + create_process_edges_work_vec::(vec); + // Use the following code to scan CodeCache directly, instead of scanning the "remembered set". + // unsafe { + // ((*UPCALLS).scan_code_cache_roots)(create_process_edges_work:: as _); + // } } } diff --git a/openjdk/mmtkHeap.cpp b/openjdk/mmtkHeap.cpp index 1179cee4..2a86bf0e 100644 --- a/openjdk/mmtkHeap.cpp +++ b/openjdk/mmtkHeap.cpp @@ -383,7 +383,7 @@ void MMTkHeap::scan_system_dictionary_roots(OopClosure& cl) { } void MMTkHeap::scan_code_cache_roots(OopClosure& cl) { MarkingCodeBlobClosure cb_cl(&cl, false); - CodeCache::scavenge_root_nmethods_do(&cb_cl); + CodeCache::blobs_do(&cb_cl); } void MMTkHeap::scan_string_table_roots(OopClosure& cl) { StringTable::oops_do(&cl); From f6e121d7569c39339881433c128ca1e7a7d6179f Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Thu, 30 Jun 2022 11:25:39 +1000 Subject: [PATCH 3/8] Fix clippy --- mmtk/src/api.rs | 6 +++--- mmtk/src/gc_work.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index edbe5537..0229ea5a 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -290,12 +290,12 @@ thread_local! { } #[no_mangle] -pub unsafe extern "C" fn mmtk_add_nmethod_oop(addr: Address) { +pub extern "C" fn mmtk_add_nmethod_oop(addr: Address) { NMETHOD_SLOTS.with(|x| x.borrow_mut().push(addr)) } #[no_mangle] -pub unsafe extern "C" fn mmtk_register_nmethod(nm: Address) { +pub extern "C" fn mmtk_register_nmethod(nm: Address) { let slots = NMETHOD_SLOTS.with(|x| { if x.borrow().len() == 0 { return None; @@ -311,7 +311,7 @@ pub unsafe extern "C" fn mmtk_register_nmethod(nm: Address) { } #[no_mangle] -pub unsafe extern "C" fn mmtk_unregister_nmethod(nm: Address) { +pub extern "C" fn mmtk_unregister_nmethod(nm: Address) { if let Some(slots) = crate::CODE_CACHE_ROOTS.lock().unwrap().remove(&nm) { crate::CODE_CACHE_ROOTS_SIZE.fetch_sub(slots.len(), Ordering::SeqCst); } diff --git a/mmtk/src/gc_work.rs b/mmtk/src/gc_work.rs index 9d856519..fb935a3a 100644 --- a/mmtk/src/gc_work.rs +++ b/mmtk/src/gc_work.rs @@ -128,7 +128,7 @@ impl> ScanCodeCacheRoots { impl> GCWork for ScanCodeCacheRoots { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { let mut vec = Vec::with_capacity(crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed)); - for (_, roots) in &*crate::CODE_CACHE_ROOTS.lock().unwrap() { + for roots in (*crate::CODE_CACHE_ROOTS.lock().unwrap()).values() { for r in roots { vec.push(*r) } From 55f1317255fcce90c96a19db5cf198a68dc77e3d Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Fri, 1 Jul 2022 13:37:03 +1000 Subject: [PATCH 4/8] Fix duplicate edges --- openjdk/mmtkUpcalls.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openjdk/mmtkUpcalls.cpp b/openjdk/mmtkUpcalls.cpp index d5c145a1..208c45c4 100644 --- a/openjdk/mmtkUpcalls.cpp +++ b/openjdk/mmtkUpcalls.cpp @@ -222,8 +222,7 @@ static void mmtk_scan_thread_roots(ProcessEdgesFn process_edges, void* tls) { ResourceMark rm; JavaThread* thread = (JavaThread*) tls; MMTkRootsClosure2 cl(process_edges); - MarkingCodeBlobClosure cb_cl(&cl, false); - thread->oops_do(&cl, &cb_cl); + thread->oops_do(&cl, NULL); } static void mmtk_scan_object(void* trace, void* object, void* tls) { From d359b0b6f54905f0ae910c6308095d517a4f75f8 Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Wed, 6 Jul 2022 15:00:18 +1000 Subject: [PATCH 5/8] minor --- mmtk/src/api.rs | 24 ++++++++++++++++-------- mmtk/src/gc_work.rs | 2 ++ mmtk/src/lib.rs | 8 ++++++-- openjdk/mmtkHeap.cpp | 2 ++ 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index 0229ea5a..ebfc4be9 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -286,14 +286,19 @@ pub extern "C" fn get_finalized_object() -> ObjectReference { } thread_local! { + /// Cache all the pointers reported by the current thread. static NMETHOD_SLOTS: RefCell> = RefCell::new(vec![]); } +/// 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)) } +/// 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. #[no_mangle] pub extern "C" fn mmtk_register_nmethod(nm: Address) { let slots = NMETHOD_SLOTS.with(|x| { @@ -302,17 +307,20 @@ pub extern "C" fn mmtk_register_nmethod(nm: Address) { } Some(x.replace(vec![])) }); - if slots.is_none() { - return; - } - let slots = slots.unwrap(); - crate::CODE_CACHE_ROOTS_SIZE.fetch_add(slots.len(), Ordering::SeqCst); - crate::CODE_CACHE_ROOTS.lock().unwrap().insert(nm, slots); + let slots = match slots { + Some(slots) => slots, + _ => return, + }; + let mut roots = crate::CODE_CACHE_ROOTS.lock().unwrap(); + 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) { - if let Some(slots) = crate::CODE_CACHE_ROOTS.lock().unwrap().remove(&nm) { - crate::CODE_CACHE_ROOTS_SIZE.fetch_sub(slots.len(), Ordering::SeqCst); + let mut roots = crate::CODE_CACHE_ROOTS.lock().unwrap(); + if let Some(slots) = roots.remove(&nm) { + crate::CODE_CACHE_ROOTS_SIZE.store(crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed) - slots.len(), Ordering::Relaxed); } } diff --git a/mmtk/src/gc_work.rs b/mmtk/src/gc_work.rs index dda6e644..098092e5 100644 --- a/mmtk/src/gc_work.rs +++ b/mmtk/src/gc_work.rs @@ -55,12 +55,14 @@ impl ScanCodeCacheRoots { impl GCWork for ScanCodeCacheRoots { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { + // Collect all the cached roots let mut edges = 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 { edges.push(*r) } } + // Create work packet self.factory.create_process_edge_roots_work(edges); // Use the following code to scan CodeCache directly, instead of scanning the "remembered set". // unsafe { diff --git a/mmtk/src/lib.rs b/mmtk/src/lib.rs index 13336648..60860dd9 100644 --- a/mmtk/src/lib.rs +++ b/mmtk/src/lib.rs @@ -144,6 +144,10 @@ lazy_static! { pub static MMTK_MARK_COMPACT_HEADER_RESERVED_IN_BYTES: usize = mmtk::util::alloc::MarkCompactAllocator::::HEADER_RESERVED_IN_BYTES; -static CODE_CACHE_ROOTS: spin::Lazy>>> = - spin::Lazy::new(|| Mutex::new(HashMap::new())); +lazy_static! { + /// A global storage for all the cached CodeCache root pointers + static ref 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); diff --git a/openjdk/mmtkHeap.cpp b/openjdk/mmtkHeap.cpp index 165cf7d7..035270f6 100644 --- a/openjdk/mmtkHeap.cpp +++ b/openjdk/mmtkHeap.cpp @@ -348,8 +348,10 @@ class MMTkRegisterNMethodOopClosure: public OopClosure { void MMTkHeap::register_nmethod(nmethod* nm) { + // Scan and report pointers in this nmethod MMTkRegisterNMethodOopClosure reg_cl; nm->oops_do(®_cl); + // Register the nmethod mmtk_register_nmethod((void*) nm); } From 7ce923f1747b8c5a3982606f194e0da44284fe0f Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Wed, 6 Jul 2022 18:08:06 +1000 Subject: [PATCH 6/8] cargo fmt --- mmtk/src/api.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index ebfc4be9..1e645933 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -312,7 +312,10 @@ pub extern "C" fn mmtk_register_nmethod(nm: Address) { _ => return, }; let mut roots = crate::CODE_CACHE_ROOTS.lock().unwrap(); - crate::CODE_CACHE_ROOTS_SIZE.store(crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed) + slots.len(), Ordering::Relaxed); + crate::CODE_CACHE_ROOTS_SIZE.store( + crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed) + slots.len(), + Ordering::Relaxed, + ); roots.insert(nm, slots); } @@ -321,6 +324,9 @@ pub extern "C" fn mmtk_register_nmethod(nm: Address) { 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) { - crate::CODE_CACHE_ROOTS_SIZE.store(crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed) - slots.len(), Ordering::Relaxed); + crate::CODE_CACHE_ROOTS_SIZE.store( + crate::CODE_CACHE_ROOTS_SIZE.load(Ordering::Relaxed) - slots.len(), + Ordering::Relaxed, + ); } } From 334e0acc8f5e7ab8f7d32b30b214161e9cf703c3 Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Thu, 7 Jul 2022 10:42:05 +1000 Subject: [PATCH 7/8] Add comments --- mmtk/src/api.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mmtk/src/api.rs b/mmtk/src/api.rs index 1e645933..3529771b 100644 --- a/mmtk/src/api.rs +++ b/mmtk/src/api.rs @@ -312,6 +312,7 @@ pub extern "C" fn mmtk_register_nmethod(nm: Address) { _ => 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, @@ -324,6 +325,7 @@ pub extern "C" fn mmtk_register_nmethod(nm: Address) { 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, From 8aadcbc0fbee4b2eff100f6540e6682fc03b5f3d Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Thu, 7 Jul 2022 13:46:15 +1000 Subject: [PATCH 8/8] Remove spin --- mmtk/Cargo.toml | 1 - mmtk/src/lib.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/mmtk/Cargo.toml b/mmtk/Cargo.toml index c044bb79..b1c4d296 100644 --- a/mmtk/Cargo.toml +++ b/mmtk/Cargo.toml @@ -22,7 +22,6 @@ openjdk_version = "ca90b43f0f51d9ddf754e6ab134c5030cf54118b" libc = "0.2" lazy_static = "1.1" once_cell = "1.10.0" -spin = "0.9.2" # 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. # These changes are safe: diff --git a/mmtk/src/lib.rs b/mmtk/src/lib.rs index 60860dd9..ed946885 100644 --- a/mmtk/src/lib.rs +++ b/mmtk/src/lib.rs @@ -3,7 +3,6 @@ extern crate mmtk; #[macro_use] extern crate lazy_static; extern crate once_cell; -extern crate spin; use std::collections::HashMap; use std::ptr::null_mut;