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

[Bengt Rutisson]

On 2012-05-08 07:28, Kirk Pepperdine wrote:
> This 7166894: Add gc cause to Full GC logging for all collectors 
> <http://monaco.us.oracle.com/detail.jsp?cr=7166894> leads me to an 
> internal Oracle server.

This is unfortunate, but it is a consequence of the fact that we still 
do not have a publicly available JIRA installation for OpenJDK issues. 
As you are aware there is active work on creating such an installation.

In the mean time the public access to the bug system is through 
bugs.sun.com, which is lagging several days behind. The current bug is 
still not available there, but will hopefully be soon, on this URL:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7166894

Bengt

>
> Regards,
> Kirk
>
> On 2012-05-07, at 10:09 PM, Bengt Rutisson wrote:
>
>>
>> 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/20120508/13347f04/attachment.htm>