Request for review (S): 7166894 Add gc cause to Full GC logging for all collectors
bengt.rutisson at oracle.com
Mon May 7 13:09:22 PDT 2012
Hi Vitaly and Mikael,
Thanks both of you for looking at this! Here is an updated webrev using
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.
> 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!
>> 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:
>> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev