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

[Bengt Rutisson]

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/9018b49e/attachment.htm>