8140597: Forcing an initial mark causes G1 to abort mixed collections

Postpone the initial mark request until the current mixed GC phase has finished.

Reviewed-by: mgerdin

hotspot/src/share/vm/gc/g1/g1CollectorPolicy.cpp

@@ -930,8 +930,12 @@ double G1CollectorPolicy::constant_other_time_ms(double pause_time_ms) const {
   return other_time_ms(pause_time_ms) - young_other_time_ms() - non_young_other_time_ms();
 }
 
+bool G1CollectorPolicy::about_to_start_mixed_phase() const {
+  return _g1->concurrent_mark()->cmThread()->during_cycle() || collector_state()->last_young_gc();
+}
+
 bool G1CollectorPolicy::need_to_start_conc_mark(const char* source, size_t alloc_word_size) {
-  if (_g1->concurrent_mark()->cmThread()->during_cycle()) {
+  if (about_to_start_mixed_phase()) {
     return false;
   }
 
@@ -1058,17 +1062,13 @@ void G1CollectorPolicy::record_collection_pause_end(double pause_time_ms, size_t
   if (collector_state()->last_young_gc()) {
     // This is supposed to to be the "last young GC" before we start
     // doing mixed GCs. Here we decide whether to start mixed GCs or not.
+    assert(!last_pause_included_initial_mark, "The last young GC is not allowed to be an initial mark GC");
 
-    if (!last_pause_included_initial_mark) {
+    if (next_gc_should_be_mixed("start mixed GCs",
-      if (next_gc_should_be_mixed("start mixed GCs",
+                                "do not start mixed GCs")) {
-                                  "do not start mixed GCs")) {
+      collector_state()->set_gcs_are_young(false);
-        collector_state()->set_gcs_are_young(false);
-      }
-    } else {
-      ergo_verbose0(ErgoMixedGCs,
-                    "do not start mixed GCs",
-                    ergo_format_reason("concurrent cycle is about to start"));
     }
+
     collector_state()->set_last_young_gc(false);
   }
 
@@ -1586,8 +1586,10 @@ void G1CollectorPolicy::update_survivors_policy() {
         HeapRegion::GrainWords * _max_survivor_regions, counters());
 }
 
-bool G1CollectorPolicy::force_initial_mark_if_outside_cycle(
+bool G1CollectorPolicy::force_initial_mark_if_outside_cycle(GCCause::Cause gc_cause) {
-                                                     GCCause::Cause gc_cause) {
+  // We actually check whether we are marking here and not if we are in a
+  // reclamation phase. This means that we will schedule a concurrent mark
+  // even while we are still in the process of reclaiming memory.
   bool during_cycle = _g1->concurrent_mark()->cmThread()->during_cycle();
   if (!during_cycle) {
     ergo_verbose1(ErgoConcCycles,
@@ -1607,8 +1609,7 @@ bool G1CollectorPolicy::force_initial_mark_if_outside_cycle(
   }
 }
 
-void
+void G1CollectorPolicy::decide_on_conc_mark_initiation() {
-G1CollectorPolicy::decide_on_conc_mark_initiation() {
   // We are about to decide on whether this pause will be an
   // initial-mark pause.
 
@@ -1623,21 +1624,11 @@ G1CollectorPolicy::decide_on_conc_mark_initiation() {
     // gone over the initiating threshold and we should start a
     // concurrent marking cycle. So we might initiate one.
 
-    bool during_cycle = _g1->concurrent_mark()->cmThread()->during_cycle();
+    if (!about_to_start_mixed_phase() && collector_state()->gcs_are_young()) {
-    if (!during_cycle) {
+      // Initiate a new initial mark only if there is no marking or reclamation going
-      // The concurrent marking thread is not "during a cycle", i.e.,
+      // on.
-      // it has completed the last one. So we can go ahead and
-      // initiate a new cycle.
 
       collector_state()->set_during_initial_mark_pause(true);
-      // We do not allow mixed GCs during marking.
-      if (!collector_state()->gcs_are_young()) {
-        collector_state()->set_gcs_are_young(true);
-        ergo_verbose0(ErgoMixedGCs,
-                      "end mixed GCs",
-                      ergo_format_reason("concurrent cycle is about to start"));
-      }
-
       // And we can now clear initiate_conc_mark_if_possible() as
       // we've already acted on it.
       collector_state()->set_initiate_conc_mark_if_possible(false);

hotspot/src/share/vm/gc/g1/g1CollectorPolicy.hpp

@@ -570,6 +570,8 @@ public:
 
   bool need_to_start_conc_mark(const char* source, size_t alloc_word_size = 0);
 
+  bool about_to_start_mixed_phase() const;
+
   // Record the start and end of an evacuation pause.
   void record_collection_pause_start(double start_time_sec);
   void record_collection_pause_end(double pause_time_ms, size_t cards_scanned);

hotspot/src/share/vm/prims/whitebox.cpp

@@ -322,8 +322,8 @@ WB_ENTRY(jlong, WB_G1NumFreeRegions(JNIEnv* env, jobject o))
 WB_END
 
 WB_ENTRY(jboolean, WB_G1InConcurrentMark(JNIEnv* env, jobject o))
-  G1CollectedHeap* g1 = G1CollectedHeap::heap();
+  G1CollectedHeap* g1h = G1CollectedHeap::heap();
-  return g1->concurrent_mark()->cmThread()->during_cycle();
+  return g1h->concurrent_mark()->cmThread()->during_cycle();
 WB_END
 
 WB_ENTRY(jboolean, WB_G1StartMarkCycle(JNIEnv* env, jobject o))