From 45ae7b9450e8db5fd8b609fa8a0b348b6a3182ba Mon Sep 17 00:00:00 2001 From: mgerdin Date: Tue, 14 Jan 2014 16:40:33 +0100 Subject: [PATCH] 8032379: Remove the is_scavenging flag to process_strong_roots Summary: Refactor the strong root processing to avoid using a boolean in addition to the ScanOption enum. Reviewed-by: stefank, tschatzl, ehelin, jmasa --- .../concurrentMarkSweepGeneration.cpp | 20 +++++-------- .../gc_implementation/g1/g1CollectedHeap.cpp | 10 +++---- .../vm/gc_implementation/g1/g1MarkSweep.cpp | 2 -- .../parNew/parNewGeneration.cpp | 3 +- src/share/vm/memory/defNewGeneration.cpp | 3 +- src/share/vm/memory/genCollectedHeap.cpp | 5 ++-- src/share/vm/memory/genCollectedHeap.hpp | 1 - src/share/vm/memory/genMarkSweep.cpp | 2 -- src/share/vm/memory/sharedHeap.cpp | 30 ++++++++++--------- src/share/vm/memory/sharedHeap.hpp | 7 +++-- 10 files changed, 35 insertions(+), 48 deletions(-) diff --git a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp index 0bb8a887b..bb80795e6 100644 --- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp +++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp @@ -3038,7 +3038,6 @@ void CMSCollector::verify_after_remark_work_1() { gch->gen_process_strong_roots(_cmsGen->level(), true, // younger gens are roots true, // activate StrongRootsScope - false, // not scavenging SharedHeap::ScanningOption(roots_scanning_options()), ¬Older, true, // walk code active on stacks @@ -3106,7 +3105,6 @@ void CMSCollector::verify_after_remark_work_2() { gch->gen_process_strong_roots(_cmsGen->level(), true, // younger gens are roots true, // activate StrongRootsScope - false, // not scavenging SharedHeap::ScanningOption(roots_scanning_options()), ¬Older, true, // walk code active on stacks @@ -3308,7 +3306,7 @@ bool ConcurrentMarkSweepGeneration::is_too_full() const { void CMSCollector::setup_cms_unloading_and_verification_state() { const bool should_verify = VerifyBeforeGC || VerifyAfterGC || VerifyDuringGC || VerifyBeforeExit; - const int rso = SharedHeap::SO_Strings | SharedHeap::SO_CodeCache; + const int rso = SharedHeap::SO_Strings | SharedHeap::SO_AllCodeCache; // We set the proper root for this CMS cycle here. if (should_unload_classes()) { // Should unload classes this cycle @@ -3744,10 +3742,9 @@ void CMSCollector::checkpointRootsInitialWork(bool asynch) { gch->gen_process_strong_roots(_cmsGen->level(), true, // younger gens are roots true, // activate StrongRootsScope - false, // not scavenging SharedHeap::ScanningOption(roots_scanning_options()), ¬Older, - true, // walk all of code cache if (so & SO_CodeCache) + true, // walk all of code cache if (so & SO_AllCodeCache) NULL, &klass_closure); } @@ -5244,14 +5241,13 @@ void CMSParInitialMarkTask::work(uint worker_id) { gch->gen_process_strong_roots(_collector->_cmsGen->level(), false, // yg was scanned above false, // this is parallel code - false, // not scavenging SharedHeap::ScanningOption(_collector->CMSCollector::roots_scanning_options()), &par_mri_cl, - true, // walk all of code cache if (so & SO_CodeCache) + true, // walk all of code cache if (so & SO_AllCodeCache) NULL, &klass_closure); assert(_collector->should_unload_classes() - || (_collector->CMSCollector::roots_scanning_options() & SharedHeap::SO_CodeCache), + || (_collector->CMSCollector::roots_scanning_options() & SharedHeap::SO_AllCodeCache), "if we didn't scan the code cache, we have to be ready to drop nmethods with expired weak oops"); _timer.stop(); if (PrintCMSStatistics != 0) { @@ -5381,14 +5377,13 @@ void CMSParRemarkTask::work(uint worker_id) { gch->gen_process_strong_roots(_collector->_cmsGen->level(), false, // yg was scanned above false, // this is parallel code - false, // not scavenging SharedHeap::ScanningOption(_collector->CMSCollector::roots_scanning_options()), &par_mrias_cl, - true, // walk all of code cache if (so & SO_CodeCache) + true, // walk all of code cache if (so & SO_AllCodeCache) NULL, NULL); // The dirty klasses will be handled below assert(_collector->should_unload_classes() - || (_collector->CMSCollector::roots_scanning_options() & SharedHeap::SO_CodeCache), + || (_collector->CMSCollector::roots_scanning_options() & SharedHeap::SO_AllCodeCache), "if we didn't scan the code cache, we have to be ready to drop nmethods with expired weak oops"); _timer.stop(); if (PrintCMSStatistics != 0) { @@ -5972,7 +5967,6 @@ void CMSCollector::do_remark_non_parallel() { gch->gen_process_strong_roots(_cmsGen->level(), true, // younger gens as roots false, // use the local StrongRootsScope - false, // not scavenging SharedHeap::ScanningOption(roots_scanning_options()), &mrias_cl, true, // walk code active on stacks @@ -5980,7 +5974,7 @@ void CMSCollector::do_remark_non_parallel() { NULL); // The dirty klasses will be handled below assert(should_unload_classes() - || (roots_scanning_options() & SharedHeap::SO_CodeCache), + || (roots_scanning_options() & SharedHeap::SO_AllCodeCache), "if we didn't scan the code cache, we have to be ready to drop nmethods with expired weak oops"); } diff --git a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp index 09bb53cb2..cae241406 100644 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -3396,14 +3396,12 @@ void G1CollectedHeap::verify(bool silent, VerifyOption vo) { // We apply the relevant closures to all the oops in the // system dictionary, the string table and the code cache. - const int so = SO_AllClasses | SO_Strings | SO_CodeCache; + const int so = SO_AllClasses | SO_Strings | SO_AllCodeCache; // Need cleared claim bits for the strong roots processing ClassLoaderDataGraph::clear_claimed_marks(); process_strong_roots(true, // activate StrongRootsScope - false, // we set "is scavenging" to false, - // so we don't reset the dirty cards. ScanningOption(so), // roots scanning options &rootsCl, &blobsCl, @@ -4867,13 +4865,13 @@ g1_process_strong_roots(bool is_scavenging, BufferingOopClosure buf_scan_non_heap_roots(scan_non_heap_roots); - assert(so & SO_CodeCache || scan_rs != NULL, "must scan code roots somehow"); + assert(so & SO_AllCodeCache || scan_rs != NULL, "must scan code roots somehow"); // Walk the code cache/strong code roots w/o buffering, because StarTask // cannot handle unaligned oop locations. CodeBlobToOopClosure eager_scan_code_roots(scan_non_heap_roots, true /* do_marking */); process_strong_roots(false, // no scoping; this is parallel code - is_scavenging, so, + so, &buf_scan_non_heap_roots, &eager_scan_code_roots, scan_klasses @@ -4921,7 +4919,7 @@ g1_process_strong_roots(bool is_scavenging, // the collection set. // Note all threads participate in this set of root tasks. double mark_strong_code_roots_ms = 0.0; - if (g1_policy()->during_initial_mark_pause() && !(so & SO_CodeCache)) { + if (g1_policy()->during_initial_mark_pause() && !(so & SO_AllCodeCache)) { double mark_strong_roots_start = os::elapsedTime(); mark_strong_code_roots(worker_i); mark_strong_code_roots_ms = (os::elapsedTime() - mark_strong_roots_start) * 1000.0; diff --git a/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp b/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp index a7db15c74..d5e8ee627 100644 --- a/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp +++ b/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp @@ -132,7 +132,6 @@ void G1MarkSweep::mark_sweep_phase1(bool& marked_for_unloading, ClassLoaderDataGraph::clear_claimed_marks(); sh->process_strong_roots(true, // activate StrongRootsScope - false, // not scavenging. SharedHeap::SO_SystemClasses, &GenMarkSweep::follow_root_closure, &GenMarkSweep::follow_code_root_closure, @@ -309,7 +308,6 @@ void G1MarkSweep::mark_sweep_phase3() { ClassLoaderDataGraph::clear_claimed_marks(); sh->process_strong_roots(true, // activate StrongRootsScope - false, // not scavenging. SharedHeap::SO_AllClasses, &GenMarkSweep::adjust_pointer_closure, NULL, // do not touch code cache here diff --git a/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp b/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp index 7ee404572..e8313168b 100644 --- a/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp +++ b/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp @@ -614,14 +614,13 @@ void ParNewGenTask::work(uint worker_id) { KlassScanClosure klass_scan_closure(&par_scan_state.to_space_root_closure(), gch->rem_set()->klass_rem_set()); - int so = SharedHeap::SO_AllClasses | SharedHeap::SO_Strings | SharedHeap::SO_CodeCache; + int so = SharedHeap::SO_AllClasses | SharedHeap::SO_Strings | SharedHeap::SO_ScavengeCodeCache; par_scan_state.start_strong_roots(); gch->gen_process_strong_roots(_gen->level(), true, // Process younger gens, if any, // as strong roots. false, // no scope; this is parallel code - true, // is scavenging SharedHeap::ScanningOption(so), &par_scan_state.to_space_root_closure(), true, // walk *all* scavengable nmethods diff --git a/src/share/vm/memory/defNewGeneration.cpp b/src/share/vm/memory/defNewGeneration.cpp index 62d75be6e..9c9267a4c 100644 --- a/src/share/vm/memory/defNewGeneration.cpp +++ b/src/share/vm/memory/defNewGeneration.cpp @@ -622,13 +622,12 @@ void DefNewGeneration::collect(bool full, assert(gch->no_allocs_since_save_marks(0), "save marks have not been newly set."); - int so = SharedHeap::SO_AllClasses | SharedHeap::SO_Strings | SharedHeap::SO_CodeCache; + int so = SharedHeap::SO_AllClasses | SharedHeap::SO_Strings | SharedHeap::SO_ScavengeCodeCache; gch->gen_process_strong_roots(_level, true, // Process younger gens, if any, // as strong roots. true, // activate StrongRootsScope - true, // is scavenging SharedHeap::ScanningOption(so), &fsc_with_no_gc_barrier, true, // walk *all* scavengable nmethods diff --git a/src/share/vm/memory/genCollectedHeap.cpp b/src/share/vm/memory/genCollectedHeap.cpp index d222e2b53..96d22ed53 100644 --- a/src/share/vm/memory/genCollectedHeap.cpp +++ b/src/share/vm/memory/genCollectedHeap.cpp @@ -597,7 +597,6 @@ void GenCollectedHeap:: gen_process_strong_roots(int level, bool younger_gens_as_roots, bool activate_scope, - bool is_scavenging, SharedHeap::ScanningOption so, OopsInGenClosure* not_older_gens, bool do_code_roots, @@ -606,12 +605,12 @@ gen_process_strong_roots(int level, // General strong roots. if (!do_code_roots) { - SharedHeap::process_strong_roots(activate_scope, is_scavenging, so, + SharedHeap::process_strong_roots(activate_scope, so, not_older_gens, NULL, klass_closure); } else { bool do_code_marking = (activate_scope || nmethod::oops_do_marking_is_active()); CodeBlobToOopClosure code_roots(not_older_gens, /*do_marking=*/ do_code_marking); - SharedHeap::process_strong_roots(activate_scope, is_scavenging, so, + SharedHeap::process_strong_roots(activate_scope, so, not_older_gens, &code_roots, klass_closure); } diff --git a/src/share/vm/memory/genCollectedHeap.hpp b/src/share/vm/memory/genCollectedHeap.hpp index a44e2eb0d..75e25e06e 100644 --- a/src/share/vm/memory/genCollectedHeap.hpp +++ b/src/share/vm/memory/genCollectedHeap.hpp @@ -420,7 +420,6 @@ public: // The remaining arguments are in an order // consistent with SharedHeap::process_strong_roots: bool activate_scope, - bool is_scavenging, SharedHeap::ScanningOption so, OopsInGenClosure* not_older_gens, bool do_code_roots, diff --git a/src/share/vm/memory/genMarkSweep.cpp b/src/share/vm/memory/genMarkSweep.cpp index 0eccd3419..3162d5a04 100644 --- a/src/share/vm/memory/genMarkSweep.cpp +++ b/src/share/vm/memory/genMarkSweep.cpp @@ -210,7 +210,6 @@ void GenMarkSweep::mark_sweep_phase1(int level, gch->gen_process_strong_roots(level, false, // Younger gens are not roots. true, // activate StrongRootsScope - false, // not scavenging SharedHeap::SO_SystemClasses, &follow_root_closure, true, // walk code active on stacks @@ -296,7 +295,6 @@ void GenMarkSweep::mark_sweep_phase3(int level) { gch->gen_process_strong_roots(level, false, // Younger gens are not roots. true, // activate StrongRootsScope - false, // not scavenging SharedHeap::SO_AllClasses, &adjust_pointer_closure, false, // do not walk code diff --git a/src/share/vm/memory/sharedHeap.cpp b/src/share/vm/memory/sharedHeap.cpp index 56683feeb..0006113c8 100644 --- a/src/share/vm/memory/sharedHeap.cpp +++ b/src/share/vm/memory/sharedHeap.cpp @@ -139,7 +139,6 @@ SharedHeap::StrongRootsScope::~StrongRootsScope() { } void SharedHeap::process_strong_roots(bool activate_scope, - bool is_scavenging, ScanningOption so, OopClosure* roots, CodeBlobClosure* code_roots, @@ -159,9 +158,11 @@ void SharedHeap::process_strong_roots(bool activate_scope, if (!_process_strong_tasks->is_task_claimed(SH_PS_JNIHandles_oops_do)) JNIHandles::oops_do(roots); - // All threads execute this; the individual threads are task groups. CLDToOopClosure roots_from_clds(roots); - CLDToOopClosure* roots_from_clds_p = (is_scavenging ? NULL : &roots_from_clds); + // If we limit class scanning to SO_SystemClasses we need to apply a CLD closure to + // CLDs which are strongly reachable from the thread stacks. + CLDToOopClosure* roots_from_clds_p = ((so & SO_SystemClasses) ? &roots_from_clds : NULL); + // All threads execute this; the individual threads are task groups. if (CollectedHeap::use_parallel_gc_threads()) { Threads::possibly_parallel_oops_do(roots, roots_from_clds_p, code_roots); } else { @@ -189,9 +190,9 @@ void SharedHeap::process_strong_roots(bool activate_scope, if (!_process_strong_tasks->is_task_claimed(SH_PS_ClassLoaderDataGraph_oops_do)) { if (so & SO_AllClasses) { - ClassLoaderDataGraph::oops_do(roots, klass_closure, !is_scavenging); + ClassLoaderDataGraph::oops_do(roots, klass_closure, /* must_claim */ false); } else if (so & SO_SystemClasses) { - ClassLoaderDataGraph::always_strong_oops_do(roots, klass_closure, !is_scavenging); + ClassLoaderDataGraph::always_strong_oops_do(roots, klass_closure, /* must_claim */ true); } } @@ -206,17 +207,18 @@ void SharedHeap::process_strong_roots(bool activate_scope, } if (!_process_strong_tasks->is_task_claimed(SH_PS_CodeCache_oops_do)) { - if (so & SO_CodeCache) { + if (so & SO_ScavengeCodeCache) { + assert(code_roots != NULL, "must supply closure for code cache"); + + // We only visit parts of the CodeCache when scavenging. + CodeCache::scavenge_root_nmethods_do(code_roots); + } + if (so & SO_AllCodeCache) { assert(code_roots != NULL, "must supply closure for code cache"); - if (is_scavenging) { - // We only visit parts of the CodeCache when scavenging. - CodeCache::scavenge_root_nmethods_do(code_roots); - } else { - // CMSCollector uses this to do intermediate-strength collections. - // We scan the entire code cache, since CodeCache::do_unloading is not called. - CodeCache::blobs_do(code_roots); - } + // CMSCollector uses this to do intermediate-strength collections. + // We scan the entire code cache, since CodeCache::do_unloading is not called. + CodeCache::blobs_do(code_roots); } // Verify that the code cache contents are not subject to // movement by a scavenging collection. diff --git a/src/share/vm/memory/sharedHeap.hpp b/src/share/vm/memory/sharedHeap.hpp index f5d9e05f0..5bf1dc44a 100644 --- a/src/share/vm/memory/sharedHeap.hpp +++ b/src/share/vm/memory/sharedHeap.hpp @@ -221,7 +221,8 @@ public: SO_AllClasses = 0x1, SO_SystemClasses = 0x2, SO_Strings = 0x4, - SO_CodeCache = 0x8 + SO_AllCodeCache = 0x8, + SO_ScavengeCodeCache = 0x10 }; FlexibleWorkGang* workers() const { return _workers; } @@ -232,9 +233,9 @@ public: // "SO_AllClasses" applies the closure to all entries in the SystemDictionary; // "SO_SystemClasses" to all the "system" classes and loaders; // "SO_Strings" applies the closure to all entries in StringTable; - // "SO_CodeCache" applies the closure to all elements of the CodeCache. + // "SO_AllCodeCache" applies the closure to all elements of the CodeCache. + // "SO_ScavengeCodeCache" applies the closure to elements on the scavenge root list in the CodeCache. void process_strong_roots(bool activate_scope, - bool is_scavenging, ScanningOption so, OopClosure* roots, CodeBlobClosure* code_roots, -- GitLab