changeset 1613:a03ae377b2e8

6930581: G1: assert(ParallelGCThreads > 1 || n_yielded() == _hrrs->occupied(),"Should have yielded all the .. Summary: During RSet updating, when ParallelGCThreads is zero, references that point into the collection set are added directly the referenced region's RSet. This can cause the sparse table in the RSet to expand. RSet scanning and the "occupied" routine will then operate on different instances of the sparse table causing the assert to trip. This may also cause some cards added post expansion to be missed during RSet scanning. When ParallelGCThreads is non-zero such references are recorded on the "references to be scanned" queue and the card containing the reference is recorded in a dirty card queue for use in the event of an evacuation failure. Employ the parallel code in the serial case to avoid expanding the RSets of regions in the collection set. Reviewed-by: iveresov, ysr, tonyp
author johnc
date Fri, 06 Aug 2010 10:17:21 -0700
parents 0ce1569c90e5
children 5f429ee79634
files src/share/vm/gc_implementation/g1/g1RemSet.cpp src/share/vm/gc_implementation/g1/g1RemSet.hpp src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp src/share/vm/gc_implementation/g1/sparsePRT.cpp
diffstat 4 files changed, 30 insertions(+), 36 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Wed Aug 04 13:03:23 2010 -0400
+++ b/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Fri Aug 06 10:17:21 2010 -0700
@@ -122,7 +122,7 @@
 HRInto_G1RemSet::HRInto_G1RemSet(G1CollectedHeap* g1, CardTableModRefBS* ct_bs)
   : G1RemSet(g1), _ct_bs(ct_bs), _g1p(_g1->g1_policy()),
     _cg1r(g1->concurrent_g1_refine()),
-    _par_traversal_in_progress(false),
+    _traversal_in_progress(false),
     _cset_rs_update_cl(NULL),
     _cards_scanned(NULL), _total_cards_scanned(0)
 {
@@ -484,28 +484,24 @@
   //   is updated immediately.
   DirtyCardQueue into_cset_dcq(&_g1->into_cset_dirty_card_queue_set());
 
-  if (ParallelGCThreads > 0) {
-    // The two flags below were introduced temporarily to serialize
-    // the updating and scanning of remembered sets. There are some
-    // race conditions when these two operations are done in parallel
-    // and they are causing failures. When we resolve said race
-    // conditions, we'll revert back to parallel remembered set
-    // updating and scanning. See CRs 6677707 and 6677708.
-    if (G1UseParallelRSetUpdating || (worker_i == 0)) {
-      updateRS(&into_cset_dcq, worker_i);
-    } else {
-      _g1p->record_update_rs_processed_buffers(worker_i, 0.0);
-      _g1p->record_update_rs_time(worker_i, 0.0);
-    }
-    if (G1UseParallelRSetScanning || (worker_i == 0)) {
-      scanRS(oc, worker_i);
-    } else {
-      _g1p->record_scan_rs_time(worker_i, 0.0);
-    }
+  assert((ParallelGCThreads > 0) || worker_i == 0, "invariant");
+
+  // The two flags below were introduced temporarily to serialize
+  // the updating and scanning of remembered sets. There are some
+  // race conditions when these two operations are done in parallel
+  // and they are causing failures. When we resolve said race
+  // conditions, we'll revert back to parallel remembered set
+  // updating and scanning. See CRs 6677707 and 6677708.
+  if (G1UseParallelRSetUpdating || (worker_i == 0)) {
+    updateRS(&into_cset_dcq, worker_i);
   } else {
-    assert(worker_i == 0, "invariant");
-    updateRS(&into_cset_dcq, 0);
-    scanRS(oc, 0);
+    _g1p->record_update_rs_processed_buffers(worker_i, 0.0);
+    _g1p->record_update_rs_time(worker_i, 0.0);
+  }
+  if (G1UseParallelRSetScanning || (worker_i == 0)) {
+    scanRS(oc, worker_i);
+  } else {
+    _g1p->record_scan_rs_time(worker_i, 0.0);
   }
 
   // We now clear the cached values of _cset_rs_update_cl for this worker
@@ -524,9 +520,9 @@
   DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set();
   dcqs.concatenate_logs();
 
-  assert(!_par_traversal_in_progress, "Invariant between iterations.");
+  assert(!_traversal_in_progress, "Invariant between iterations.");
+  set_traversal(true);
   if (ParallelGCThreads > 0) {
-    set_par_traversal(true);
     _seq_task->set_par_threads((int)n_workers());
   }
   guarantee( _cards_scanned == NULL, "invariant" );
@@ -623,9 +619,7 @@
   // Set all cards back to clean.
   _g1->cleanUpCardTable();
 
-  if (ParallelGCThreads > 0) {
-    set_par_traversal(false);
-  }
+  set_traversal(false);
 
   DirtyCardQueueSet& into_cset_dcqs = _g1->into_cset_dirty_card_queue_set();
   int into_cset_n_buffers = into_cset_dcqs.completed_buffers_num();
@@ -660,7 +654,7 @@
          "all buffers should be freed");
   _g1->into_cset_dirty_card_queue_set().clear_n_completed_buffers();
 
-  assert(!_par_traversal_in_progress, "Invariant between iterations.");
+  assert(!_traversal_in_progress, "Invariant between iterations.");
 }
 
 class UpdateRSObjectClosure: public ObjectClosure {
--- a/src/share/vm/gc_implementation/g1/g1RemSet.hpp	Wed Aug 04 13:03:23 2010 -0400
+++ b/src/share/vm/gc_implementation/g1/g1RemSet.hpp	Fri Aug 06 10:17:21 2010 -0700
@@ -148,10 +148,10 @@
   size_t*             _cards_scanned;
   size_t              _total_cards_scanned;
 
-  // _par_traversal_in_progress is "true" iff a parallel traversal is in
-  // progress.
-  bool _par_traversal_in_progress;
-  void set_par_traversal(bool b) { _par_traversal_in_progress = b; }
+  // _traversal_in_progress is "true" iff a traversal is in progress.
+
+  bool _traversal_in_progress;
+  void set_traversal(bool b) { _traversal_in_progress = b; }
 
   // Used for caching the closure that is responsible for scanning
   // references into the collection set.
--- a/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp	Wed Aug 04 13:03:23 2010 -0400
+++ b/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp	Fri Aug 06 10:17:21 2010 -0700
@@ -62,13 +62,13 @@
   HeapRegion* to = _g1->heap_region_containing(obj);
   // The test below could be optimized by applying a bit op to to and from.
   if (to != NULL && from != NULL && from != to) {
-    // The _par_traversal_in_progress flag is true during the collection pause,
-    // false during the evacuation failure handing. This should avoid a
+    // The _traversal_in_progress flag is true during the collection pause,
+    // false during the evacuation failure handling. This should avoid a
     // potential loop if we were to add the card containing 'p' to the DCQS
     // that's used to regenerate the remembered sets for the collection set,
     // in the event of an evacuation failure, here. The UpdateRSImmediate
     // closure will eventally call this routine.
-    if (_par_traversal_in_progress &&
+    if (_traversal_in_progress &&
         to->in_collection_set() && !self_forwarded(obj)) {
 
       assert(_cset_rs_update_cl[tid] != NULL, "should have been set already");
--- a/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Wed Aug 04 13:03:23 2010 -0400
+++ b/src/share/vm/gc_implementation/g1/sparsePRT.cpp	Fri Aug 06 10:17:21 2010 -0700
@@ -424,7 +424,7 @@
 
 
 SparsePRT::SparsePRT(HeapRegion* hr) :
-  _expanded(false), _next_expanded(NULL)
+  _hr(hr), _expanded(false), _next_expanded(NULL)
 {
   _cur = new RSHashTable(InitialCapacity);
   _next = _cur;