7114303: G1: assert(_g1->mark_in_progress()) failed: shouldn't be here otherwise
authorjohnc
Mon, 28 Nov 2011 09:49:05 -0800
changeset 11175 7fde26aecbe5
parent 11174 fccee5238e70
child 11176 9bb1ddd8da51
7114303: G1: assert(_g1->mark_in_progress()) failed: shouldn't be here otherwise Summary: Race between the VM thread reading G1CollectedHeap::_mark_in_progress and it being set by the concurrent mark thread when concurrent marking is aborted by a full GC. Have the concurrent mark thread join the SuspendibleThreadSet before changing the marking state. Reviewed-by: tonyp, brutisso
hotspot/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp
hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp
--- a/hotspot/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp	Tue Aug 09 10:16:01 2011 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp	Mon Nov 28 09:49:05 2011 -0800
@@ -191,7 +191,11 @@
         VM_CGC_Operation op(&cl_cl, verbose_str);
         VMThread::execute(&op);
       } else {
+        // We don't want to update the marking status if a GC pause
+        // is already underway.
+        _sts.join();
         g1h->set_marking_complete();
+        _sts.leave();
       }
 
       // Check if cleanup set the free_regions_coming flag. If it
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp	Tue Aug 09 10:16:01 2011 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp	Mon Nov 28 09:49:05 2011 -0800
@@ -946,10 +946,9 @@
     _cur_aux_times_set[i] = false;
   }
 
-  // These are initialized to zero here and they are set during
+  // This is initialized to zero here and is set during
   // the evacuation pause if marking is in progress.
   _cur_satb_drain_time_ms = 0.0;
-  _last_satb_drain_processed_buffers = 0;
 
   _last_young_gc_full = false;
 
@@ -1367,7 +1366,6 @@
 
     if (print_marking_info) {
       print_stats(1, "SATB Drain Time", _cur_satb_drain_time_ms);
-      print_stats(2, "Processed Buffers", _last_satb_drain_processed_buffers);
     }
 
     if (parallel) {
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp	Tue Aug 09 10:16:01 2011 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp	Mon Nov 28 09:49:05 2011 -0800
@@ -534,7 +534,6 @@
   double sum_of_values (double* data);
   double max_sum (double* data1, double* data2);
 
-  int _last_satb_drain_processed_buffers;
   double _last_pause_time_ms;
 
   size_t _bytes_in_collection_set_before_gc;
@@ -774,11 +773,6 @@
     _cur_satb_drain_time_ms = ms;
   }
 
-  void record_satb_drain_processed_buffers(int processed_buffers) {
-    assert(_g1->mark_in_progress(), "shouldn't be here otherwise");
-    _last_satb_drain_processed_buffers = processed_buffers;
-  }
-
   void record_update_rs_time(int thread, double ms) {
     _par_last_update_rs_times_ms[thread] = ms;
   }