diff src/share/vm/gc_implementation/g1/g1OopClosures.inline.hpp @ 20335:eec72fa4b108

8040722: G1: Clean up usages of heap_region_containing Reviewed-by: tschatzl, jmasa
author brutisso
date Tue, 19 Aug 2014 12:39:06 +0200
parents a3953c777565
children 7848fc12602b
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1OopClosures.inline.hpp	Tue Apr 01 07:46:51 2014 +0200
+++ b/src/share/vm/gc_implementation/g1/g1OopClosures.inline.hpp	Tue Aug 19 12:39:06 2014 +0200
@@ -130,9 +130,7 @@
   if (!oopDesc::is_null(heap_oop)) {
     oop obj = oopDesc::decode_heap_oop_not_null(heap_oop);
     HeapRegion* hr = _g1h->heap_region_containing((HeapWord*) obj);
-    if (hr != NULL) {
-      _cm->grayRoot(obj, obj->size(), _worker_id, hr);
-    }
+    _cm->grayRoot(obj, obj->size(), _worker_id, hr);
   }
 }
 
@@ -159,57 +157,61 @@
 template <class T>
 inline void G1UpdateRSOrPushRefOopClosure::do_oop_nv(T* p) {
   oop obj = oopDesc::load_decode_heap_oop(p);
+  if (obj == NULL) {
+    return;
+  }
 #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();
+  oopDesc* o = obj.obj();
 #else
-    oopDesc* o = obj;
+  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");
-  }
+  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");
   assert(_from->is_in_reserved(p), "p is not in from");
 
   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 (_from == to) {
+    // Normally this closure should only be called with cross-region references.
+    // But since Java threads are manipulating the references concurrently and we
+    // reload the values things may have changed.
+    return;
+  }
+  // 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 remembered 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);
-      }
+  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().
-      return;
-    }
-
+    // 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