# HG changeset patch
# User johnc
# Date 1357259302 28800
# Node ID d275c3dc73e60ca11293eed447c15e7e452edb05
# Parent  37f7535e5f18c04f1629fed5e8819432ea0cffc0
8004816: G1: Kitchensink failures after marking stack changes
Summary: Reset the marking state, including the mark stack overflow flag, in the event of a marking stack overflow during serial reference processing.
Reviewed-by: jmasa

diff -r 37f7535e5f18 -r d275c3dc73e6 src/share/vm/gc_implementation/g1/concurrentMark.cpp
--- a/src/share/vm/gc_implementation/g1/concurrentMark.cpp	Fri Dec 21 11:45:34 2012 -0800
+++ b/src/share/vm/gc_implementation/g1/concurrentMark.cpp	Thu Jan 03 16:28:22 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -192,6 +192,7 @@
   setEmpty();
   _capacity = (jint) capacity;
   _saved_index = -1;
+  _should_expand = false;
   NOT_PRODUCT(_max_depth = 0);
   return true;
 }
@@ -747,8 +748,8 @@
   assert(_heap_end != NULL, "heap bounds should look ok");
   assert(_heap_start < _heap_end, "heap bounds should look ok");
 
-  // reset all the marking data structures and any necessary flags
-  clear_marking_state();
+  // Reset all the marking data structures and any necessary flags
+  reset_marking_state();
 
   if (verbose_low()) {
     gclog_or_tty->print_cr("[global] resetting");
@@ -766,6 +767,23 @@
   set_concurrent_marking_in_progress();
 }
 
+
+void ConcurrentMark::reset_marking_state(bool clear_overflow) {
+  _markStack.set_should_expand();
+  _markStack.setEmpty();        // Also clears the _markStack overflow flag
+  if (clear_overflow) {
+    clear_has_overflown();
+  } else {
+    assert(has_overflown(), "pre-condition");
+  }
+  _finger = _heap_start;
+
+  for (uint i = 0; i < _max_worker_id; ++i) {
+    CMTaskQueue* queue = _task_queues->queue(i);
+    queue->set_empty();
+  }
+}
+
 void ConcurrentMark::set_phase(uint active_tasks, bool concurrent) {
   assert(active_tasks <= _max_worker_id, "we should not have more");
 
@@ -796,7 +814,7 @@
 void ConcurrentMark::set_non_marking_state() {
   // We set the global marking state to some default values when we're
   // not doing marking.
-  clear_marking_state();
+  reset_marking_state();
   _active_tasks = 0;
   clear_concurrent_marking_in_progress();
 }
@@ -963,7 +981,7 @@
     // not clear the overflow flag since we rely on it being true when
     // we exit this method to abort the pause and restart concurent
     // marking.
-    clear_marking_state(concurrent() /* clear_overflow */);
+    reset_marking_state(concurrent() /* clear_overflow */);
     force_overflow()->update();
 
     if (G1Log::fine()) {
@@ -1257,8 +1275,9 @@
   if (has_overflown()) {
     // Oops.  We overflowed.  Restart concurrent marking.
     _restart_for_overflow = true;
-    // Clear the flag. We do not need it any more.
-    clear_has_overflown();
+    // Clear the marking state because we will be restarting
+    // marking due to overflowing the global mark stack.
+    reset_marking_state();
     if (G1TraceMarkStackOverflow) {
       gclog_or_tty->print_cr("\nRemark led to restart for overflow.");
     }
@@ -1282,6 +1301,8 @@
                        /* option */ VerifyOption_G1UseNextMarking);
     }
     assert(!restart_for_overflow(), "sanity");
+    // Completely reset the marking state since marking completed
+    set_non_marking_state();
   }
 
   // Expand the marking stack, if we have to and if we can.
@@ -1289,11 +1310,6 @@
     _markStack.expand();
   }
 
-  // Reset the marking state if marking completed
-  if (!restart_for_overflow()) {
-    set_non_marking_state();
-  }
-
 #if VERIFY_OBJS_PROCESSED
   _scan_obj_cl.objs_processed = 0;
   ThreadLocalObjQueue::objs_enqueued = 0;
@@ -2963,22 +2979,6 @@
 }
 #endif // PRODUCT
 
-void ConcurrentMark::clear_marking_state(bool clear_overflow) {
-  _markStack.set_should_expand();
-  _markStack.setEmpty();        // Also clears the _markStack overflow flag
-  if (clear_overflow) {
-    clear_has_overflown();
-  } else {
-    assert(has_overflown(), "pre-condition");
-  }
-  _finger = _heap_start;
-
-  for (uint i = 0; i < _max_worker_id; ++i) {
-    CMTaskQueue* queue = _task_queues->queue(i);
-    queue->set_empty();
-  }
-}
-
 // Aggregate the counting data that was constructed concurrently
 // with marking.
 class AggregateCountDataHRClosure: public HeapRegionClosure {
@@ -3185,7 +3185,7 @@
   // Clear the liveness counting data
   clear_all_count_data();
   // Empty mark stack
-  clear_marking_state();
+  reset_marking_state();
   for (uint i = 0; i < _max_worker_id; ++i) {
     _tasks[i]->clear_region_fields();
   }
diff -r 37f7535e5f18 -r d275c3dc73e6 src/share/vm/gc_implementation/g1/concurrentMark.hpp
--- a/src/share/vm/gc_implementation/g1/concurrentMark.hpp	Fri Dec 21 11:45:34 2012 -0800
+++ b/src/share/vm/gc_implementation/g1/concurrentMark.hpp	Thu Jan 03 16:28:22 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -478,15 +478,18 @@
   // It resets the global marking data structures, as well as the
   // task local ones; should be called during initial mark.
   void reset();
-  // It resets all the marking data structures.
-  void clear_marking_state(bool clear_overflow = true);
+
+  // Resets all the marking data structures. Called when we have to restart
+  // marking or when marking completes (via set_non_marking_state below).
+  void reset_marking_state(bool clear_overflow = true);
+
+  // We do this after we're done with marking so that the marking data
+  // structures are initialised to a sensible and predictable state.
+  void set_non_marking_state();
 
   // It should be called to indicate which phase we're in (concurrent
   // mark or remark) and how many threads are currently active.
   void set_phase(uint active_tasks, bool concurrent);
-  // We do this after we're done with marking so that the marking data
-  // structures are initialised to a sensible and predictable state.
-  void set_non_marking_state();
 
   // prints all gathered CM-related statistics
   void print_stats();
diff -r 37f7535e5f18 -r d275c3dc73e6 src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp
--- a/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp	Fri Dec 21 11:45:34 2012 -0800
+++ b/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp	Thu Jan 03 16:28:22 2013 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -159,13 +159,11 @@
           VM_CGC_Operation op(&final_cl, verbose_str, true /* needs_pll */);
           VMThread::execute(&op);
         }
-        if (cm()->restart_for_overflow() &&
-            G1TraceMarkStackOverflow) {
-          gclog_or_tty->print_cr("Restarting conc marking because of MS overflow "
-                                 "in remark (restart #%d).", iter);
-        }
-
         if (cm()->restart_for_overflow()) {
+          if (G1TraceMarkStackOverflow) {
+            gclog_or_tty->print_cr("Restarting conc marking because of MS overflow "
+                                   "in remark (restart #%d).", iter);
+          }
           if (G1Log::fine()) {
             gclog_or_tty->date_stamp(PrintGCDateStamps);
             gclog_or_tty->stamp(PrintGCTimeStamps);