--- old/src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp 2019-09-18 17:03:31.305419288 +0200 +++ new/src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp 2019-09-18 17:03:31.117418171 +0200 @@ -62,8 +62,7 @@ _gc_par_phases[JVMTIRoots] = new WorkerDataArray(max_gc_threads, "JVMTI Roots (ms):"); AOT_ONLY(_gc_par_phases[AOTCodeRoots] = new WorkerDataArray(max_gc_threads, "AOT Root Scan (ms):");) _gc_par_phases[CMRefRoots] = new WorkerDataArray(max_gc_threads, "CM RefProcessor Roots (ms):"); - _gc_par_phases[WaitForStrongCLD] = new WorkerDataArray(max_gc_threads, "Wait For Strong CLD (ms):"); - _gc_par_phases[WeakCLDRoots] = new WorkerDataArray(max_gc_threads, "Weak CLD Roots (ms):"); + _gc_par_phases[WaitForStrongRoots] = new WorkerDataArray(max_gc_threads, "Wait For Strong Roots (ms):"); _gc_par_phases[MergeER] = new WorkerDataArray(max_gc_threads, "Eager Reclaim (ms):"); @@ -567,8 +566,7 @@ "JVMTIRoots", AOT_ONLY("AOTCodeRoots" COMMA) "CMRefRoots", - "WaitForStrongCLD", - "WeakCLDRoots", + "WaitForStrongRoots", "MergeER", "MergeRS", "OptMergeRS", --- old/src/hotspot/share/gc/g1/g1GCPhaseTimes.hpp 2019-09-18 17:03:32.001423418 +0200 +++ new/src/hotspot/share/gc/g1/g1GCPhaseTimes.hpp 2019-09-18 17:03:31.809422278 +0200 @@ -57,8 +57,7 @@ JVMTIRoots, AOT_ONLY(AOTCodeRoots COMMA) CMRefRoots, - WaitForStrongCLD, - WeakCLDRoots, + WaitForStrongRoots, MergeER, MergeRS, OptMergeRS, @@ -84,7 +83,7 @@ }; static const GCParPhases ExtRootScanSubPhasesFirst = ThreadRoots; - static const GCParPhases ExtRootScanSubPhasesLast = WeakCLDRoots; + static const GCParPhases ExtRootScanSubPhasesLast = WaitForStrongRoots; enum GCMergeRSWorkTimes { MergeRSMergedSparse, --- old/src/hotspot/share/gc/g1/g1OopClosures.cpp 2019-09-18 17:03:32.677427429 +0200 +++ new/src/hotspot/share/gc/g1/g1OopClosures.cpp 2019-09-18 17:03:32.485426291 +0200 @@ -43,16 +43,14 @@ void G1CLDScanClosure::do_cld(ClassLoaderData* cld) { // If the class loader data has not been dirtied we know that there's - // no references into the young gen and we can skip it. + // no references into the young gen and we can skip it. if (!_process_only_dirty || cld->has_modified_oops()) { // Tell the closure that this class loader data is the CLD to scavenge // and is the one to dirty if oops are left pointing into the young gen. _closure->set_scanned_cld(cld); - - // Clean the cld since we're going to scavenge all the metadata. - // Clear modified oops only if this cld is claimed. - cld->oops_do(_closure, _claim, /*clear_modified_oops*/true); + // Clean modified oops since we're going to scavenge all the metadata. + cld->oops_do(_closure, ClassLoaderData::_claim_none, true /*clear_modified_oops*/); _closure->set_scanned_cld(NULL); --- old/src/hotspot/share/gc/g1/g1OopClosures.hpp 2019-09-18 17:03:33.325431275 +0200 +++ new/src/hotspot/share/gc/g1/g1OopClosures.hpp 2019-09-18 17:03:33.137430160 +0200 @@ -175,12 +175,10 @@ class G1CLDScanClosure : public CLDClosure { G1ParCopyHelper* _closure; bool _process_only_dirty; - int _claim; int _count; public: - G1CLDScanClosure(G1ParCopyHelper* closure, - bool process_only_dirty, int claim_value) - : _closure(closure), _process_only_dirty(process_only_dirty), _claim(claim_value), _count(0) {} + G1CLDScanClosure(G1ParCopyHelper* closure, bool process_only_dirty) + : _closure(closure), _process_only_dirty(process_only_dirty), _count(0) {} void do_cld(ClassLoaderData* cld); }; --- old/src/hotspot/share/gc/g1/g1RootClosures.cpp 2019-09-18 17:03:33.997435264 +0200 +++ new/src/hotspot/share/gc/g1/g1RootClosures.cpp 2019-09-18 17:03:33.801434101 +0200 @@ -35,14 +35,13 @@ G1EvacuationClosures(G1CollectedHeap* g1h, G1ParScanThreadState* pss, bool in_young_gc) : - _closures(g1h, pss, in_young_gc, /* cld_claim */ ClassLoaderData::_claim_none) {} + _closures(g1h, pss, in_young_gc) {} OopClosure* weak_oops() { return &_closures._oops; } OopClosure* strong_oops() { return &_closures._oops; } CLDClosure* weak_clds() { return &_closures._clds; } CLDClosure* strong_clds() { return &_closures._clds; } - CLDClosure* second_pass_weak_clds() { return NULL; } CodeBlobClosure* strong_codeblobs() { return &_closures._codeblobs; } CodeBlobClosure* weak_codeblobs() { return &_closures._codeblobs; } @@ -58,33 +57,18 @@ G1SharedClosures _strong; G1SharedClosures _weak; - // Filter method to help with returning the appropriate closures - // depending on the class template parameter. - template - T* null_if(T* t) { - if (Mark == MarkWeak) { - return NULL; - } - return t; - } - public: G1InitialMarkClosures(G1CollectedHeap* g1h, G1ParScanThreadState* pss) : - _strong(g1h, pss, /* process_only_dirty_klasses */ false, /* cld_claim */ ClassLoaderData::_claim_strong), - _weak(g1h, pss, /* process_only_dirty_klasses */ false, /* cld_claim */ ClassLoaderData::_claim_strong) {} + _strong(g1h, pss, /* process_only_dirty_klasses */ false), + _weak(g1h, pss, /* process_only_dirty_klasses */ false) {} OopClosure* weak_oops() { return &_weak._oops; } OopClosure* strong_oops() { return &_strong._oops; } - // If MarkWeak is G1MarkPromotedFromRoot then the weak CLDs must be processed in a second pass. - CLDClosure* weak_clds() { return null_if(&_weak._clds); } + CLDClosure* weak_clds() { return &_weak._clds; } CLDClosure* strong_clds() { return &_strong._clds; } - // If MarkWeak is G1MarkFromRoot then all CLDs are processed by the weak and strong variants - // return a NULL closure for the following specialized versions in that case. - CLDClosure* second_pass_weak_clds() { return null_if(&_weak._clds); } - CodeBlobClosure* strong_codeblobs() { return &_strong._codeblobs; } CodeBlobClosure* weak_codeblobs() { return &_weak._codeblobs; } --- old/src/hotspot/share/gc/g1/g1RootClosures.hpp 2019-09-18 17:03:34.677439299 +0200 +++ new/src/hotspot/share/gc/g1/g1RootClosures.hpp 2019-09-18 17:03:34.481438136 +0200 @@ -49,10 +49,6 @@ class G1EvacuationRootClosures : public G1RootClosures { public: - // Applied to the weakly reachable CLDs when all strongly reachable - // CLDs are guaranteed to have been processed. - virtual CLDClosure* second_pass_weak_clds() = 0; - // Applied to code blobs treated as weak roots. virtual CodeBlobClosure* weak_codeblobs() = 0; --- old/src/hotspot/share/gc/g1/g1RootProcessor.cpp 2019-09-18 17:03:35.337443216 +0200 +++ new/src/hotspot/share/gc/g1/g1RootProcessor.cpp 2019-09-18 17:03:35.145442075 +0200 @@ -71,7 +71,7 @@ _g1h(g1h), _process_strong_tasks(G1RP_PS_NumElements), _srs(n_workers), - _lock(Mutex::leaf, "G1 Root Scanning barrier lock", false, Monitor::_safepoint_check_never), + _lock(Mutex::leaf, "G1 Root Scan barrier lock", false, Monitor::_safepoint_check_never), _n_workers_discovered_strong_classes(0) {} void G1RootProcessor::evacuate_roots(G1ParScanThreadState* pss, uint worker_i) { @@ -80,13 +80,7 @@ G1EvacPhaseTimesTracker timer(phase_times, pss, G1GCPhaseTimes::ExtRootScan, worker_i); G1EvacuationRootClosures* closures = pss->closures(); - process_java_roots(closures, phase_times, worker_i); - - // This is the point where this worker thread will not find more strong CLDs/nmethods. - // Report this so G1 can synchronize the strong and weak CLDs/nmethods processing. - if (closures->trace_metadata()) { - worker_has_discovered_all_strong_classes(); - } + process_java_roots(closures, phase_times, worker_i, closures->trace_metadata() /* notify_claimed_roots_done */); process_vm_roots(closures, phase_times, worker_i); @@ -103,21 +97,9 @@ } if (closures->trace_metadata()) { - { - G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::WaitForStrongCLD, worker_i); - // Barrier to make sure all workers passed - // the strong CLD and strong nmethods phases. - wait_until_all_strong_classes_discovered(); - } - - // Now take the complement of the strong CLDs. - G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::WeakCLDRoots, worker_i); - assert(closures->second_pass_weak_clds() != NULL, "Should be non-null if we are tracing metadata."); - ClassLoaderDataGraph::roots_cld_do(NULL, closures->second_pass_weak_clds()); - } else { - phase_times->record_time_secs(G1GCPhaseTimes::WaitForStrongCLD, worker_i, 0.0); - phase_times->record_time_secs(G1GCPhaseTimes::WeakCLDRoots, worker_i, 0.0); - assert(closures->second_pass_weak_clds() == NULL, "Should be null if not tracing metadata."); + G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::WaitForStrongRoots, worker_i); + // Wait here to make sure all workers passed the strong nmethods phase. + wait_until_all_strong_classes_discovered(); } _process_strong_tasks.all_tasks_completed(n_workers()); @@ -189,17 +171,13 @@ void G1RootProcessor::process_java_roots(G1RootClosures* closures, G1GCPhaseTimes* phase_times, - uint worker_i) { - // Iterating over the CLDG and the Threads are done early to allow us to - // first process the strong CLDs and nmethods and then, after a barrier, - // let the thread process the weak CLDs and nmethods. - { - G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::CLDGRoots, worker_i); - if (_process_strong_tasks.try_claim_task(G1RP_PS_ClassLoaderDataGraph_oops_do)) { - ClassLoaderDataGraph::roots_cld_do(closures->strong_clds(), closures->weak_clds()); - } - } - + uint worker_i, + bool notify_claimed_roots_done) { + // Iterating over the the threads is done early to allow us to make sure that + // the "strong" nmethods are processed first using the strong closure. After a barrier, + // let the thread process the weak nmethods. + // The problem is that nmethods are claimed to avoid duplicate iteration. This is + // a way to make sure that for these nmethods we always apply the strong closure. { G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::ThreadRoots, worker_i); bool is_par = n_workers() > 1; @@ -207,6 +185,19 @@ closures->strong_oops(), closures->strong_codeblobs()); } + + // This is the point where this worker thread will not find more strong nmethods. + // Report this so G1 can synchronize the strong and weak CLDs/nmethods processing. + if (notify_claimed_roots_done) { + worker_has_discovered_all_strong_classes(); + } + + { + G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::CLDGRoots, worker_i); + if (_process_strong_tasks.try_claim_task(G1RP_PS_ClassLoaderDataGraph_oops_do)) { + ClassLoaderDataGraph::roots_cld_do(closures->strong_clds(), closures->weak_clds()); + } + } } void G1RootProcessor::process_vm_roots(G1RootClosures* closures, --- old/src/hotspot/share/gc/g1/g1RootProcessor.hpp 2019-09-18 17:03:36.049447440 +0200 +++ new/src/hotspot/share/gc/g1/g1RootProcessor.hpp 2019-09-18 17:03:35.845446229 +0200 @@ -74,7 +74,8 @@ void process_java_roots(G1RootClosures* closures, G1GCPhaseTimes* phase_times, - uint worker_i); + uint worker_i, + bool notify_claimed_roots_done = false); void process_vm_roots(G1RootClosures* closures, G1GCPhaseTimes* phase_times, --- old/src/hotspot/share/gc/g1/g1SharedClosures.hpp 2019-09-18 17:03:36.741451546 +0200 +++ new/src/hotspot/share/gc/g1/g1SharedClosures.hpp 2019-09-18 17:03:36.537450336 +0200 @@ -39,9 +39,9 @@ G1CLDScanClosure _clds; G1CodeBlobClosure _codeblobs; - G1SharedClosures(G1CollectedHeap* g1h, G1ParScanThreadState* pss, bool process_only_dirty, int cld_claim) : + G1SharedClosures(G1CollectedHeap* g1h, G1ParScanThreadState* pss, bool process_only_dirty) : _oops(g1h, pss), _oops_in_cld(g1h, pss), - _clds(&_oops_in_cld, process_only_dirty, cld_claim), + _clds(&_oops_in_cld, process_only_dirty), _codeblobs(&_oops) {} }; --- old/test/hotspot/jtreg/gc/g1/TestGCLogMessages.java 2019-09-18 17:03:37.413455533 +0200 +++ new/test/hotspot/jtreg/gc/g1/TestGCLogMessages.java 2019-09-18 17:03:37.217454370 +0200 @@ -128,8 +128,7 @@ new LogMessageWithLevel("CLDG Roots", Level.TRACE), new LogMessageWithLevel("JVMTI Roots", Level.TRACE), new LogMessageWithLevel("CM RefProcessor Roots", Level.TRACE), - new LogMessageWithLevel("Wait For Strong CLD", Level.TRACE), - new LogMessageWithLevel("Weak CLD Roots", Level.TRACE), + new LogMessageWithLevel("Wait For Strong Roots", Level.TRACE), // Redirty Cards new LogMessageWithLevel("Redirty Cards", Level.DEBUG), new LogMessageWithLevel("Parallel Redirty", Level.TRACE), --- old/test/jdk/jdk/jfr/event/gc/collection/TestG1ParallelPhases.java 2019-09-18 17:03:38.093459569 +0200 +++ new/test/jdk/jdk/jfr/event/gc/collection/TestG1ParallelPhases.java 2019-09-18 17:03:37.901458429 +0200 @@ -98,8 +98,7 @@ "CLDGRoots", "JVMTIRoots", "CMRefRoots", - "WaitForStrongCLD", - "WeakCLDRoots", + "WaitForStrongRoots", "MergeER", "MergeHCC", "MergeRS",