Request for review (S): 7166894 Add gc cause to Full GC logging for all collectors

[Bengt Rutisson]

Hi Vitaly and Mikael,

Thanks both of you for looking at this! Here is an updated webrev using 
jio_snprintf:
http://cr.openjdk.java.net/~brutisso/7166894/webrev.01/

On 2012-05-07 15:45, Vitaly Davidovich wrote:
>
> Yup, I don't think it would simplify the code but rather make any 
> potential maintenance easier, I suspect.  For example (and it's a 
> stretch), if you wanted to change the size of the message buffer, 
> you'd modify just one place.  However, this is very minor so your call.
>

When I went back to look at the G1 code I think that code will benefit 
even more from some abstraction like you suggested. But on the other 
hand there is a lot of logging code that would benefit from that. So, I 
have to think a bit about how much I would like to change. I'll leave it 
as it is for now.

Thanks again,
Bengt

> Thanks,
>
> Vitaly
>
> Sent from my phone
>
> On May 7, 2012 9:41 AM, "Bengt Rutisson" <bengt.rutisson at oracle.com 
> <mailto:bengt.rutisson at oracle.com>> wrote:
>
>
>     Hi Vitaly,
>
>     Thanks for looking at this!
>
>     On 2012-05-07 15:31, Vitaly Davidovich wrote:
>>
>>     Hi Bengt,
>>
>>     The allocation of the buffer to hold the string and sprintf'ing
>>     to it is repeated at the places you changed.  Do you think it's
>>     worth it to factor that out into a common function or maybe some
>>     stack allocated class whose destructor would free the message
>>     buffer? Just a thought ...
>>
>
>     I see your point, but I'm not sure this would actually simplify
>     the code.
>
>>     Also, probably better to use snprintf instead of sprintf.
>>
>
>     Good point. I'll fix this. This code fragment is actually copied
>     from existing G1 code, so I'll go back and change that too.
>
>     Thanks again for looking at it!
>     Bengt
>
>>     Regards,
>>
>>     Vitaly
>>
>>     Sent from my phone
>>
>>     On May 7, 2012 8:35 AM, "Bengt Rutisson"
>>     <bengt.rutisson at oracle.com <mailto:bengt.rutisson at oracle.com>> wrote:
>>
>>
>>         Hi all,
>>
>>         Can I get a couple of reviews of this simple change:
>>         http://cr.openjdk.java.net/~brutisso/7166894/webrev/
>>         <http://cr.openjdk.java.net/%7Ebrutisso/7166894/webrev/>
>>
>>         Background:
>>         I recently pushed a similar fix for G1 as "7163848: G1: Log
>>         GC Cause for a GC". That fix adds the GC cause information to
>>         all G1 GCs. It was discussed if we should do this for all
>>         collectors and we came to the conclusion that it would be
>>         fairly safe to do it for all Full GCs. These log messages
>>         already contain the text "(System)" or "(System.gc())" when a
>>         System.gc() happens and PrintGCDetails are enabled. Now they
>>         will always contain the cause in parenthesis, even when only
>>         PrintGC is enabled. Hopefully most parsing will work with this.
>>
>>         Thanks,
>>         Bengt
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20120507/f1eb8793/attachment.htm>