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

[Jesper Wilhelmsson]

21 sep 2011 kl. 19:06 skrev Ramki Ramakrishna <y.s.ramakrishna at oracle.com>:

> As regards, the sizes in bytes, and the efficiency, would it be a good idea to provide the units?
> (Is efficiency in bytes/ms?). Perhaps they are for internal consumption and left without units
> explicitly stated (with the understanding that if you don't know the units you should not be
> reading this log), but if this is for general user consumption, a header line somewhere with
> the unit would not hurt :-)
> 
> But this of course does not have to be done now.

If it's not done now it won't be done later. IMHO if the numbers are interesting to us during development they will be interesting to sustaining in the future. I don't think we should leave logging without units and assume that people will understand. 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
>>>