8221435: Shenandoah should not mark through weak roots
Reviewed-by: rkennke, shade
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp Tue Mar 26 12:12:49 2019 -0400
@@ -120,11 +120,10 @@
CLDToOopClosure clds_cl(oops, ClassLoaderData::_claim_strong);
MarkingCodeBlobClosure blobs_cl(oops, ! CodeBlobToOopClosure::FixRelocations);
- OopClosure* weak_oops = _process_refs ? NULL : oops;
ResourceMark m;
if (heap->unload_classes()) {
- _rp->process_strong_roots(oops, weak_oops, &clds_cl, NULL, &blobs_cl, NULL, worker_id);
+ _rp->process_strong_roots(oops, &clds_cl, NULL, &blobs_cl, NULL, worker_id);
} else {
if (ShenandoahConcurrentScanCodeRoots) {
CodeBlobClosure* code_blobs = NULL;
@@ -137,9 +136,9 @@
code_blobs = &assert_to_space;
}
#endif
- _rp->process_all_roots(oops, weak_oops, &clds_cl, code_blobs, NULL, worker_id);
+ _rp->process_all_roots(oops, &clds_cl, code_blobs, NULL, worker_id);
} else {
- _rp->process_all_roots(oops, weak_oops, &clds_cl, &blobs_cl, NULL, worker_id);
+ _rp->process_all_roots(oops, &clds_cl, &blobs_cl, NULL, worker_id);
}
}
}
@@ -177,7 +176,7 @@
DEBUG_ONLY(&assert_to_space)
NOT_DEBUG(NULL);
}
- _rp->process_all_roots(&cl, &cl, &cldCl, code_blobs, NULL, worker_id);
+ _rp->update_all_roots(&cl, &cldCl, code_blobs, NULL, worker_id);
}
};
@@ -446,11 +445,16 @@
weak_refs_work(full_gc);
}
+ weak_roots_work();
+
// And finally finish class unloading
if (_heap->unload_classes()) {
_heap->unload_classes_and_cleanup_tables(full_gc);
+ } else if (ShenandoahStringDedup::is_enabled()) {
+ ShenandoahIsAliveSelector alive;
+ BoolObjectClosure* is_alive = alive.is_alive_closure();
+ ShenandoahStringDedup::unlink_or_oops_do(is_alive, NULL, false);
}
-
assert(task_queues()->is_empty(), "Should be empty");
TASKQUEUE_STATS_ONLY(task_queues()->print_taskqueue_stats());
TASKQUEUE_STATS_ONLY(task_queues()->reset_taskqueue_stats());
@@ -555,11 +559,13 @@
private:
template <class T>
inline void do_oop_work(T* p) {
+#ifdef ASSERT
T o = RawAccess<>::oop_load(p);
if (!CompressedOops::is_null(o)) {
oop obj = CompressedOops::decode_not_null(o);
shenandoah_assert_not_forwarded(p, obj);
}
+#endif
}
public:
@@ -648,6 +654,23 @@
}
+// Process leftover weak oops: update them, if needed or assert they do not
+// need updating otherwise.
+// Weak processor API requires us to visit the oops, even if we are not doing
+// anything to them.
+void ShenandoahConcurrentMark::weak_roots_work() {
+ WorkGang* workers = _heap->workers();
+ ShenandoahIsAliveSelector is_alive;
+
+ if (_heap->has_forwarded_objects()) {
+ ShenandoahWeakUpdateClosure cl;
+ WeakProcessor::weak_oops_do(workers, is_alive.is_alive_closure(), &cl, 1);
+ } else {
+ ShenandoahWeakAssertNotForwardedClosure cl;
+ WeakProcessor::weak_oops_do(workers, is_alive.is_alive_closure(), &cl, 1);
+ }
+}
+
void ShenandoahConcurrentMark::weak_refs_work_doit(bool full_gc) {
ReferenceProcessor* rp = _heap->ref_processor();
@@ -689,26 +712,18 @@
ShenandoahGCPhase phase(phase_process);
ShenandoahTerminationTracker phase_term(phase_process_termination);
- // Process leftover weak oops: update them, if needed (using parallel version),
- // or assert they do not need updating (using serial version) otherwise.
- // Weak processor API requires us to visit the oops, even if we are not doing
- // anything to them.
if (_heap->has_forwarded_objects()) {
ShenandoahCMKeepAliveUpdateClosure keep_alive(get_queue(serial_worker_id));
rp->process_discovered_references(is_alive.is_alive_closure(), &keep_alive,
&complete_gc, &executor,
&pt);
- ShenandoahWeakUpdateClosure cl;
- WeakProcessor::weak_oops_do(workers, is_alive.is_alive_closure(), &cl, 1);
} else {
ShenandoahCMKeepAliveClosure keep_alive(get_queue(serial_worker_id));
rp->process_discovered_references(is_alive.is_alive_closure(), &keep_alive,
&complete_gc, &executor,
&pt);
- ShenandoahWeakAssertNotForwardedClosure cl;
- WeakProcessor::weak_oops_do(is_alive.is_alive_closure(), &cl);
}
pt.print_all_references();
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.hpp Tue Mar 26 12:12:49 2019 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013, 2018, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2013, 2019, Red Hat, Inc. All rights reserved.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
@@ -86,6 +86,8 @@
void weak_refs_work(bool full_gc);
void weak_refs_work_doit(bool full_gc);
+ void weak_roots_work();
+
public:
void preclean_weak_refs();
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Tue Mar 26 12:12:49 2019 -0400
@@ -1127,16 +1127,6 @@
#endif
}
-void ShenandoahHeap::roots_iterate(OopClosure* cl) {
- assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Only iterate roots while world is stopped");
-
- CodeBlobToOopClosure blobsCl(cl, false);
- CLDToOopClosure cldCl(cl, ClassLoaderData::_claim_strong);
-
- ShenandoahRootProcessor rp(this, 1, ShenandoahPhaseTimings::_num_phases);
- rp.process_all_roots(cl, NULL, &cldCl, &blobsCl, NULL, 0);
-}
-
// Returns size in bytes
size_t ShenandoahHeap::unsafe_max_tlab_alloc(Thread *thread) const {
if (ShenandoahElasticTLAB) {
@@ -1337,7 +1327,7 @@
ObjectIterateScanRootClosure oops(&_aux_bit_map, &oop_stack);
CLDToOopClosure clds(&oops, ClassLoaderData::_claim_none);
CodeBlobToOopClosure blobs(&oops, false);
- rp.process_all_roots(&oops, &oops, &clds, &blobs, NULL, 0);
+ rp.process_all_roots(&oops, &clds, &blobs, NULL, 0);
// Work through the oop stack to traverse heap.
while (! oop_stack.is_empty()) {
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp Tue Mar 26 12:12:49 2019 -0400
@@ -742,8 +742,6 @@
void stop_concurrent_marking();
- void roots_iterate(OopClosure* cl);
-
private:
void trash_cset_regions();
void update_heap_references(bool concurrent);
--- a/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp Tue Mar 26 12:12:49 2019 -0400
@@ -572,9 +572,9 @@
MarkingCodeBlobClosure adjust_code_closure(&cl,
CodeBlobToOopClosure::FixRelocations);
- _rp->process_all_roots(&cl, &cl,
- &adjust_cld_closure,
- &adjust_code_closure, NULL, worker_id);
+ _rp->update_all_roots(&cl,
+ &adjust_cld_closure,
+ &adjust_code_closure, NULL, worker_id);
}
};
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp Tue Mar 26 12:12:49 2019 -0400
@@ -119,14 +119,9 @@
Management::oops_do(oops);
JvmtiExport::oops_do(oops);
JNIHandles::oops_do(oops);
- WeakProcessor::oops_do(oops);
ObjectSynchronizer::oops_do(oops);
SystemDictionary::oops_do(oops);
- if (ShenandoahStringDedup::is_enabled()) {
- ShenandoahStringDedup::oops_do_slow(oops);
- }
-
// Do thread roots the last. This allows verification code to find
// any broken objects from those special roots first, not the accidental
// dangling reference from the thread root.
@@ -134,7 +129,6 @@
}
void ShenandoahRootProcessor::process_strong_roots(OopClosure* oops,
- OopClosure* weak_oops,
CLDClosure* clds,
CLDClosure* weak_clds,
CodeBlobClosure* blobs,
@@ -142,13 +136,12 @@
uint worker_id) {
process_java_roots(oops, clds, weak_clds, blobs, thread_cl, worker_id);
- process_vm_roots(oops, NULL, weak_oops, worker_id);
+ process_vm_roots(oops, worker_id);
_process_strong_tasks->all_tasks_completed(n_workers());
}
void ShenandoahRootProcessor::process_all_roots(OopClosure* oops,
- OopClosure* weak_oops,
CLDClosure* clds,
CodeBlobClosure* blobs,
ThreadClosure* thread_cl,
@@ -156,7 +149,7 @@
ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
process_java_roots(oops, clds, clds, blobs, thread_cl, worker_id);
- process_vm_roots(oops, oops, weak_oops, worker_id);
+ process_vm_roots(oops, worker_id);
if (blobs != NULL) {
ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
@@ -164,6 +157,40 @@
}
_process_strong_tasks->all_tasks_completed(n_workers());
+
+}
+
+void ShenandoahRootProcessor::update_all_roots(OopClosure* oops,
+ CLDClosure* clds,
+ CodeBlobClosure* blobs,
+ ThreadClosure* thread_cl,
+ uint worker_id) {
+ process_all_roots(oops, clds, blobs, thread_cl, worker_id);
+
+ AlwaysTrueClosure always_true;
+ _weak_processor_task.work<AlwaysTrueClosure, OopClosure>(worker_id, &always_true, oops);
+ _processed_weak_roots = true;
+
+ if (ShenandoahStringDedup::is_enabled()) {
+ ShenandoahStringDedup::parallel_oops_do(&always_true, oops, worker_id);
+ }
+}
+
+void ShenandoahRootProcessor::traversal_update_all_roots(OopClosure* oops,
+ CLDClosure* clds,
+ CodeBlobClosure* blobs,
+ ThreadClosure* thread_cl,
+ uint worker_id) {
+ process_all_roots(oops, clds, blobs, thread_cl, worker_id);
+
+ ShenandoahIsAliveSelector is_alive;
+ BoolObjectClosure* is_alive_closure = is_alive.is_alive_closure();
+ _weak_processor_task.work<BoolObjectClosure, OopClosure>(worker_id, is_alive_closure, oops);
+ _processed_weak_roots = true;
+
+ if (ShenandoahStringDedup::is_enabled()) {
+ ShenandoahStringDedup::parallel_oops_do(is_alive_closure, oops, worker_id);
+ }
}
class ShenandoahParallelOopsDoThreadClosure : public ThreadClosure {
@@ -209,10 +236,7 @@
}
void ShenandoahRootProcessor::process_vm_roots(OopClosure* strong_roots,
- OopClosure* weak_roots,
- OopClosure* jni_weak_roots,
- uint worker_id)
-{
+ uint worker_id) {
ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
if (_process_strong_tasks->try_claim_task(SHENANDOAH_RP_PS_Universe_oops_do)) {
ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::UniverseRoots, worker_id);
@@ -235,15 +259,6 @@
ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::SystemDictionaryRoots, worker_id);
SystemDictionary::oops_do(strong_roots);
}
- if (jni_weak_roots != NULL) {
- AlwaysTrueClosure always_true;
- _weak_processor_task.work<AlwaysTrueClosure, OopClosure>(worker_id, &always_true, jni_weak_roots);
- _processed_weak_roots = true;
- }
-
- if (ShenandoahStringDedup::is_enabled() && weak_roots != NULL) {
- ShenandoahStringDedup::parallel_oops_do(weak_roots, worker_id);
- }
{
ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::ObjectSynchronizerRoots, worker_id);
--- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp Tue Mar 26 12:12:49 2019 -0400
@@ -73,8 +73,6 @@
uint worker_i);
void process_vm_roots(OopClosure* scan_non_heap_roots,
- OopClosure* scan_non_heap_weak_roots,
- OopClosure* weak_jni_roots,
uint worker_i);
void weak_processor_timing_to_shenandoah_timing(const WeakProcessorPhases::Phase wpp,
@@ -86,21 +84,40 @@
ShenandoahPhaseTimings::Phase phase);
~ShenandoahRootProcessor();
- // Apply oops, clds and blobs to all strongly reachable roots in the system
- void process_strong_roots(OopClosure* oops, OopClosure* weak_oops,
+ // Apply oops, clds and blobs to all strongly reachable roots in the system.
+ // Optionally, apply class loader closure to weak clds, depending on class unloading
+ // for the particular GC cycles.
+ void process_strong_roots(OopClosure* oops,
CLDClosure* clds,
CLDClosure* weak_clds,
CodeBlobClosure* blobs,
ThreadClosure* thread_cl,
uint worker_id);
- // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
- void process_all_roots(OopClosure* oops, OopClosure* weak_oops,
+ // Apply oops, clds and blobs to strongly reachable roots in the system
+ void process_all_roots(OopClosure* oops,
CLDClosure* clds,
CodeBlobClosure* blobs,
ThreadClosure* thread_cl,
uint worker_id);
+ // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
+ void update_all_roots(OopClosure* oops,
+ CLDClosure* clds,
+ CodeBlobClosure* blobs,
+ ThreadClosure* thread_cl,
+ uint worker_id);
+
+
+ // Apply oops, clds and blobs to strongly and weakly reachable roots in the system
+ // during traversal GC.
+ // It cleans up and updates weak roots in one iteration.
+ void traversal_update_all_roots(OopClosure* oops,
+ CLDClosure* clds,
+ CodeBlobClosure* blobs,
+ ThreadClosure* thread_cl,
+ uint worker_id);
+
// For slow debug/verification code
void process_all_roots_slow(OopClosure* oops);
--- a/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.cpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.cpp Tue Mar 26 12:12:49 2019 -0400
@@ -71,13 +71,13 @@
StringDedupTable::deduplicate(java_string, &dummy);
}
-void ShenandoahStringDedup::parallel_oops_do(OopClosure* cl, uint worker_id) {
+void ShenandoahStringDedup::parallel_oops_do(BoolObjectClosure* is_alive, OopClosure* cl, uint worker_id) {
assert(SafepointSynchronize::is_at_safepoint(), "Must be at a safepoint");
assert(is_enabled(), "String deduplication not enabled");
ShenandoahWorkerTimings* worker_times = ShenandoahHeap::heap()->phase_timings()->worker_times();
- StringDedupUnlinkOrOopsDoClosure sd_cl(NULL, cl);
+ StringDedupUnlinkOrOopsDoClosure sd_cl(is_alive, cl);
{
ShenandoahWorkerTimingsTracker x(worker_times, ShenandoahPhaseTimings::StringDedupQueueRoots, worker_id);
--- a/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.hpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahStringDedup.hpp Tue Mar 26 12:12:49 2019 -0400
@@ -38,11 +38,11 @@
// Deduplicate a string, the call is lock-free
static void deduplicate(oop java_string);
- static void parallel_oops_do(OopClosure* cl, uint worker_id);
+ static void parallel_oops_do(BoolObjectClosure* is_alive, OopClosure* cl, uint worker_id);
static void oops_do_slow(OopClosure* cl);
static inline bool is_candidate(oop obj);
-private:
+
static void unlink_or_oops_do(BoolObjectClosure* is_alive,
OopClosure* keep_alive,
bool allow_resize_and_rehash);
--- a/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp Fri Mar 29 15:59:28 2019 +0100
+++ b/src/hotspot/share/gc/shenandoah/shenandoahTraversalGC.cpp Tue Mar 26 12:12:49 2019 -0400
@@ -188,13 +188,13 @@
ShenandoahMarkCLDClosure cld_cl(&roots_cl);
MarkingCodeBlobClosure code_cl(&roots_cl, CodeBlobToOopClosure::FixRelocations);
if (unload_classes) {
- _rp->process_strong_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, NULL, NULL, NULL, worker_id);
+ _rp->process_strong_roots(&roots_cl, &cld_cl, NULL, NULL, NULL, worker_id);
// Need to pre-evac code roots here. Otherwise we might see from-space constants.
ShenandoahWorkerTimings* worker_times = _heap->phase_timings()->worker_times();
ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::CodeCacheRoots, worker_id);
_cset_coderoots->possibly_parallel_blobs_do(&code_cl);
} else {
- _rp->process_all_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, &code_cl, NULL, worker_id);
+ _rp->process_all_roots(&roots_cl, &cld_cl, &code_cl, NULL, worker_id);
}
}
}
@@ -270,9 +270,9 @@
ShenandoahTraversalSATBThreadsClosure tc(&satb_cl);
if (unload_classes) {
ShenandoahRemarkCLDClosure weak_cld_cl(&roots_cl);
- _rp->process_strong_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
+ _rp->process_strong_roots(&roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
} else {
- _rp->process_all_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, NULL, &tc, worker_id);
+ _rp->process_all_roots(&roots_cl, &cld_cl, NULL, &tc, worker_id);
}
} else {
ShenandoahTraversalDegenClosure roots_cl(q, rp);
@@ -280,9 +280,9 @@
ShenandoahTraversalSATBThreadsClosure tc(&satb_cl);
if (unload_classes) {
ShenandoahRemarkCLDClosure weak_cld_cl(&roots_cl);
- _rp->process_strong_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
+ _rp->process_strong_roots(&roots_cl, &cld_cl, &weak_cld_cl, NULL, &tc, worker_id);
} else {
- _rp->process_all_roots(&roots_cl, process_refs ? NULL : &roots_cl, &cld_cl, NULL, &tc, worker_id);
+ _rp->process_all_roots(&roots_cl, &cld_cl, NULL, &tc, worker_id);
}
}
@@ -594,8 +594,11 @@
weak_refs_work();
}
- if (!_heap->cancelled_gc() && _heap->unload_classes()) {
- _heap->unload_classes_and_cleanup_tables(false);
+ if (!_heap->cancelled_gc()) {
+ if (_heap->unload_classes()) {
+ _heap->unload_classes_and_cleanup_tables(false);
+ }
+
fixup_roots();
}
@@ -698,7 +701,7 @@
ShenandoahTraversalFixRootsClosure cl;
MarkingCodeBlobClosure blobsCl(&cl, CodeBlobToOopClosure::FixRelocations);
CLDToOopClosure cldCl(&cl, ClassLoaderData::_claim_strong);
- _rp->process_all_roots(&cl, &cl, &cldCl, &blobsCl, NULL, worker_id);
+ _rp->traversal_update_all_roots(&cl, &cldCl, &blobsCl, NULL, worker_id);
}
};