RFR(XXXS): 7092245: G1: Wrong format specifier in G1PrintRegionLivenessInfo header output

[Tony Printezis]

Jesper Wilhelmsson wrote:
> If it's not done now it won't be done later. 

Man, this argument sounds REALLY familiar. :-) :-) :-)

> IMHO if the numbers are interesting to us during development they will be interesting to sustaining in the future.

Of course.

>  I don't think we should leave logging without units and assume that people will understand. 

Jesper, you have a point. But, let's face it, if someone looks at this 
output and cannot figure out that the units are bytes, then their 
chances of actually understanding the output and doing something with it 
are close to nil.

Tony

> I do think that if it's possible to make the wrong assumption someone will make the wrong assumption and then file a bug on us to get it fixed.
> /Jesper
>
>   
>> -- ramki
>>
>>     
>>> Tony
>>>
>>> Bengt Rutisson wrote:
>>>       
>>>> John,
>>>>
>>>> Looks good.
>>>>
>>>> I agree with Ramki that it would have been nice to have the size value in an appropriate unit (especially considering that the minimum size i 1 MB and the size will always be a multiple of 1 MB). If you feel that such a change is outside the scope of this CR I am fine with your change as it is.
>>>>
>>>> Bengt
>>>>
>>>> On 2011-09-21 00:31, Y. S. Ramakrishna wrote:
>>>>         
>>>>> Would it be a good idea to provide size in KB or MB, to make
>>>>> the output more concise and easier for human consumption?
>>>>>
>>>>> Your change looks fine though.
>>>>> -- ramki
>>>>>
>>>>> On 09/20/11 15:22, John Cuthbertson wrote:
>>>>>           
>>>>>> Hi Everyone,
>>>>>>
>>>>>> Can I have a review of the tiny fix for this CR? The webrev can be found at: http://cr.openjdk.java.net/~johnc/7092245/webrev.0/
>>>>>>
>>>>>> Summary: In the G1PrintRegionLivenessInfo output, the region size in the header is printed using a SIZE_FORMAT but is typed as an int. That can cause some incorrect output by the 64-bit VM on some systems. The solution is to cast the value of HeapRegion::GrainBytes to a size_t in the print statement.
>>>>>>
>>>>>> Tested using gcbasher:
>>>>>>
>>>>>> Old output:
>>>>>> ### PHASE Post-Marking @ 1.481
>>>>>> ### HEAP  committed: 0x00000000eae00000-0x00000000f4d00000  reserved: 0x00000000eae00000-0x00000000fae00000  region-size: 139672337514496
>>>>>> ###
>>>>>>
>>>>>> New output:
>>>>>> ### PHASE Post-Marking @ 1.480
>>>>>> ### HEAP  committed: 0x00000000eae00000-0x00000000f4d00000  reserved: 0x00000000eae00000-0x00000000fae00000  region-size: 1048576
>>>>>> ###
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> JohnC
>>>>>>