diff src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp @ 1960:878b57474103

6978187: G1: assert(ParallelGCThreads> 1 || n_yielded() == _hrrs->occupied()) strikes again Summary: An evacuation failure while copying the roots caused an object, A, to be forwarded to itself. During the subsequent RSet updating a reference to A was processed causing the reference to be added to the RSet of A's heap region. As a result of adding to the remembered set we ran into the issue described in 6930581 - the sparse table expanded and the RSet scanning code walked the cards in one instance of RHashTable (_cur) while the occupied() counts the cards in the expanded table (_next). Reviewed-by: tonyp, iveresov
author johnc
date Tue, 16 Nov 2010 14:07:33 -0800
parents c32059ef4dc0
children f95d63e2154a
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp	Mon Nov 15 16:25:14 2010 -0800
+++ b/src/share/vm/gc_implementation/g1/g1RemSet.inline.hpp	Tue Nov 16 14:07:33 2010 -0800
@@ -31,17 +31,12 @@
 }
 
 template <class T>
-inline void G1RemSet::write_ref_nv(HeapRegion* from, T* p) {
-  par_write_ref_nv(from, p, 0);
-}
-
-inline bool G1RemSet::self_forwarded(oop obj) {
-  bool result =  (obj->is_forwarded() && (obj->forwardee()== obj));
-  return result;
+inline void G1RemSet::write_ref(HeapRegion* from, T* p) {
+  par_write_ref(from, p, 0);
 }
 
 template <class T>
-inline void G1RemSet::par_write_ref_nv(HeapRegion* from, T* p, int tid) {
+inline void G1RemSet::par_write_ref(HeapRegion* from, T* p, int tid) {
   oop obj = oopDesc::load_decode_heap_oop(p);
 #ifdef ASSERT
   // can't do because of races
@@ -62,34 +57,15 @@
   assert(from == NULL || from->is_in_reserved(p), "p is not in from");
 
   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 _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 (_traversal_in_progress &&
-        to->in_collection_set() && !self_forwarded(obj)) {
-
-      assert(_cset_rs_update_cl[tid] != NULL, "should have been set already");
-      _cset_rs_update_cl[tid]->do_oop(p);
-
-      // Deferred updates to the CSet are either discarded (in the normal case),
-      // or processed (if an evacuation failure occurs) at the end
-      // of the collection.
-      // See G1RemSet::cleanup_after_oops_into_collection_set_do().
-    } else {
+  if (to != NULL && from != to) {
 #if G1_REM_SET_LOGGING
-      gclog_or_tty->print_cr("Adding " PTR_FORMAT " (" PTR_FORMAT ") to RS"
-                             " for region [" PTR_FORMAT ", " PTR_FORMAT ")",
-                             p, obj,
-                             to->bottom(), to->end());
+    gclog_or_tty->print_cr("Adding " PTR_FORMAT " (" PTR_FORMAT ") to RS"
+                           " for region [" PTR_FORMAT ", " PTR_FORMAT ")",
+                           p, obj,
+                           to->bottom(), to->end());
 #endif
-      assert(to->rem_set() != NULL, "Need per-region 'into' remsets.");
-      to->rem_set()->add_reference(p, tid);
-    }
+    assert(to->rem_set() != NULL, "Need per-region 'into' remsets.");
+    to->rem_set()->add_reference(p, tid);
   }
 }
 
@@ -108,3 +84,64 @@
   }
 }
 
+template <class T>
+inline void UpdateRSOrPushRefOopClosure::do_oop_work(T* p) {
+  oop obj = oopDesc::load_decode_heap_oop(p);
+#ifdef ASSERT
+  // can't do because of races
+  // assert(obj == NULL || obj->is_oop(), "expected an oop");
+
+  // Do the safe subset of is_oop
+  if (obj != NULL) {
+#ifdef CHECK_UNHANDLED_OOPS
+    oopDesc* o = obj.obj();
+#else
+    oopDesc* o = obj;
+#endif // CHECK_UNHANDLED_OOPS
+    assert((intptr_t)o % MinObjAlignmentInBytes == 0, "not oop aligned");
+    assert(Universe::heap()->is_in_reserved(obj), "must be in heap");
+  }
+#endif // ASSERT
+
+  assert(_from != NULL, "from region must be non-NULL");
+
+  HeapRegion* to = _g1->heap_region_containing(obj);
+  if (to != NULL && _from != to) {
+    // The _record_refs_into_cset flag is true during the RSet
+    // updating part of an evacuation pause. It is false at all
+    // other times:
+    //  * rebuilding the rembered sets after a full GC
+    //  * during concurrent refinement.
+    //  * updating the remembered sets of regions in the collection
+    //    set in the event of an evacuation failure (when deferred
+    //    updates are enabled).
+
+    if (_record_refs_into_cset && to->in_collection_set()) {
+      // We are recording references that point into the collection
+      // set and this particular reference does exactly that...
+      // If the referenced object has already been forwarded
+      // to itself, we are handling an evacuation failure and
+      // we have already visited/tried to copy this object
+      // there is no need to retry.
+      if (!self_forwarded(obj)) {
+        assert(_push_ref_cl != NULL, "should not be null");
+        // Push the reference in the refs queue of the G1ParScanThreadState
+        // instance for this worker thread.
+        _push_ref_cl->do_oop(p);
+      }
+
+      // Deferred updates to the CSet are either discarded (in the normal case),
+      // or processed (if an evacuation failure occurs) at the end
+      // of the collection.
+      // See G1RemSet::cleanup_after_oops_into_collection_set_do().
+    } else {
+      // We either don't care about pushing references that point into the
+      // collection set (i.e. we're not during an evacuation pause) _or_
+      // the reference doesn't point into the collection set. Either way
+      // we add the reference directly to the RSet of the region containing
+      // the referenced object.
+      _g1_rem_set->par_write_ref(_from, p, _worker_i);
+    }
+  }
+}
+