8166663: Simplify oops_on_card_seq_iterate_careful
Summary: Remove unnecessary parameter, change return value.
Reviewed-by: tschatzl, mgerdin
--- a/hotspot/src/share/vm/gc/g1/g1RemSet.cpp Thu Sep 15 18:18:39 2016 +0300
+++ b/hotspot/src/share/vm/gc/g1/g1RemSet.cpp Mon Sep 26 14:38:35 2016 -0400
@@ -668,20 +668,18 @@
// 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
- // (perhaps the unfilled portion of a TLAB.) For now, we'll dirty the
- // card 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
- // the full complexity of the technique used in "regular" precleaning.
- if (stop_point != NULL) {
+ // 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.
+ if (!card_processed) {
+ 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()) {
--- a/hotspot/src/share/vm/gc/g1/heapRegion.cpp Thu Sep 15 18:18:39 2016 +0300
+++ b/hotspot/src/share/vm/gc/g1/heapRegion.cpp Mon Sep 26 14:38:35 2016 -0400
@@ -352,19 +352,10 @@
_prev_marked_bytes = marked_bytes;
}
-HeapWord*
-HeapRegion::
-oops_on_card_seq_iterate_careful(MemRegion mr,
- FilterOutOfRegionClosure* cl,
- 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");
- }
+bool HeapRegion::oops_on_card_seq_iterate_careful(MemRegion mr,
+ FilterOutOfRegionClosure* cl,
+ jbyte* card_ptr) {
+ 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 @@
} 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 @@
// 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) {
- return NULL;
+ if (is_young()) {
+ 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
- // asked to (i.e., card_ptr != NULL).
- if (card_ptr != NULL) {
- *card_ptr = CardTableModRefBS::clean_card_val();
- // We must complete this write before we do any of the reads below.
- OrderAccess::storeload();
- }
+ // the card is not young.
+ *card_ptr = CardTableModRefBS::clean_card_val();
+ // 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
- // to update the BOT while we do this...
+ // Update BOT as needed while finding start of (potential) object.
HeapWord* cur = block_start(start);
assert(cur <= start, "Postcondition");
@@ -416,7 +403,9 @@
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 @@
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 @@
}
} while (cur < end);
- return NULL;
+ return true;
}
// Code roots support
--- a/hotspot/src/share/vm/gc/g1/heapRegion.hpp Thu Sep 15 18:18:39 2016 +0300
+++ b/hotspot/src/share/vm/gc/g1/heapRegion.hpp Mon Sep 26 14:38:35 2016 -0400
@@ -653,16 +653,17 @@
}
}
- // filter_young: if true and the region is a young region then we
- // skip the iteration.
- // card_ptr: if not NULL, and we decide that the card is not young
- // and we iterate over it, we'll clean the card before we start the
- // iteration.
- HeapWord*
- oops_on_card_seq_iterate_careful(MemRegion mr,
- FilterOutOfRegionClosure* cl,
- bool filter_young,
- jbyte* card_ptr);
+ // Iterate over the card in the card designated by card_ptr,
+ // applying cl to all references in the region.
+ // 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
+ // 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,
+ 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; }