8232747: Shenandoah: Concurrent GC should deactivate SATB before processing weak roots
Reviewed-by: shade
--- a/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahConcurrentMark.cpp Tue Oct 22 11:59:42 2019 -0400
@@ -442,8 +442,6 @@
weak_refs_work(full_gc);
}
- _heap->parallel_cleaning(full_gc);
-
assert(task_queues()->is_empty(), "Should be empty");
TASKQUEUE_STATS_ONLY(task_queues()->print_taskqueue_stats());
TASKQUEUE_STATS_ONLY(task_queues()->reset_taskqueue_stats());
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Tue Oct 22 11:59:42 2019 -0400
@@ -1478,6 +1478,12 @@
if (!cancelled_gc()) {
concurrent_mark()->finish_mark_from_roots(/* full_gc = */ false);
+ // Marking is completed, deactivate SATB barrier
+ set_concurrent_mark_in_progress(false);
+ mark_complete_marking_context();
+
+ parallel_cleaning(false /* full gc*/);
+
if (has_forwarded_objects()) {
// Degen may be caused by failed evacuation of roots
if (is_degenerated_gc_in_progress()) {
@@ -1485,14 +1491,12 @@
} else {
concurrent_mark()->update_thread_roots(ShenandoahPhaseTimings::update_roots);
}
+ set_has_forwarded_objects(false);
}
if (ShenandoahVerify) {
verifier()->verify_roots_no_forwarded();
}
-
- stop_concurrent_marking();
-
// All allocations past TAMS are implicitly live, adjust the region data.
// Bitmaps/TAMS are swapped at this point, so we need to poll complete bitmap.
{
@@ -1575,8 +1579,10 @@
}
} else {
+ // If this cycle was updating references, we need to keep the has_forwarded_objects
+ // flag on, for subsequent phases to deal with it.
concurrent_mark()->cancel();
- stop_concurrent_marking();
+ set_concurrent_mark_in_progress(false);
if (process_references()) {
// Abandon reference processing right away: pre-cleaning must have failed.
@@ -1872,17 +1878,6 @@
op_full(GCCause::_shenandoah_upgrade_to_full_gc);
}
-void ShenandoahHeap::stop_concurrent_marking() {
- assert(is_concurrent_mark_in_progress(), "How else could we get here?");
- set_concurrent_mark_in_progress(false);
- if (!cancelled_gc()) {
- // If we needed to update refs, and concurrent marking has been cancelled,
- // we need to finish updating references.
- set_has_forwarded_objects(false);
- mark_complete_marking_context();
- }
-}
-
void ShenandoahHeap::force_satb_flush_all_threads() {
if (!is_concurrent_mark_in_progress() && !is_concurrent_traversal_in_progress()) {
// No need to flush SATBs
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp Tue Oct 22 11:59:42 2019 -0400
@@ -717,8 +717,6 @@
void deduplicate_string(oop str);
- void stop_concurrent_marking();
-
private:
void trash_cset_regions();
void update_heap_references(bool concurrent);
--- a/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp Tue Oct 22 14:05:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkCompact.cpp Tue Oct 22 11:59:42 2019 -0400
@@ -110,7 +110,7 @@
// b. Cancel concurrent mark, if in progress
if (heap->is_concurrent_mark_in_progress()) {
heap->concurrent_mark()->cancel();
- heap->stop_concurrent_marking();
+ heap->set_concurrent_mark_in_progress(false);
}
assert(!heap->is_concurrent_mark_in_progress(), "sanity");
@@ -243,8 +243,8 @@
cm->update_roots(ShenandoahPhaseTimings::full_gc_roots);
cm->mark_roots(ShenandoahPhaseTimings::full_gc_roots);
cm->finish_mark_from_roots(/* full_gc = */ true);
-
heap->mark_complete_marking_context();
+ heap->parallel_cleaning(true /* full_gc */);
}
class ShenandoahPrepareForCompactionObjectClosure : public ObjectClosure {