8166607: G1 needs klass_or_null_acquire
authorkbarrett
Tue, 22 Nov 2016 20:24:47 -0500
changeset 42588 d628ae4e12a2
parent 42587 80c66fce4b3b
child 42589 037b433e8069
8166607: G1 needs klass_or_null_acquire Summary: Use acquire where needed, split refinment humongous and non to simplify. Reviewed-by: tschatzl, ehelin
hotspot/src/share/vm/gc/g1/g1BlockOffsetTable.cpp
hotspot/src/share/vm/gc/g1/g1BlockOffsetTable.inline.hpp
hotspot/src/share/vm/gc/g1/g1RemSet.cpp
hotspot/src/share/vm/gc/g1/heapRegion.cpp
hotspot/src/share/vm/gc/g1/heapRegion.hpp
--- a/hotspot/src/share/vm/gc/g1/g1BlockOffsetTable.cpp	Tue Nov 22 13:59:45 2016 -0800
+++ b/hotspot/src/share/vm/gc/g1/g1BlockOffsetTable.cpp	Tue Nov 22 20:24:47 2016 -0500
@@ -227,7 +227,7 @@
     while (n <= next_boundary) {
       q = n;
       oop obj = oop(q);
-      if (obj->klass_or_null() == NULL) return q;
+      if (obj->klass_or_null_acquire() == NULL) return q;
       n += block_size(q);
     }
     assert(q <= next_boundary && n > next_boundary, "Consequence of loop");
