<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi Bengt,<div><br></div><div>This is a much more reasonable change then adding log levels of fine and finest (changes which I still strongly object to as they are flags with absolutely no meaning). The PrintReason flag also seems like a reasonable thing to include across all collectors. If the information is contained in brackets (similar to (system) indication of System.gc()) then that is something parser will either handle or should easily be adjusted to handle. I'd welcome that also.</div><div><br></div><div>Regards,</div><div>Kirk</div><div><br><div><div>On 2012-05-07, at 10:09 PM, Bengt Rutisson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
  
    <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
  
  <div bgcolor="#FFFFFF" text="#000000">
    <br>
    Hi Vitaly and Mikael,<br>
    <br>
    Thanks both of you for looking at this! Here is an updated webrev
    using jio_snprintf:<br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~brutisso/7166894/webrev.01/">http://cr.openjdk.java.net/~brutisso/7166894/webrev.01/</a><br>
    <br>
    On 2012-05-07 15:45, Vitaly Davidovich wrote:
    <blockquote cite="mid:CAHjP37Fd=7q5we=_f7hSuQeefKD-sUWP5muP3cf=kYTwzY+B4g@mail.gmail.com" type="cite"><p>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.</p>
    </blockquote>
    <br>
    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.<br>
    <br>
    Thanks again,<br>
    Bengt<br>
    <br>
    <blockquote cite="mid:CAHjP37Fd=7q5we=_f7hSuQeefKD-sUWP5muP3cf=kYTwzY+B4g@mail.gmail.com" type="cite"><p>Thanks,</p><p>Vitaly</p><p>Sent from my phone</p>
      <div class="gmail_quote">On May 7, 2012 9:41 AM, "Bengt Rutisson"
        <<a moz-do-not-send="true" href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>>
        wrote:<br type="attribution">
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex">
          <div bgcolor="#ffffff" text="#000000"> <br>
            Hi Vitaly,<br>
            <br>
            Thanks for looking at this!<br>
            <br>
            On 2012-05-07 15:31, Vitaly Davidovich wrote:
            <blockquote type="cite"><p>Hi Bengt,</p><p>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 ...</p>
            </blockquote>
            <br>
            I see your point, but I'm not sure this would actually
            simplify the code.<br>
            <br>
            <blockquote type="cite"><p>Also, probably better to use snprintf instead of
                sprintf.</p>
            </blockquote>
            <br>
            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.<br>
            <br>
            Thanks again for looking at it!<br>
            Bengt<br>
            <br>
            <blockquote type="cite"><p>Regards,</p><p>Vitaly</p><p>Sent from my phone</p>
              <div class="gmail_quote">On May 7, 2012 8:35 AM, "Bengt
                Rutisson" <<a moz-do-not-send="true" href="mailto:bengt.rutisson@oracle.com" target="_blank">bengt.rutisson@oracle.com</a>>
                wrote:<br type="attribution">
                <blockquote class="gmail_quote" style="margin:0pt 0pt
                  0pt 0.8ex;border-left:1px solid
                  rgb(204,204,204);padding-left:1ex"> <br>
                  Hi all,<br>
                  <br>
                  Can I get a couple of reviews of this simple change:<br>
                  <a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Ebrutisso/7166894/webrev/" target="_blank">http://cr.openjdk.java.net/~brutisso/7166894/webrev/</a><br>
                  <br>
                  Background:<br>
                  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.<br>
                  <br>
                  Thanks,<br>
                  Bengt<br>
                </blockquote>
              </div>
            </blockquote>
            <br>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </div>

</blockquote></div><br></div></body></html>