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

[Srinivas Ramakrishna]

Mikael, thanks for sounding that note of caution... what you say makes
sense.

-- ramki

On Fri, May 11, 2012 at 10:17 AM, Mikael Vidstedt <
mikael.vidstedt at oracle.com> wrote:

>
> I'm all for improving the information in the log messages, great work!
> However, I'm not sure I'm warm and fuzzy about potentially breaking users'
> log parsers in a minor update. My preference would be to have the
> PrintGCCause flag be default false in jdk7 and default true in jdk8+. In
> general I'd prefer to only change the log messages in major releases.
>
> Reasonable?
>
> Cheers,
> Mikael
>
>
> On 2012-05-11 07:30, Bengt Rutisson wrote:
>
>
> Hi Kris,
>
> Thanks again for looking at this.
>
> I had to make some minor changes make it compile on all platforms. Mostly
> some explicit casts to const char*. Here is an updated webrev:
> http://cr.openjdk.java.net/~brutisso/7166894/webrev.04/
>
> More comments inline.
>
> On 2012-05-08 16:43, Krystal Mok wrote:
>
> Hi Bengt,
>
>  The current factoring looks nice and uniform. Thanks :-)
>
>  But for most minor GCs and both CMS pause phases, the extra logging
> doesn't really give additional information.
> Most minor GCs are going to say "Allocation Failure", and the two CMS
> phases would change from, e.g.
>
>  [GC [1 CMS-initial-mark
>
>  to something like
>
>  [GC (CMS Initial Mark) [1 CMS-initial-mark
>
>  which is probably reasonable given the scope of the change, but not
> really helpful.
> The "real cause", such as which generation (or perhaps System.gc() with
> ExplicitGCInvokesConcurrent, or even GC locker) is triggering this
> collection cycle, may be more useful, but it's hard to fit into the current
> form.
>
>
> Yes, I think you are correct in both cases. The gc cause that we have
> available does not always add a lot of information. This is relevant to fix
> but it is a slightly different issue than what this patch sets out to fix.
> Let's try to get this in first and then evaluate how the GC causes should
> be set.
>
> Thanks,
> Bengt
>
>
>  - Kris
>
> On Tue, May 8, 2012 at 10:18 PM, Bengt Rutisson <bengt.rutisson at oracle.com
> > wrote:
>
>>
>> Hi again everyone,
>>
>> It seems like the feedback on hotspot-gc-use is that we should add the GC
>> cause to all collectors but also provide a switch to turn this logging off.
>>
>> Here is an updated webrev:
>> http://cr.openjdk.java.net/~brutisso/7166894/webrev.03/
>>
>> Changes:
>> * GC cause logged for all collectors
>> * Added the flag -XX:-PrintGCCause to turn the new information off
>> * Refactored the string concatenation code into a helper class
>>
>> I guess I will also have to update the CR to now reflect the fact that
>> this does not just concern full GCs anymore.
>>
>> Thanks,
>> Bengt
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20120511/4c826bea/attachment.htm>