--- a/hotspot/src/share/vm/gc/g1/g1BlockOffsetTable.inline.hpp	Tue Nov 22 13:59:45 2016 -0800
+++ b/hotspot/src/share/vm/gc/g1/g1BlockOffsetTable.inline.hpp	Tue Nov 22 20:24:47 2016 -0500
@@ -136,7 +136,7 @@
   while (n <= addr) {
     q = n;
     oop obj = oop(q);
-    if (obj->klass_or_null() == NULL) {
+    if (obj->klass_or_null_acquire() == NULL) {
       return q;
     }
     n += block_size(q);
@@ -148,7 +148,7 @@
 
 inline HeapWord* G1BlockOffsetTablePart::forward_to_block_containing_addr(HeapWord* q,
                                                                           const void* addr) {
-  if (oop(q)->klass_or_null() == NULL) {
+  if (oop(q)->klass_or_null_acquire() == NULL) {
     return q;
   }
   HeapWord* n = q + block_size(q);
--- a/hotspot/src/share/vm/gc/g1/g1RemSet.cpp	Tue Nov 22 13:59:45 2016 -0800
+++ b/hotspot/src/share/vm/gc/g1/g1RemSet.cpp	Tue Nov 22 20:24:47 2016 -0500
@@ -675,9 +675,10 @@
                                         card_ptr);
 
   // If unable to process the card then we encountered an unparsable
-  // part of the heap (e.g. a partially allocated object).  Redirty
-  // and re-enqueue: if we put off the card until a GC pause, then the
-  // allocation will have completed.
+  // part of the heap (e.g. a partially allocated object) while
+  // processing a stale card.  Despite the card being stale, redirty
+  // and re-enqueue, because we've already cleaned the card.  Without
+  // this we could incorrectly discard a non-stale card.
   if (!card_processed) {
     assert(!_g1->is_gc_active(), "Unparsable heap during GC");
     // The card might have gotten re-dirtied and re-enqueued while we
--- a/hotspot/src/share/vm/gc/g1/heapRegion.cpp	Tue Nov 22 13:59:45 2016 -0800
+++ b/hotspot/src/share/vm/gc/g1/heapRegion.cpp	Tue Nov 22 20:24:47 2016 -0500
@@ -352,6 +352,49 @@
   _prev_marked_bytes = marked_bytes;
 }
 
+// Humongous objects are allocated directly in the old-gen.  Need
+// special handling for concurrent processing encountering an
+// in-progress allocation.
+static bool do_oops_on_card_in_humongous(MemRegion mr,
+                                         FilterOutOfRegionClosure* cl,
+                                         HeapRegion* hr,
+                                         G1CollectedHeap* g1h) {
+  assert(hr->is_humongous(), "precondition");
+  HeapRegion* sr = hr->humongous_start_region();
+  oop obj = oop(sr->bottom());
+
+  // If concurrent and klass_or_null is NULL, then space has been
+  // allocated but the object has not yet been published by setting
+  // the klass.  That can only happen if the card is stale.  However,
+  // we've already set the card clean, so we must return failure,
+  // since the allocating thread could have performed a write to the
+  // card that might be missed otherwise.
+  if (!g1h->is_gc_active() && (obj->klass_or_null_acquire() == NULL)) {
+    return false;
+  }
+
+  // Only filler objects follow a humongous object in the containing
+  // regions, and we can ignore those.  So only process the one
+  // humongous object.
+  if (!g1h->is_obj_dead(obj, sr)) {
+    if (obj->is_objArray() || (sr->bottom() < mr.start())) {
+      // objArrays are always marked precisely, so limit processing
+      // with mr.  Non-objArrays might be precisely marked, and since
+      // it's humongous it's worthwhile avoiding full processing.
+      // However, the card could be stale and only cover filler
+      // objects.  That should be rare, so not worth checking for;
+      // instead let it fall out from the bounded iteration.
+      obj->oop_iterate(cl, mr);
+    } else {
+      // If obj is not an objArray and mr contains the start of the
+      // obj, then this could be an imprecise mark, and we need to
+      // process the entire object.
+      obj->oop_iterate(cl);
+    }
+  }
+  return true;
+}
+
 bool HeapRegion::oops_on_card_seq_iterate_careful(MemRegion mr,
                                                   FilterOutOfRegionClosure* cl,
                                                   jbyte* card_ptr) {
@@ -369,7 +412,6 @@
   if (mr.is_empty()) {
     return true;
   }
-  // Otherwise, find the obj that extends onto mr.start().
 
   // The intersection of the incoming mr (for the card) and the
   // allocated part of the region is non-empty. This implies that
@@ -387,54 +429,52 @@
   // We must complete this write before we do any of the reads below.
   OrderAccess::storeload();
 
+  // Special handling for humongous regions.
+  if (is_humongous()) {
+    return do_oops_on_card_in_humongous(mr, cl, this, g1h);
+  }
+
+  // During GC we limit mr by scan_top. So we never get here with an
+  // mr covering objects allocated during GC.  Non-humongous objects
+  // are only allocated in the old-gen during GC.  So the parts of the
+  // heap that may be examined here are always parsable; there's no
+  // need to use klass_or_null here to detect in-progress allocations.
+
   // Cache the boundaries of the memory region in some const locals
   HeapWord* const start = mr.start();
   HeapWord* const end = mr.end();
 
-  // Update BOT as needed while finding start of (potential) object.
+  // Find the obj that extends onto mr.start().
+  // Update BOT as needed while finding start of (possibly dead)
+  // object containing the start of the region.
   HeapWord* cur = block_start(start);
-  assert(cur <= start, "Postcondition");
-
-  oop obj;
 
-  HeapWord* next = cur;
-  do {
-    cur = next;
-    obj = oop(cur);
-    if (obj->klass_or_null() == NULL) {
-      // Ran into an unparseable point.
-      assert(!g1h->is_gc_active(),
-             "Unparsable heap during GC at " PTR_FORMAT, p2i(cur));
-      return false;
-    }
-    // Otherwise...
-    next = cur + block_size(cur);
-  } while (next <= start);
-
-  // If we finish the above loop...We have a parseable object that
-  // begins on or before the start of the memory region, and ends
-  // inside or spans the entire region.
-  assert(cur <= start, "Loop postcondition");
-  assert(obj->klass_or_null() != NULL, "Loop postcondition");
+#ifdef ASSERT
+  {
+    assert(cur <= start,
+           "cur: " PTR_FORMAT ", start: " PTR_FORMAT, p2i(cur), p2i(start));
+    HeapWord* next = cur + block_size(cur);
+    assert(start < next,
+           "start: " PTR_FORMAT ", next: " PTR_FORMAT, p2i(start), p2i(next));
+  }
+#endif
 
   do {
-    obj = oop(cur);
-    assert((cur + block_size(cur)) > (HeapWord*)obj, "Loop invariant");
-    if (obj->klass_or_null() == NULL) {
-      // Ran into an unparseable point.
-      assert(!g1h->is_gc_active(),
-             "Unparsable heap during GC at " PTR_FORMAT, p2i(cur));
-      return false;
-    }
+    oop obj = oop(cur);
+    assert(obj->is_oop(true), "Not an oop at " PTR_FORMAT, p2i(cur));
+    assert(obj->klass_or_null() != NULL,
+           "Unparsable heap at " PTR_FORMAT, p2i(cur));
 
-    // Advance the current pointer. "obj" still points to the object to iterate.
-    cur = cur + block_size(cur);
-
-    if (!g1h->is_obj_dead(obj)) {
-      // Non-objArrays are sometimes marked imprecise at the object start. We
-      // always need to iterate over them in full.
-      // We only iterate over object arrays in full if they are completely contained
-      // in the memory region.
+    if (g1h->is_obj_dead(obj, this)) {
+      // Carefully step over dead object.
+      cur += block_size(cur);
+    } else {
+      // Step over live object, and process its references.
+      cur += obj->size();
+      // Non-objArrays are usually marked imprecise at the object
+      // start, in which case we need to iterate over them in full.
+      // objArrays are precisely marked, but can still be iterated
+      // over in full if completely covered.
       if (!obj->is_objArray() || (((HeapWord*)obj) >= start && cur <= end)) {
         obj->oop_iterate(cl);
       } else {
--- a/hotspot/src/share/vm/gc/g1/heapRegion.hpp	Tue Nov 22 13:59:45 2016 -0800
+++ b/hotspot/src/share/vm/gc/g1/heapRegion.hpp	Tue Nov 22 20:24:47 2016 -0500
@@ -658,7 +658,7 @@
   // mr: the memory region covered by the card.
   // card_ptr: if we decide that the card is not young and we iterate
   // over it, we'll clean the card before we start the iteration.
-  // Returns true if card was successfully processed, false if an
+  // Returns true if the card was successfully processed, false if an
   // unparsable part of the heap was encountered, which should only
   // happen when invoked concurrently with the mutator.
   bool oops_on_card_seq_iterate_careful(MemRegion mr,