Code review request: 6855834: G1: minimize the output when -XX:+PrintHeapAtGC is set (S)

[Tony Printezis]

John,

Hi.

John Coomes wrote:
> Tony Printezis (Antonios.Printezis at Sun.COM) wrote:
>   
>>> 1.  if (PrintHeapAtGC){    <---- no space before the '{' 
>>>
>>> 	This occurs 4 times (I think).
>>>   
>>>       
>> Yep, again bad cut-and-paste.
>>     
> Sure, I guessed as much.
>   
:-( Thanks again for spotting it.
>>> 2.  No blank line between methods print(), print_on(outputStream*) and
>>> print_on(outputStream*, bool) (lines 2336, 2339).
>>>   
>>>       
>> I'll add them, but it's the same in genCollectedHeap.cpp:
>>
>> void GenCollectedHeap::print() const { print_on(tty); }
>> void GenCollectedHeap::print_on(outputStream* st) const {
>>   for (int i = 0; i < _n_gens; i++) {
>>     _gens[i]->print_on(st);
>>   }
>>   perm_gen()->print_on(st);
>> }
>>     
>
> Thanks.  I think it's easier to read with the blank line.
>   
I agree, and it's consistent with the code base...
>>> 3.  No space between the double quote and INTPTR_FORMAT or
>>> SIZE_FORMAT in the new print_on() code.  AFAICT, most of hotspot uses
>>> this:
>>>
>>> 	print("count = " SIZE_FORMAT "K", count);
>>>
>>> instead of
>>>
>>> 	print("count = "SIZE_FORMAT"K", count);
>>>
>>> I find the latter harder to parse.  I see some existing examples of
>>> the latter in the same file, but haven't seen it outside of G1.
>>>   
>>>       
>> I personally find the second version easier to parse, given that it's a 
>> bit clearer where there's white space and where there's not. I'll change 
>> it in an attempt to be consistent though.
>>     
>
> Strange how we see things.  The "SIZE_FORMAT" in the latter is what I
> notice at first glance, which makes me think the wrong thing is
> quoted.
>   
Different people have different approaches I suppose!

Tony