7199349: NPG: PS: Crash seen in jprt

Reviewed-by: johnc

hotspot/src/share/vm/gc_implementation/parallelScavenge/cardTableExtension.cpp

@@ -121,103 +121,12 @@ class CheckForPreciseMarks : public OopClosure {
 
 // We get passed the space_top value to prevent us from traversing into
 // the old_gen promotion labs, which cannot be safely parsed.
-void CardTableExtension::scavenge_contents(ObjectStartArray* start_array,
-                                           MutableSpace* sp,
-                                           HeapWord* space_top,
-                                           PSPromotionManager* pm)
-{
-  assert(start_array != NULL && sp != NULL && pm != NULL, "Sanity");
-  assert(start_array->covered_region().contains(sp->used_region()),
-         "ObjectStartArray does not cover space");
 
-  if (sp->not_empty()) {
-    oop* sp_top = (oop*)space_top;
-    oop* prev_top = NULL;
-    jbyte* current_card = byte_for(sp->bottom());
-    jbyte* end_card     = byte_for(sp_top - 1);    // sp_top is exclusive
-    // scan card marking array
-    while (current_card <= end_card) {
-      jbyte value = *current_card;
-      // skip clean cards
-      if (card_is_clean(value)) {
-        current_card++;
-      } else {
-        // we found a non-clean card
-        jbyte* first_nonclean_card = current_card++;
-        oop* bottom = (oop*)addr_for(first_nonclean_card);
-        // find object starting on card
-        oop* bottom_obj = (oop*)start_array->object_start((HeapWord*)bottom);
-        // bottom_obj = (oop*)start_array->object_start((HeapWord*)bottom);
-        assert(bottom_obj <= bottom, "just checking");
-        // make sure we don't scan oops we already looked at
-        if (bottom < prev_top) bottom = prev_top;
-        // figure out when to stop scanning
-        jbyte* first_clean_card;
-        oop* top;
-        bool restart_scanning;
-        do {
-          restart_scanning = false;
-          // find a clean card
-          while (current_card <= end_card) {
-            value = *current_card;
-            if (card_is_clean(value)) break;
-            current_card++;
-          }
-          // check if we reached the end, if so we are done
-          if (current_card >= end_card) {
-            first_clean_card = end_card + 1;
-            current_card++;
-            top = sp_top;
-          } else {
-            // we have a clean card, find object starting on that card
-            first_clean_card = current_card++;
-            top = (oop*)addr_for(first_clean_card);
-            oop* top_obj = (oop*)start_array->object_start((HeapWord*)top);
-            // top_obj = (oop*)start_array->object_start((HeapWord*)top);
-            assert(top_obj <= top, "just checking");
-            if (oop(top_obj)->is_objArray() || oop(top_obj)->is_typeArray()) {
-              // an arrayOop is starting on the clean card - since we do exact store
-              // checks for objArrays we are done
-            } else {
-              // otherwise, it is possible that the object starting on the clean card
-              // spans the entire card, and that the store happened on a later card.
-              // figure out where the object ends
-              top = top_obj + oop(top_obj)->size();
-              jbyte* top_card = CardTableModRefBS::byte_for(top - 1);   // top is exclusive
-              if (top_card > first_clean_card) {
-                // object ends a different card
-                current_card = top_card + 1;
-                if (card_is_clean(*top_card)) {
-                  // the ending card is clean, we are done
-                  first_clean_card = top_card;
-                } else {
-                  // the ending card is not clean, continue scanning at start of do-while
-                  restart_scanning = true;
-                }
-              } else {
-                // object ends on the clean card, we are done.
-                assert(first_clean_card == top_card, "just checking");
-              }
-            }
-          }
-        } while (restart_scanning);
-        // we know which cards to scan, now clear them
-        while (first_nonclean_card < first_clean_card) {
-          *first_nonclean_card++ = clean_card;
-        }
-        // scan oops in objects
-        do {
-          oop(bottom_obj)->push_contents(pm);
-          bottom_obj += oop(bottom_obj)->size();
-          assert(bottom_obj <= sp_top, "just checking");
-        } while (bottom_obj < top);
-        pm->drain_stacks_cond_depth();
-        // remember top oop* scanned
-        prev_top = top;
-      }
-    }
-  }
-}
+// Do not call this method if the space is empty.
+// It is a waste to start tasks and get here only to
+// do no work.  If this method needs to be called
+// when the space is empty, fix the calculation of
+// end_card to allow sp_top == sp->bottom().
 
 void CardTableExtension::scavenge_contents_parallel(ObjectStartArray* start_array,
                                                     MutableSpace* sp,
@@ -228,10 +137,11 @@ void CardTableExtension::scavenge_contents_parallel(ObjectStartArray* start_arra
   int ssize = 128; // Naked constant!  Work unit = 64k.
   int dirty_card_count = 0;
 
+  // It is a waste to get here if empty.
+  assert(sp->bottom() < sp->top(), "Should not be called if empty");
   oop* sp_top = (oop*)space_top;
-  oop* sp_last = sp->bottom() == space_top ? sp_top : sp_top - 1;
   jbyte* start_card = byte_for(sp->bottom());
-  jbyte* end_card   = byte_for(sp_last) + 1;
+  jbyte* end_card   = byte_for(sp_top - 1) + 1;
   oop* last_scanned = NULL; // Prevent scanning objects more than once
   // The width of the stripe ssize*stripe_total must be
   // consistent with the number of stripes so that the complete slice
@@ -255,6 +165,16 @@ void CardTableExtension::scavenge_contents_parallel(ObjectStartArray* start_arra
     HeapWord* slice_start = addr_for(worker_start_card);
     HeapWord* slice_end = MIN2((HeapWord*) sp_top, addr_for(worker_end_card));
 
+#ifdef ASSERT
+    if (GCWorkerDelayMillis > 0) {
+      // Delay 1 worker so that it proceeds after all the work
+      // has been completed.
+      if (stripe_number < 2) {
+        os::sleep(Thread::current(), GCWorkerDelayMillis, false);
+      }
+    }
+#endif
+
     // If there are not objects starting within the chunk, skip it.
     if (!start_array->object_starts_in_range(slice_start, slice_end)) {
       continue;

hotspot/src/share/vm/gc_implementation/parallelScavenge/cardTableExtension.hpp

@@ -60,11 +60,6 @@ class CardTableExtension : public CardTableModRefBS {
   // BarrierSet::Name kind() { return BarrierSet::CardTableExtension; }
 
   // Scavenge support
-  void scavenge_contents(ObjectStartArray* start_array,
-                         MutableSpace* sp,
-                         HeapWord* space_top,
-                         PSPromotionManager* pm);
-
   void scavenge_contents_parallel(ObjectStartArray* start_array,
                                   MutableSpace* sp,
                                   HeapWord* space_top,

hotspot/src/share/vm/gc_implementation/parallelScavenge/psPromotionManager.inline.hpp

@@ -136,6 +136,13 @@ oop PSPromotionManager::copy_to_survivor_space(oop o) {
 
             HeapWord* lab_base = old_gen()->cas_allocate(OldPLABSize);
             if(lab_base != NULL) {
+#ifdef ASSERT
+              // Delay the initialization of the promotion lab (plab).
+              // This exposes uninitialized plabs to card table processing.
+              if (GCWorkerDelayMillis > 0) {
+                os::sleep(Thread::current(), GCWorkerDelayMillis, false);
+              }
+#endif
               _old_lab.initialize(MemRegion(lab_base, OldPLABSize));
               // Try the old lab allocation again.
               new_obj = (oop) _old_lab.allocate(new_obj_size);

hotspot/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp

@@ -395,9 +395,13 @@ bool PSScavenge::invoke_no_policy() {
 
       GCTaskQueue* q = GCTaskQueue::create();
 
-      uint stripe_total = active_workers;
-      for(uint i=0; i < stripe_total; i++) {
-        q->enqueue(new OldToYoungRootsTask(old_gen, old_top, i, stripe_total));
+      if (!old_gen->object_space()->is_empty()) {
+        // There are only old-to-young pointers if there are objects
+        // in the old gen.
+        uint stripe_total = active_workers;
+        for(uint i=0; i < stripe_total; i++) {
+          q->enqueue(new OldToYoungRootsTask(old_gen, old_top, i, stripe_total));
+        }
       }
 
       q->enqueue(new ScavengeRootsTask(ScavengeRootsTask::universe));

hotspot/src/share/vm/gc_implementation/parallelScavenge/psTasks.cpp

@@ -164,36 +164,14 @@ void StealTask::do_it(GCTaskManager* manager, uint which) {
   guarantee(pm->stacks_empty(), "stacks should be empty at this point");
 }
 
-//
-// SerialOldToYoungRootsTask
-//
-
-void SerialOldToYoungRootsTask::do_it(GCTaskManager* manager, uint which) {
-  assert(_gen != NULL, "Sanity");
-  assert(_gen->object_space()->contains(_gen_top) || _gen_top == _gen->object_space()->top(), "Sanity");
-
-  {
-    PSPromotionManager* pm = PSPromotionManager::gc_thread_promotion_manager(which);
-
-    assert(Universe::heap()->kind() == CollectedHeap::ParallelScavengeHeap, "Sanity");
-    CardTableExtension* card_table = (CardTableExtension *)Universe::heap()->barrier_set();
-    // FIX ME! Assert that card_table is the type we believe it to be.
-
-    card_table->scavenge_contents(_gen->start_array(),
-                                  _gen->object_space(),
-                                  _gen_top,
-                                  pm);
-
-    // Do the real work
-    pm->drain_stacks(false);
-  }
-}
-
 //
 // OldToYoungRootsTask
 //
 
 void OldToYoungRootsTask::do_it(GCTaskManager* manager, uint which) {
+  // There are not old-to-young pointers if the old gen is empty.
+  assert(!_gen->object_space()->is_empty(),
+    "Should not be called is there is no work");
   assert(_gen != NULL, "Sanity");
   assert(_gen->object_space()->contains(_gen_top) || _gen_top == _gen->object_space()->top(), "Sanity");
   assert(_stripe_number < ParallelGCThreads, "Sanity");

hotspot/src/share/vm/gc_implementation/parallelScavenge/psTasks.hpp

@@ -112,25 +112,6 @@ class StealTask : public GCTask {
   virtual void do_it(GCTaskManager* manager, uint which);
 };
 
-//
-// SerialOldToYoungRootsTask
-//
-// This task is used to scan for roots in the perm gen
-
-class SerialOldToYoungRootsTask : public GCTask {
- private:
-  PSOldGen* _gen;
-  HeapWord* _gen_top;
-
- public:
-  SerialOldToYoungRootsTask(PSOldGen *gen, HeapWord* gen_top) :
-    _gen(gen), _gen_top(gen_top) { }
-
-  char* name() { return (char *)"serial-old-to-young-roots-task"; }
-
-  virtual void do_it(GCTaskManager* manager, uint which);
-};
-
 //
 // OldToYoungRootsTask
 //

hotspot/src/share/vm/runtime/globals.hpp

@@ -3065,6 +3065,9 @@ class CommandLineFlags {
   develop(uintx, GCExpandToAllocateDelayMillis, 0,                          \
           "Delay in ms between expansion and allocation")                   \
                                                                             \
+  develop(uintx, GCWorkerDelayMillis, 0,                                    \
+          "Delay in ms in scheduling GC workers")                           \
+                                                                            \
   product(intx, DeferThrSuspendLoopCount,     4000,                         \
           "(Unstable) Number of times to iterate in safepoint loop "        \
           " before blocking VM threads ")                                   \