jdk-sandbox: comparison src/hotspot/share/gc/g1/g1RemSet.cpp

equal deleted inserted replaced

-:d4ddf19c2624
+:bd9dba789919
 p2i(ct->addr_for(card_ptr)),
 g1h->addr_to_region(ct->addr_for(card_ptr)));
 #endif
 }
-void G1RemSet::refine_card_concurrently(CardValue* card_ptr,
+bool G1RemSet::clean_card_before_refine(CardValue** const card_ptr_addr) {
-uint worker_id) {
 assert(!_g1h->is_gc_active(), "Only call concurrently");
-// Construct the region representing the card.
+CardValue* card_ptr = *card_ptr_addr;
+// Find the start address represented by the card.
 HeapWord* start = _ct->addr_for(card_ptr);
 // And find the region containing it.
 HeapRegion* r = _g1h->heap_region_containing_or_null(start);
 // If this is a (stale) card into an uncommitted region, exit.
 if (r == NULL) {
-return;
+return false;
 }
 check_card_ptr(card_ptr, _ct);
 // If the card is no longer dirty, nothing to do.
+// We cannot load the card value before the "r == NULL" check, because G1
+// could uncommit parts of the card table covering uncommitted regions.
 if (*card_ptr != G1CardTable::dirty_card_val()) {
-return;
+return false;
 }
 // This check is needed for some uncommon cases where we should
 // ignore the card.
 //
 //
 // In the normal (non-stale) case, the synchronization between the
 // enqueueing of the card and processing it here will have ensured
 // we see the up-to-date region type here.
 if (!r->is_old_or_humongous_or_archive()) {
-return;
+return false;
 }
 // The result from the hot card cache insert call is either:
 //   * pointer to the current card
 //     (implying that the current card is not 'hot'),
 const CardValue* orig_card_ptr = card_ptr;
 card_ptr = _hot_card_cache->insert(card_ptr);
 if (card_ptr == NULL) {
 // There was no eviction. Nothing to do.
-return;
+return false;
 } else if (card_ptr != orig_card_ptr) {
 // Original card was inserted and an old card was evicted.
 start = _ct->addr_for(card_ptr);
 r = _g1h->heap_region_containing(start);
 // Check whether the region formerly in the cache should be
 // ignored, as discussed earlier for the original card.  The
 // region could have been freed while in the cache.
 if (!r->is_old_or_humongous_or_archive()) {
-return;
+return false;
 }
+*card_ptr_addr = card_ptr;
 } // Else we still have the original card.
 }
 // Trim the region designated by the card to what's been allocated
 // in the region.  The card could be stale, or the card could cover
 // (part of) an object at the end of the allocated space and extend
 // beyond the end of allocation.
-// Non-humongous objects are only allocated in the old-gen during
+// Non-humongous objects are either allocated in the old regions during GC,
-// GC, so if region is old then top is stable.  Humongous object
+// or mapped in archive regions during startup. So if region is old or
-// allocation sets top last; if top has not yet been set, this is
+// archive then top is stable.
-// a stale card and we'll end up with an empty intersection.  If
+// Humongous object allocation sets top last; if top has not yet been set,
-// this is not a stale card, the synchronization between the
+// this is a stale card and we'll end up with an empty intersection.
+// If this is not a stale card, the synchronization between the
 // enqueuing of the card and processing it here will have ensured
 // we see the up-to-date top here.
 HeapWord* scan_limit = r->top();
 if (scan_limit <= start) {
 // If the trimmed region is empty, the card must be stale.
-return;
+return false;
 }
 // Okay to clean and process the card now.  There are still some
 // stale card cases that may be detected by iteration and dealt with
 // as iteration failure.
 *const_cast<volatile CardValue*>(card_ptr) = G1CardTable::clean_card_val();
-// This fence serves two purposes.  First, the card must be cleaned
+return true;
-// before processing the contents.  Second, we can't proceed with
+}
-// processing until after the read of top, for synchronization with
-// possibly concurrent humongous object allocation.  It's okay that
+void G1RemSet::refine_card_concurrently(CardValue* const card_ptr,
-// reading top and reading type were racy wrto each other.  We need
+const uint worker_id) {
-// both set, in any order, to proceed.
+assert(!_g1h->is_gc_active(), "Only call concurrently");
-OrderAccess::fence();
+check_card_ptr(card_ptr, _ct);
+// Construct the MemRegion representing the card.
+HeapWord* start = _ct->addr_for(card_ptr);
+// And find the region containing it.
+HeapRegion* r = _g1h->heap_region_containing(start);
+// This reload of the top is safe even though it happens after the full
+// fence, because top is stable for old, archive and unfiltered humongous
+// regions, so it must return the same value as the previous load when
+// cleaning the card. Also cleaning the card and refinement of the card
+// cannot span across safepoint, so we don't need to worry about top being
+// changed during safepoint.
+HeapWord* scan_limit = r->top();
+assert(scan_limit > start, "sanity");
 // Don't use addr_for(card_ptr + 1) which can ask for
 // a card beyond the heap.
 HeapWord* end = start + G1CardTable::card_size_in_words;
 MemRegion dirty_region(start, MIN2(scan_limit, end));

changeset 59233	bd9dba789919
parent 58652	9b67dd88a931
child 59249	29b0d0b61615