RFR(S/M): 8001985: G1: Backport fix for 7200261 to hsx24

[John Cuthbertson]

Hi Everyone,

Can I have a couple of volunteers review the changes for this CR? The 
webrev can be found at: http://cr.openjdk.java.net/~johnc/8001985/webrev.0/

Summary:
A patch containing the changes for 7200261, generated by hg export, did 
not import cleanly into hsx24 sources - the following two hunks in 
concurrentMark.cpp:

@@ -1188,29 +1188,14 @@
  // liveness counting data.
  class CMCountDataClosureBase: public HeapRegionClosure {
  protected:
+  G1CollectedHeap* _g1h;
    ConcurrentMark* _cm;
+  CardTableModRefBS* _ct_bs;
+
    BitMap* _region_bm;
    BitMap* _card_bm;

-  void set_card_bitmap_range(BitMap::idx_t start_idx, BitMap::idx_t 
last_idx) {
-    assert(start_idx <= last_idx, "sanity");
-
-    // Set the inclusive bit range [start_idx, last_idx].
-    // For small ranges (up to 8 cards) use a simple loop; otherwise
-    // use par_at_put_range.
-    if ((last_idx - start_idx) < 8) {
-      for (BitMap::idx_t i = start_idx; i <= last_idx; i += 1) {
-        _card_bm->par_set_bit(i);
-      }
-    } else {
-      assert(last_idx < _card_bm->size(), "sanity");
-      // Note BitMap::par_at_put_range() is exclusive.
-      BitMap::idx_t max_idx = MAX2(last_idx+1, _card_bm->size());
-      _card_bm->par_at_put_range(start_idx, max_idx, true);
-    }
-  }
-
-  // It takes a region that's not empty (i.e., it has at least one
+  // Takes a region that's not empty (i.e., it has at least one
    // live object in it and sets its corresponding bit on the region
    // bitmap to 1. If the region is "starts humongous" it will also set
    // to 1 the bits on the region bitmap that correspond to its

@@ -1548,24 +1555,29 @@
      if (ntams < top) {
        // This definitely means the region has live objects.
        set_bit_for_region(hr);
-    }
-
-    // Now set the bits for [ntams, top]
-    BitMap::idx_t start_idx = _cm->card_bitmap_index_for(ntams);
-    // set_card_bitmap_range() expects the last_idx to be with
-    // the range of the bit map (see assertion in set_card_bitmap_range()),
-    // so limit it to that range with this application of MIN2.
-    BitMap::idx_t last_idx = MIN2(_cm->card_bitmap_index_for(top),
-                                  _card_bm->size()-1);
-    if (start_idx < _card_bm->size()) {
-    set_card_bitmap_range(start_idx, last_idx);
-    } else {
-      // To reach here start_idx must be beyond the end of
-      // the bit map and last_idx must have been limited by
-      // the MIN2().
-      assert(start_idx == last_idx + 1,
-        err_msg("Not beyond end start_idx " SIZE_FORMAT " last_idx "
-                SIZE_FORMAT, start_idx, last_idx));
+
+      // Now set the bits in the card bitmap for [ntams, top)
+      BitMap::idx_t start_idx = _cm->card_bitmap_index_for(ntams);
+      BitMap::idx_t end_idx = _cm->card_bitmap_index_for(top);
+
+      // Note: if we're looking at the last region in heap - top
+      // could be actually just beyond the end of the heap; end_idx
+      // will then correspond to a (non-existent) card that is also
+      // just beyond the heap.
+      if (_g1h->is_in_g1_reserved(top) && !_ct_bs->is_card_aligned(top)) {
+        // end of object is not card aligned - increment to cover
+        // all the cards spanned by the object
+        end_idx += 1;
+      }
+
+      assert(end_idx <= _card_bm->size(),
+             err_msg("oob: end_idx=  "SIZE_FORMAT", bitmap size= 
"SIZE_FORMAT,
+                     end_idx, _card_bm->size()));
+      assert(start_idx < _card_bm->size(),
+             err_msg("oob: start_idx=  "SIZE_FORMAT", bitmap size= 
"SIZE_FORMAT,
+                     start_idx, _card_bm->size()));
+
+      _cm->set_card_bitmap_range(_card_bm, start_idx, end_idx, true /* 
is_par */);
      }

      // Set the bit for the region if it contains live data

did not apply cleanly. These had to be fixed up by hand. The original 
webrev for 7200261 can be found at: 
http://cr.openjdk.java.net/~johnc/7200261/webrev.1 and was originally 
reviewed by Bengt, Jesper, and Jon Masamitsu.

Thanks,

JohnC