8159984: Remove call to ClassLoaderDataGraph::clear_claimed_marks during the initial mark pause
Summary: The CLDG is only iterated once during garbage collection, so we do not need to claim CLDs any more.
Reviewed-by: sjohanss, kbarrett
--- a/src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp Mon Sep 23 11:37:02 2019 +0200
@@ -62,8 +62,7 @@
_gc_par_phases[JVMTIRoots] = new WorkerDataArray<double>(max_gc_threads, "JVMTI Roots (ms):");
AOT_ONLY(_gc_par_phases[AOTCodeRoots] = new WorkerDataArray<double>(max_gc_threads, "AOT Root Scan (ms):");)
_gc_par_phases[CMRefRoots] = new WorkerDataArray<double>(max_gc_threads, "CM RefProcessor Roots (ms):");
- _gc_par_phases[WaitForStrongCLD] = new WorkerDataArray<double>(max_gc_threads, "Wait For Strong CLD (ms):");
- _gc_par_phases[WeakCLDRoots] = new WorkerDataArray<double>(max_gc_threads, "Weak CLD Roots (ms):");
+ _gc_par_phases[WaitForStrongRoots] = new WorkerDataArray<double>(max_gc_threads, "Wait For Strong Roots (ms):");
_gc_par_phases[MergeER] = new WorkerDataArray<double>(max_gc_threads, "Eager Reclaim (ms):");
@@ -567,8 +566,7 @@
"JVMTIRoots",
AOT_ONLY("AOTCodeRoots" COMMA)
"CMRefRoots",
- "WaitForStrongCLD",
- "WeakCLDRoots",
+ "WaitForStrongRoots",
"MergeER",
"MergeRS",
"OptMergeRS",
--- a/src/hotspot/share/gc/g1/g1GCPhaseTimes.hpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1GCPhaseTimes.hpp Mon Sep 23 11:37:02 2019 +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,
--- a/src/hotspot/share/gc/g1/g1OopClosures.cpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1OopClosures.cpp Mon Sep 23 11:37:02 2019 +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);
--- a/src/hotspot/share/gc/g1/g1OopClosures.hpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1OopClosures.hpp Mon Sep 23 11:37:02 2019 +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);
};
--- a/src/hotspot/share/gc/g1/g1RootClosures.cpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1RootClosures.cpp Mon Sep 23 11:37:02 2019 +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<G1MarkFromRoot> _strong;
G1SharedClosures<MarkWeak> _weak;
- // Filter method to help with returning the appropriate closures
- // depending on the class template parameter.
- template <G1Mark Mark, typename T>
- 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<G1MarkPromotedFromRoot>(&_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<G1MarkFromRoot>(&_weak._clds); }
-
CodeBlobClosure* strong_codeblobs() { return &_strong._codeblobs; }
CodeBlobClosure* weak_codeblobs() { return &_weak._codeblobs; }
--- a/src/hotspot/share/gc/g1/g1RootClosures.hpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1RootClosures.hpp Mon Sep 23 11:37:02 2019 +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;
--- a/src/hotspot/share/gc/g1/g1RootProcessor.cpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1RootProcessor.cpp Mon Sep 23 11:37:02 2019 +0200
@@ -45,7 +45,7 @@
#include "services/management.hpp"
#include "utilities/macros.hpp"
-void G1RootProcessor::worker_has_discovered_all_strong_classes() {
+void G1RootProcessor::worker_has_discovered_all_strong_nmethods() {
assert(ClassUnloadingWithConcurrentMark, "Currently only needed when doing G1 Class Unloading");
uint new_value = (uint)Atomic::add(1, &_n_workers_discovered_strong_classes);
@@ -56,7 +56,7 @@
}
}
-void G1RootProcessor::wait_until_all_strong_classes_discovered() {
+void G1RootProcessor::wait_until_all_strong_nmethods_discovered() {
assert(ClassUnloadingWithConcurrentMark, "Currently only needed when doing G1 Class Unloading");
if ((uint)_n_workers_discovered_strong_classes != n_workers()) {
@@ -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_nmethods_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 to make sure all workers passed the strong nmethods phase.
+ wait_until_all_strong_nmethods_discovered();
}
_process_strong_tasks.all_tasks_completed(n_workers());
@@ -189,17 +171,24 @@
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_nmethods_done) {
+ // We need to make make sure that the "strong" nmethods are processed first
+ // using the strong closure. Only after that we process the weakly reachable
+ // nmethods.
+ // We need to strictly separate the strong and weak nmethod processing because
+ // any processing claims that nmethod, i.e. will not be iterated again.
+ // Which means if an nmethod is processed first and claimed, the strong processing
+ // will not happen, and the oops reachable by that nmethod will not be marked
+ // properly.
+ //
+ // That is why we process strong nmethods first, synchronize all threads via a
+ // barrier, and only then allow weak processing. To minimize the wait time at
+ // that barrier we do the strong nmethod processing first, and immediately after-
+ // wards indicate that that thread is done. Hopefully other root processing after
+ // nmethod processing is enough so there is no need to wait.
+ //
+ // This is only required in the concurrent start pause with class unloading enabled.
{
G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::ThreadRoots, worker_i);
bool is_par = n_workers() > 1;
@@ -207,6 +196,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_nmethods_done) {
+ worker_has_discovered_all_strong_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());
+ }
+ }
}
void G1RootProcessor::process_vm_roots(G1RootClosures* closures,
--- a/src/hotspot/share/gc/g1/g1RootProcessor.hpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1RootProcessor.hpp Mon Sep 23 11:37:02 2019 +0200
@@ -69,12 +69,13 @@
G1RP_PS_NumElements
};
- void worker_has_discovered_all_strong_classes();
- void wait_until_all_strong_classes_discovered();
+ void worker_has_discovered_all_strong_nmethods();
+ void wait_until_all_strong_nmethods_discovered();
void process_java_roots(G1RootClosures* closures,
G1GCPhaseTimes* phase_times,
- uint worker_i);
+ uint worker_i,
+ bool notify_claimed_nmethods_done = false);
void process_vm_roots(G1RootClosures* closures,
G1GCPhaseTimes* phase_times,
--- a/src/hotspot/share/gc/g1/g1SharedClosures.hpp Mon Sep 23 11:36:53 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1SharedClosures.hpp Mon Sep 23 11:37:02 2019 +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) {}
};
--- a/test/hotspot/jtreg/gc/g1/TestGCLogMessages.java Mon Sep 23 11:36:53 2019 +0200
+++ b/test/hotspot/jtreg/gc/g1/TestGCLogMessages.java Mon Sep 23 11:37:02 2019 +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),
--- a/test/jdk/jdk/jfr/event/gc/collection/TestG1ParallelPhases.java Mon Sep 23 11:36:53 2019 +0200
+++ b/test/jdk/jdk/jfr/event/gc/collection/TestG1ParallelPhases.java Mon Sep 23 11:37:02 2019 +0200
@@ -98,8 +98,7 @@
"CLDGRoots",
"JVMTIRoots",
"CMRefRoots",
- "WaitForStrongCLD",
- "WeakCLDRoots",
+ "WaitForStrongRoots",
"MergeER",
"MergeHCC",
"MergeRS",