RFR(S): 7200261: G1: Liveness counting inconsistencies during marking verification

[John Cuthbertson]

Hi Everyone,

A new webrev for these changes can be found at: 
http://cr.openjdk.java.net/~johnc/7200261/webrev.1/

Based upon the code review comments - the set_card_bitmap_range() 
routine now takes an exclusive range of cards. As a result I have to 
check whether the end address of the current address range/region/object 
(i.e. the start the next) is card aligned (and in the heap) and 
increment the card index range appropriately. The essence of the bugfix 
is the same.

Testing:
The failing tests; GC test suite with a low IHOP and verification; GC 
test suite with forced evacuation failures; nsk  (gc,  jit, regression, 
and runtime) tests with a low IHOP and verification; jprt.

Thanks,

JohnC

On 09/25/12 10:54, John Cuthbertson wrote:
>
> Anyway the routine now takes an exclusive range - which, IMO, doesn't 
> look any cleaner than the previous code. Expect a new webrev after 
> some more testing.
>
> JohnC
>
> On 09/25/12 06:41, Jon Masamitsu wrote:
>> John,
>>
>> Is it common practice in G1 code to have the ranges
>> be inclusive?
>>
>> Jon
>>
>> PS.  Sorry for being tardy on my reply - so much mail,
>> so little time :-)
>>
>> On 09/24/12 10:03, John Cuthbertson wrote:
>>> Hi Jon,
>>>
>>> Thanks for the comments. I'm going to refer you to my reply to 
>>> Jesper. If people feel strongly about making the routine exclusive - 
>>> I'll make it so.
>>>
>>> JohnC
>>>
>>> On 09/21/12 22:48, Jon Masamitsu wrote:
>>>> I'm used to seeing a range like [start, end).  That the second 
>>>> index is named
>>>> last_idx helps but if I were just looking at the call site, I would 
>>>> have guessed
>>>> wrongly - i.e., thinking it was [start, end).
>>>>
>>>> Jon
>>>>
>>>> On 9/21/2012 9:16 PM, Jesper Wilhelmsson wrote:
>>>>> John,
>>>>>
>>>>> Looks good!
>>>>>
>>>>> Would it make sense to change set_card_bitmap_range to be 
>>>>> exclusive, or even to take a start and a size of the area? I think 
>>>>> inclusive functions like this one are unintuitive, especially when 
>>>>> the only use of last_idx is used with a +1. Maybe that's a 
>>>>> different change?
>>>>> /Jesper
>>>>>
>>>>>
>>>>>
>>>>> 22 sep 2012 kl. 01:37 skrev John 
>>>>> Cuthbertson<john.cuthbertson at oracle.com>:
>>>>>
>>>>>> Hi Everyone,
>>>>>>
>>>>>> Can I have a couple of volunteers look over the fix for this CR? 
>>>>>> The webrev can be found at: 
>>>>>> http://cr.openjdk.java.net/~johnc/7200261/webrev.0/
>>>>>>
>>>>>> Summary:
>>>>>> The clipping in the code that sets the bits for a range of cards 
>>>>>> in the "expected" card bitmap that we check the liveness 
>>>>>> accounting data against was incorrect. This could lead to 
>>>>>> spurious verification failures. In addition to fixing the 
>>>>>> clipping, I've upleveled this routine and moved it into 
>>>>>> ConcurrentMark and now use it to generate the real liveness data.
>>>>>>
>>>>>> Testing:
>>>>>> The failing test cases with marking verification; jprt.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> JohnC
>>>
>