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

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Wed Sep 21 21:55:40 UTC 2011


21 sep 2011 kl. 23:05 skrev Tony Printezis <tony.printezis at oracle.com>:

> I didn't implement it, it was a mock-up. :-)

Why.. Tony, I don't know what to say.

Adding a print statement in front of that line might be doable though. :-)
/J

> 
> Jesper Wilhelmsson wrote:
>> 21 sep 2011 kl. 22:53 skrev Tony Printezis <tony.printezis at oracle.com>:
>> 
>>  
>>> 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.
>>>    
>> 
>> That's probably true, but since you did implement the nice header with the units in your reply to Ramki, I think you should keep it :-)
>> /J
>> 
>>  
>>> 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
>>>>>>>>>                           



More information about the hotspot-gc-dev mailing list