8295118: G1: Clear CLD claim marks concurrently

Reviewed-by: iwalulya, sjohanss, ayang

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp

@@ -725,8 +725,6 @@ void G1ConcurrentMark::clear_bitmap(WorkerThreads* workers) {
 }
 
 class G1PreConcurrentStartTask : public G1BatchedTask {
-  // Concurrent start needs claim bits to keep track of the marked-through CLDs.
-  class CLDClearClaimedMarksTask;
   // Reset marking state.
   class ResetMarkingStateTask;
   // For each region note start of marking.
@@ -736,14 +734,6 @@ class G1PreConcurrentStartTask : public G1BatchedTask {
   G1PreConcurrentStartTask(GCCause::Cause cause, G1ConcurrentMark* cm);
 };
 
-class G1PreConcurrentStartTask::CLDClearClaimedMarksTask : public G1AbstractSubTask {
-public:
-  CLDClearClaimedMarksTask() : G1AbstractSubTask(G1GCPhaseTimes::CLDClearClaimedMarks) { }
-
-  double worker_cost() const override { return 1.0; }
-  void do_work(uint worker_id) override;
-};
-
 class G1PreConcurrentStartTask::ResetMarkingStateTask : public G1AbstractSubTask {
   G1ConcurrentMark* _cm;
 public:
@@ -769,10 +759,6 @@ class G1PreConcurrentStartTask::NoteStartOfMarkTask : public G1AbstractSubTask {
   void do_work(uint worker_id) override;
 };
 
-void G1PreConcurrentStartTask::CLDClearClaimedMarksTask::do_work(uint worker_id) {
-  ClassLoaderDataGraph::clear_claimed_marks();
-}
-
 void G1PreConcurrentStartTask::ResetMarkingStateTask::do_work(uint worker_id) {
   // Reset marking state.
   _cm->reset();
@@ -797,7 +783,6 @@ void G1PreConcurrentStartTask::NoteStartOfMarkTask::set_max_workers(uint max_wor
 
 G1PreConcurrentStartTask::G1PreConcurrentStartTask(GCCause::Cause cause, G1ConcurrentMark* cm) :
   G1BatchedTask("Pre Concurrent Start", G1CollectedHeap::heap()->phase_times()) {
-  add_serial_task(new CLDClearClaimedMarksTask());
   add_serial_task(new ResetMarkingStateTask(cm));
   add_parallel_task(new NoteStartOfMarkTask());
 };
@@ -807,6 +792,8 @@ void G1ConcurrentMark::pre_concurrent_start(GCCause::Cause cause) {
 
   G1CollectedHeap::start_codecache_marking_cycle_if_inactive();
 
+  ClassLoaderDataGraph::verify_claimed_marks_cleared(ClassLoaderData::_claim_strong);
+
   G1PreConcurrentStartTask cl(cause, this);
   G1CollectedHeap::heap()->run_batch_task(&cl);
 

src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp

@@ -161,9 +161,10 @@ bool G1ConcurrentMarkThread::wait_for_next_cycle() {
   return !should_terminate();
 }
 
-void G1ConcurrentMarkThread::phase_clear_cld_claimed_marks() {
+bool G1ConcurrentMarkThread::phase_clear_cld_claimed_marks() {
   G1ConcPhaseTimer p(_cm, "Concurrent Clear Claimed Marks");
   ClassLoaderDataGraph::clear_claimed_marks();
+  return _cm->has_aborted();
 }
 
 bool G1ConcurrentMarkThread::phase_scan_root_regions() {
@@ -267,9 +268,6 @@ void G1ConcurrentMarkThread::concurrent_mark_cycle_do() {
   HandleMark hm(Thread::current());
   ResourceMark rm;
 
-  // Phase 1: Clear CLD claimed marks.
-  phase_clear_cld_claimed_marks();
-
   // We have to ensure that we finish scanning the root regions
   // before the next GC takes place. To ensure this we have to
   // make sure that we do not join the STS until the root regions
@@ -287,21 +285,24 @@ void G1ConcurrentMarkThread::concurrent_mark_cycle_do() {
   // We can not easily abort before root region scan either because of the
   // reasons mentioned in G1CollectedHeap::abort_concurrent_cycle().
 
-  // Phase 2: Scan root regions.
+  // Phase 1: Scan root regions.
   if (phase_scan_root_regions()) return;
 
-  // Phase 3: Actual mark loop.
+  // Phase 2: Actual mark loop.
   if (phase_mark_loop()) return;
 
-  // Phase 4: Rebuild remembered sets and scrub dead objects.
+  // Phase 3: Rebuild remembered sets and scrub dead objects.
   if (phase_rebuild_and_scrub()) return;
 
-  // Phase 5: Wait for Cleanup.
+  // Phase 4: Wait for Cleanup.
   if (phase_delay_to_keep_mmu_before_cleanup()) return;
 
-  // Phase 6: Cleanup pause
+  // Phase 5: Cleanup pause
   if (phase_cleanup()) return;
 
+  // Phase 6: Clear CLD claimed marks.
+  if (phase_clear_cld_claimed_marks()) return;
+
   // Phase 7: Clear bitmap for next mark.
   phase_clear_bitmap_for_next_mark();
 }
@@ -316,7 +317,10 @@ void G1ConcurrentMarkThread::concurrent_undo_cycle_do() {
 
   _cm->flush_all_task_caches();
 
-  // Phase 1: Clear bitmap for next mark.
+  // Phase 1: Clear CLD claimed marks.
+  if (phase_clear_cld_claimed_marks()) return;
+
+  // Phase 2: Clear bitmap for next mark.
   phase_clear_bitmap_for_next_mark();
 }
 

src/hotspot/share/gc/g1/g1ConcurrentMarkThread.hpp

@@ -55,10 +55,8 @@ class G1ConcurrentMarkThread: public ConcurrentGCThread {
 
   // Phases and subphases for the full concurrent marking cycle in order.
   //
-  // All these methods return true if the marking should be aborted. Except
-  // phase_clear_cld_claimed_marks() because we must not abort before
-  // scanning the root regions because of a potential deadlock otherwise.
-  void phase_clear_cld_claimed_marks();
+  // All these methods return true if the marking should be aborted.
+  bool phase_clear_cld_claimed_marks();
   bool phase_scan_root_regions();
 
   bool phase_mark_loop();

src/hotspot/share/gc/g1/g1FullCollector.cpp

@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "classfile/classLoaderDataGraph.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "code/codeCache.hpp"
 #include "compiler/oopMap.hpp"
@@ -180,6 +181,10 @@ void G1FullCollector::prepare_collection() {
   if (in_concurrent_cycle) {
     GCTraceTime(Debug, gc) debug("Clear Bitmap");
     _heap->concurrent_mark()->clear_bitmap(_heap->workers());
+    // Need cleared claim bits for the mark phase.
+    ClassLoaderDataGraph::clear_claimed_marks();
+  } else {
+    ClassLoaderDataGraph::verify_claimed_marks_cleared(ClassLoaderData::_claim_strong);
   }
 
   _heap->gc_prologue(true);
@@ -222,6 +227,9 @@ void G1FullCollector::complete_collection() {
   // update the derived pointer table.
   update_derived_pointers();
 
+  // Need cleared claim bits for the next concurrent marking.
+  ClassLoaderDataGraph::clear_claimed_marks();
+
   // Prepare the bitmap for the next (potentially concurrent) marking.
   _heap->concurrent_mark()->clear_bitmap(_heap->workers());
 

src/hotspot/share/gc/g1/g1FullGCMarkTask.cpp

@@ -23,7 +23,6 @@
  */
 
 #include "precompiled.hpp"
-#include "classfile/classLoaderDataGraph.hpp"
 #include "gc/g1/g1CollectedHeap.hpp"
 #include "gc/g1/g1FullCollector.hpp"
 #include "gc/g1/g1FullGCMarker.hpp"
@@ -37,8 +36,6 @@ G1FullGCMarkTask::G1FullGCMarkTask(G1FullCollector* collector) :
     G1FullGCTask("G1 Parallel Marking Task", collector),
     _root_processor(G1CollectedHeap::heap(), collector->workers()),
     _terminator(collector->workers(), collector->array_queue_set()) {
-  // Need cleared claim bits for the roots processing
-  ClassLoaderDataGraph::clear_claimed_marks();
 }
 
 void G1FullGCMarkTask::work(uint worker_id) {

src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp

@@ -159,7 +159,6 @@ G1GCPhaseTimes::G1GCPhaseTimes(STWGCTimer* gc_timer, uint max_gc_threads) :
   _gc_par_phases[NonYoungFreeCSet] = new WorkerDataArray<double>("NonYoungFreeCSet", "Non-Young Free Collection Set (ms):", max_gc_threads);
   _gc_par_phases[RebuildFreeList] = new WorkerDataArray<double>("RebuildFreeList", "Parallel Rebuild Free List (ms):", max_gc_threads);
 
-  _gc_par_phases[CLDClearClaimedMarks] = new WorkerDataArray<double>("CLDClearClaimedMarks", "Clear Claimed Marks (ms):", max_gc_threads);
   _gc_par_phases[ResetMarkingState] = new WorkerDataArray<double>("ResetMarkingState", "Reset Marking State (ms):", max_gc_threads);
   _gc_par_phases[NoteStartOfMark] = new WorkerDataArray<double>("NoteStartOfMark", "Note Start Of Mark (ms):", max_gc_threads);
 
@@ -384,8 +383,7 @@ void G1GCPhaseTimes::trace_count(const char* name, size_t value) const {
 }
 
 double G1GCPhaseTimes::print_pre_evacuate_collection_set() const {
-  const double pre_concurrent_start_ms = average_time_ms(CLDClearClaimedMarks) +
-                                         average_time_ms(ResetMarkingState) +
+  const double pre_concurrent_start_ms = average_time_ms(ResetMarkingState) +
                                          average_time_ms(NoteStartOfMark);
 
   const double sum_ms = _root_region_scan_wait_time_ms +
@@ -410,7 +408,6 @@ double G1GCPhaseTimes::print_pre_evacuate_collection_set() const {
   debug_time("Prepare Heap Roots", _recorded_prepare_heap_roots_time_ms);
 
   if (pre_concurrent_start_ms > 0.0) {
-    debug_phase(_gc_par_phases[CLDClearClaimedMarks]);
     debug_phase(_gc_par_phases[ResetMarkingState]);
     debug_phase(_gc_par_phases[NoteStartOfMark]);
   }

src/hotspot/share/gc/g1/g1GCPhaseTimes.hpp

@@ -89,7 +89,6 @@ class G1GCPhaseTimes : public CHeapObj<mtGC> {
     EagerlyReclaimHumongousObjects,
     RestorePreservedMarks,
     ClearRetainedRegionBitmaps,
-    CLDClearClaimedMarks,
     ResetMarkingState,
     NoteStartOfMark,
     GCParPhasesSentinel

test/hotspot/jtreg/gc/g1/TestGCLogMessages.java

@@ -301,7 +301,6 @@ private void testWithEvacuationFailureLogs() throws Exception {
     }
 
     LogMessageWithLevel concurrentStartMessages[] = new LogMessageWithLevel[] {
-        new LogMessageWithLevel("Clear Claimed Marks", Level.DEBUG),
         new LogMessageWithLevel("Reset Marking State", Level.DEBUG),
         new LogMessageWithLevel("Note Start Of Mark", Level.DEBUG),
     };

test/jdk/jdk/jfr/event/gc/collection/TestG1ParallelPhases.java

@@ -117,7 +117,6 @@ public static void main(String[] args) throws IOException {
             "YoungFreeCSet",
             "RebuildFreeList",
             "SampleCandidates",
-            "CLDClearClaimedMarks",
             "ResetMarkingState",
             "NoteStartOfMark"
         );