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

[John Cuthbertson]

Hi Jesper,

Thanks for reviewing the changes (for the second time) :).

JohnC

On 10/30/12 14:22, Jesper Wilhelmsson wrote:
> Hi John,
>
> Looks good to me.
> /Jesper
>
>
> On 2012-10-30 19:59, John Cuthbertson wrote:
>> 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
>