8166663: Simplify oops_on_card_seq_iterate_careful

Remove unnecessary parameter, change return value.

Reviewed-by: tschatzl, mgerdin

hotspot/src/share/vm/gc/g1/g1RemSet.cpp

@@ -668,20 +668,18 @@ bool G1RemSet::refine_card(jbyte* card_ptr,
   // fail arbitrarily). We tell the iteration code to perform this
   // filtering when it has been determined that there has been an actual
   // allocation in this region and making it safe to check the young type.
-  bool filter_young = true;
 
-  HeapWord* stop_point =
+  bool card_processed =
     r->oops_on_card_seq_iterate_careful(dirtyRegion,
                                         &filter_then_update_rs_oop_cl,
-                                        filter_young,
                                         card_ptr);
 
-  // If stop_point is non-null, then we encountered an unallocated region
+  // If unable to process the card then we encountered an unparsable
-  // (perhaps the unfilled portion of a TLAB.)  For now, we'll dirty the
+  // part of the heap (e.g. a partially allocated object).  Redirty
-  // card and re-enqueue: if we put off the card until a GC pause, then the
+  // and re-enqueue: if we put off the card until a GC pause, then the
-  // unallocated portion will be filled in.  Alternatively, we might try
+  // allocation will have completed.
-  // the full complexity of the technique used in "regular" precleaning.
+  if (!card_processed) {
-  if (stop_point != NULL) {
+    assert(!_g1->is_gc_active(), "Unparsable heap during GC");
     // The card might have gotten re-dirtied and re-enqueued while we
     // worked.  (In fact, it's pretty likely.)
     if (*card_ptr != CardTableModRefBS::dirty_card_val()) {

hotspot/src/share/vm/gc/g1/heapRegion.cpp

@@ -352,19 +352,10 @@ void HeapRegion::note_self_forwarding_removal_end(bool during_initial_mark,
   _prev_marked_bytes = marked_bytes;
 }
 
-HeapWord*
+bool HeapRegion::oops_on_card_seq_iterate_careful(MemRegion mr,
-HeapRegion::
+                                                  FilterOutOfRegionClosure* cl,
-oops_on_card_seq_iterate_careful(MemRegion mr,
+                                                  jbyte* card_ptr) {
-                                 FilterOutOfRegionClosure* cl,
+  assert(card_ptr != NULL, "pre-condition");
-                                 bool filter_young,
-                                 jbyte* card_ptr) {
-  // Currently, we should only have to clean the card if filter_young
-  // is true and vice versa.
-  if (filter_young) {
-    assert(card_ptr != NULL, "pre-condition");
-  } else {
-    assert(card_ptr == NULL, "pre-condition");
-  }
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
 
   // If we're within a stop-world GC, then we might look at a card in a
@@ -375,7 +366,9 @@ oops_on_card_seq_iterate_careful(MemRegion mr,
   } else {
     mr = mr.intersection(used_region());
   }
-  if (mr.is_empty()) return NULL;
+  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
@@ -384,27 +377,21 @@ oops_on_card_seq_iterate_careful(MemRegion mr,
   // G1CollectedHeap.cpp that allocates a new region sets the
   // is_young tag on the region before allocating. Thus we
   // safely know if this region is young.
-  if (is_young() && filter_young) {
+  if (is_young()) {
-    return NULL;
+    return true;
   }
 
-  assert(!is_young(), "check value of filter_young");
-
   // We can only clean the card here, after we make the decision that
-  // the card is not young. And we only clean the card if we have been
+  // the card is not young.
-  // asked to (i.e., card_ptr != NULL).
+  *card_ptr = CardTableModRefBS::clean_card_val();
-  if (card_ptr != NULL) {
+  // We must complete this write before we do any of the reads below.
-    *card_ptr = CardTableModRefBS::clean_card_val();
+  OrderAccess::storeload();
-    // We must complete this write before we do any of the reads below.
-    OrderAccess::storeload();
-  }
 
   // Cache the boundaries of the memory region in some const locals
   HeapWord* const start = mr.start();
   HeapWord* const end = mr.end();
 
-  // We used to use "block_start_careful" here.  But we're actually happy
+  // Update BOT as needed while finding start of (potential) object.
-  // to update the BOT while we do this...
   HeapWord* cur = block_start(start);
   assert(cur <= start, "Postcondition");
 
@@ -416,7 +403,9 @@ oops_on_card_seq_iterate_careful(MemRegion mr,
     obj = oop(cur);
     if (obj->klass_or_null() == NULL) {
       // Ran into an unparseable point.
-      return cur;
+      assert(!g1h->is_gc_active(),
+             "Unparsable heap during GC at " PTR_FORMAT, p2i(cur));
+      return false;
     }
     // Otherwise...
     next = cur + block_size(cur);
@@ -433,7 +422,9 @@ oops_on_card_seq_iterate_careful(MemRegion mr,
     assert((cur + block_size(cur)) > (HeapWord*)obj, "Loop invariant");
     if (obj->klass_or_null() == NULL) {
       // Ran into an unparseable point.
-      return cur;
+      assert(!g1h->is_gc_active(),
+             "Unparsable heap during GC at " PTR_FORMAT, p2i(cur));
+      return false;
     }
 
     // Advance the current pointer. "obj" still points to the object to iterate.
@@ -452,7 +443,7 @@ oops_on_card_seq_iterate_careful(MemRegion mr,
     }
   } while (cur < end);
 
-  return NULL;
+  return true;
 }
 
 // Code roots support

hotspot/src/share/vm/gc/g1/heapRegion.hpp

@@ -653,16 +653,17 @@ class HeapRegion: public G1ContiguousSpace {
     }
   }
 
-  // filter_young: if true and the region is a young region then we
+  // Iterate over the card in the card designated by card_ptr,
-  // skip the iteration.
+  // applying cl to all references in the region.
-  // card_ptr: if not NULL, and we decide that the card is not young
+  // mr: the memory region covered by the card.
-  // and we iterate over it, we'll clean the card before we start the
+  // card_ptr: if we decide that the card is not young and we iterate
-  // iteration.
+  // over it, we'll clean the card before we start the iteration.
-  HeapWord*
+  // Returns true if card was successfully processed, false if an
-  oops_on_card_seq_iterate_careful(MemRegion mr,
+  // unparsable part of the heap was encountered, which should only
-                                   FilterOutOfRegionClosure* cl,
+  // happen when invoked concurrently with the mutator.
-                                   bool filter_young,
+  bool oops_on_card_seq_iterate_careful(MemRegion mr,
-                                   jbyte* card_ptr);
+                                        FilterOutOfRegionClosure* cl,
+                                        jbyte* card_ptr);
 
   size_t recorded_rs_length() const        { return _recorded_rs_length; }
   double predicted_elapsed_time_ms() const { return _predicted_elapsed_time_ms; }