7092412: G1: Some roots not marked during an initial mark that gets an evacuation failure
authorjohnc
Tue, 20 Sep 2011 15:39:17 -0700
changeset 10664 702062c83bd7
parent 10663 3ef855a3329b
child 10665 f9e21413c65a
7092412: G1: Some roots not marked during an initial mark that gets an evacuation failure Summary: As a result of the changes for 7080389, an evacuation failure during an initial mark pause may result in some root objects not being marked. Pass whether the caller is a root scanning closure into the evacuation failure handling code so that the thread that successfully forwards an object to itself also marks the object. Reviewed-by: ysr, brutisso, tonyp
hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp
hotspot/src/share/vm/gc_implementation/g1/g1OopClosures.hpp
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Tue Sep 20 09:59:59 2011 -0400
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Tue Sep 20 15:39:17 2011 -0700
@@ -3946,7 +3946,8 @@
 
 oop
 G1CollectedHeap::handle_evacuation_failure_par(OopsInHeapRegionClosure* cl,
-                                               oop old) {
+                                               oop old,
+                                               bool should_mark_root) {
   assert(obj_in_cs(old),
          err_msg("obj: "PTR_FORMAT" should still be in the CSet",
                  (HeapWord*) old));
@@ -3954,6 +3955,16 @@
   oop forward_ptr = old->forward_to_atomic(old);
   if (forward_ptr == NULL) {
     // Forward-to-self succeeded.
+
+    // should_mark_root will be true when this routine is called
+    // from a root scanning closure during an initial mark pause.
+    // In this case the thread that succeeds in self-forwarding the
+    // object is also responsible for marking the object.
+    if (should_mark_root) {
+      assert(!oopDesc::is_null(old), "shouldn't be");
+      _cm->grayRoot(old);
+    }
+
     if (_evac_failure_closure != cl) {
       MutexLockerEx x(EvacFailureStack_lock, Mutex::_no_safepoint_check_flag);
       assert(!_drain_in_progress,
@@ -4208,7 +4219,8 @@
   }
 }
 
-oop G1ParCopyHelper::copy_to_survivor_space(oop old, bool should_mark_copy) {
+oop G1ParCopyHelper::copy_to_survivor_space(oop old, bool should_mark_root,
+                                                     bool should_mark_copy) {
   size_t    word_sz = old->size();
   HeapRegion* from_region = _g1->heap_region_containing_raw(old);
   // +1 to make the -1 indexes valid...
@@ -4228,7 +4240,7 @@
     // This will either forward-to-self, or detect that someone else has
     // installed a forwarding pointer.
     OopsInHeapRegionClosure* cl = _par_scan_state->evac_failure_closure();
-    return _g1->handle_evacuation_failure_par(cl, old);
+    return _g1->handle_evacuation_failure_par(cl, old, should_mark_root);
   }
 
   // We're going to allocate linearly, so might as well prefetch ahead.
@@ -4330,11 +4342,26 @@
       // we also need to handle marking of roots in the
       // event of an evacuation failure. In the event of an
       // evacuation failure, the object is forwarded to itself
-      // and not copied so let's mark it here.
+      // and not copied. For root-scanning closures, the
+      // object would be marked after a successful self-forward
+      // but an object could be pointed to by both a root and non
+      // root location and be self-forwarded by a non-root-scanning
+      // closure. Therefore we also have to attempt to mark the
+      // self-forwarded root object here.
       if (do_mark_object && obj->forwardee() == obj) {
         mark_object(p);
       }
     } else {
+      // During an initial mark pause, objects that are pointed to
+      // by the roots need to be marked - even in the event of an
+      // evacuation failure. We pass the template parameter
+      // do_mark_object (which is true for root scanning closures
+      // during an initial mark pause) to copy_to_survivor_space
+      // which will pass it on to the evacuation failure handling
+      // code. The thread that successfully self-forwards a root
+      // object to itself is responsible for marking the object.
+      bool should_mark_root = do_mark_object;
+
       // We need to mark the copied object if we're a root scanning
       // closure during an initial mark pause (i.e. do_mark_object
       // will be true), or the object is already marked and we need
@@ -4343,7 +4370,8 @@
                               _during_initial_mark ||
                               (_mark_in_progress && !_g1->is_obj_ill(obj));
 
-      oop copy_oop = copy_to_survivor_space(obj, should_mark_copy);
+      oop copy_oop = copy_to_survivor_space(obj, should_mark_root,
+                                                 should_mark_copy);
       oopDesc::encode_store_heap_oop(p, copy_oop);
     }
     // When scanning the RS, we only care about objs in CS.
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp	Tue Sep 20 09:59:59 2011 -0400
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp	Tue Sep 20 15:39:17 2011 -0700
@@ -822,7 +822,8 @@
   void finalize_for_evac_failure();
 
   // An attempt to evacuate "obj" has failed; take necessary steps.
-  oop handle_evacuation_failure_par(OopsInHeapRegionClosure* cl, oop obj);
+  oop handle_evacuation_failure_par(OopsInHeapRegionClosure* cl, oop obj,
+                                    bool should_mark_root);
   void handle_evacuation_failure_common(oop obj, markOop m);
 
   // Instance of the concurrent mark is_alive closure for embedding
--- a/hotspot/src/share/vm/gc_implementation/g1/g1OopClosures.hpp	Tue Sep 20 09:59:59 2011 -0400
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1OopClosures.hpp	Tue Sep 20 15:39:17 2011 -0700
@@ -105,7 +105,8 @@
   G1ParScanClosure *_scanner;
 protected:
   template <class T> void mark_object(T* p);
-  oop copy_to_survivor_space(oop obj, bool should_mark_copy);
+  oop copy_to_survivor_space(oop obj, bool should_mark_root,
+                                      bool should_mark_copy);
 public:
   G1ParCopyHelper(G1CollectedHeap* g1, G1ParScanThreadState* par_scan_state,
                   G1ParScanClosure *scanner) :