<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi, Lois and all<br>
    <br>
      This is fourth version after suggestions from Lois,<br>
      URL: <a moz-do-not-send="true"
      href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev0">http://cr.openjdk.java.net/~minqi/7164841/webrev0</a>3<br>
    <br>
      Add a function dump_loggc_header to fileStream to dump vm version
    etc information as reusable function.<br>
      Also some minor format and comments changes.<br>
    <br>
      Thanks<br>
      Yumin<br>
    <br>
    <div class="moz-cite-prefix">On 9/4/2013 10:30 AM, Lois Foltan
      wrote:<br>
    </div>
    <blockquote cite="mid:52276E3E.6040706@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      <br>
      <div class="moz-cite-prefix">On 9/4/2013 12:27 PM, Yumin Qi wrote:<br>
      </div>
      <blockquote cite="mid:52275F68.9080704@oracle.com" type="cite">
        <meta content="text/html; charset=UTF-8"
          http-equiv="Content-Type">
        Thanks, Lois<br>
        <br>
        <blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
          arguments.cpp - <br>
              - line # 1867, please add more information on the context
          in which this function can be called<br>
          <span class="new"><br>
          </span></blockquote>
        I change the comments to:<br>
        <br>
        // This function is called for -Xloggc:&lt;filename&gt;, it can
        be used<br>
        // to check if a given file name(or string) conform to following<br>
        // request:<br>
        // A valid string only contains "[A-Z][a-z][0-9].-_%[p|t]"<br>
        // %p and %t only allowed once.<br>
        <br>
        Is this OK?<br>
      </blockquote>
      <br>
      Hi Yumin,<br>
      Yes, thank you, just a nit, instead of "conform to following
      request:" clearer to state "conforms to the following
      specification:"<br>
      <br>
      <blockquote cite="mid:52275F68.9080704@oracle.com" type="cite">
        <blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite"><span
            class="new">     - line #  1868, // A valid file name only
            contains "[A-Z][a-z][0-9].-_%[p|t]"</span> <br>
                I really like the feature that allows a user to tailor
          the log file name to include or not include the pid and
          timestamp.<br>
                I think this is a big improvement.  However, this does
          represent a change over current accepted behavior.  By
          accepting <br>
                only "[A-Z][a-z][0-9] and "." in the file name, a user,
          who once specified -Xloggc:foo%.log for example, will now
          receive <br>
                an error.  Or for that matter any special character like
          ?, *, +.   So, I don't know if this is change in behavior is<br>
                allowable.<br>
          <br>
        </blockquote>
        I think we can constrain the special chars for file name usage
        here --- there will be problem when user upgrade to new version
        which contains this changeset so they need to change their
        scripts to follow the requirement in this change. <br>
        <br>
        Wish to get more input for this change.<br>
        <blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
          ostream.hpp - looks good, no comments<br>
          <br>
          ostream.cpp -<br>
              - Remove extra lines (#372 &amp; #446) around
          extend_file_name() function<br>
            <br>
        </blockquote>
        will<br>
        <blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
             - It would be good to have a couple more comments in
          extend_file_name(), <br>
               like for example, between line #384 and #385 something
          like:<br>
                      // No user specified request for pid &amp;
          timestamp in -Xloggc file name.<br>
               Between 431 &amp; 432, maybe // Only requested pid <br>
               Between 438 &amp; 439, maybe // Only requested timestamp<br>
          <br>
        </blockquote>
        Will add<br>
        <blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
             - In the situation where NumberOfGCLogFiles &gt; 1, how
          does the very first file opened <br>
               in the rotation obtain the additional GC log file header
          info you have added?  The first <br>
               file is opened in the constructor so does not have the
          benefit of being opened at line <br>
               #647 and then having the header info written to it in
          line #658-663.<br>
          <br>
              - I think the additional GC log file header info would be
          beneficial to a user even in the situation<br>
                where no UseGCLogFileRotation options are specified on
          the command line.  A user may<br>
                be confused why when just -Xloggc:&lt;filename&gt; is
          specified no additional header info is provided.<br>
                One possible solution to this and the previous comment
          is to add similar code as #658-663 to the<br>
                constructor.<br>
          <br>
              - In rotatingFileStream::rotate_log(), I still have
          questions about the error situation where the next file <br>
                in the rotation can not be opened.  The current file was
          closed at line #619, UseGCLogFileRotation is<br>
                turned off at line #675, so how will e current file be
          reopened to accommodate for the next write in<br>
                rotatingFileStream::write()?<br>
          <br>
        </blockquote>
        Good catch.  I thought of this, and think rotation will override
        that info. Will change to add this when the first time file is
        created for both non-rotate and rotate settings.<br>
      </blockquote>
      My original suggestion of just duplicating lines #658-663 in the
      constructor can be improved upon.  I think it would be worthwhile
      to consider adding a function named something like,
      dump_xloggc_header(), for example.  This allows for future
      additional pertinent information to be included in the header very
      easily without having to visit all the different places where the
      header info is generated.  <br>
      <br>
      Thanks,<br>
      Lois<br>
      <blockquote cite="mid:52275F68.9080704@oracle.com" type="cite"> <br>
        Thanks<br>
        Yumin<br>
        <br>
        <blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
                <br>
          Thanks,<br>
          Lois<br>
          <br>
          <div class="moz-cite-prefix">On 8/31/2013 2:36 AM, Yumin Qi
            wrote:<br>
          </div>
          <blockquote cite="mid:52218EDE.4060508@oracle.com" type="cite">
            <meta content="text/html; charset=UTF-8"
              http-equiv="Content-Type">
            Hi, all<br>
            <br>
              <br>
              With more feedback on the second version, the third
            version make following changes:<br>
            <br>
              1) take parametrized filename after -Xloggc:, the filename
            will be forced to follow following rules:<br>
                  can only contain [A-Z][a-z][0-9].-_%[p|t], any other
            character used for file name will be rejected. %p or %t can
            only appear once in file name.<br>
                  example:  -Xloggc:test.log-%t-%p           OK<br>
                  example: -Xloggc:test.log-%p-%p           FAIL<br>
            <br>
               2) removed unused rotatingFileStream constructors<br>
               <br>
               3) log more information at head of rotation file:<br>
                   vm version, os version, build id etc<br>
                   memory usage<br>
                   commandline  flags<br>
            <br>
               Tested on Linux/Windows<br>
            <br>
               URL: <a moz-do-not-send="true"
              href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev0">http://cr.openjdk.java.net/~minqi/7164841/webrev0</a>2<br>
            <br>
              Thanks<br>
              Yumin<br>
            <br>
            <div class="moz-cite-prefix">On 8/21/2013 3:43 PM, Yumin Qi
              wrote:<br>
            </div>
            <blockquote cite="mid:521542A3.3020906@oracle.com"
              type="cite">
              <meta content="text/html; charset=UTF-8"
                http-equiv="Content-Type">
              Hi, all<br>
              <br>
                This is second version after feedback from first round.<br>
                The changes are:<br>
              <br>
                1) file name will be based on gc log file name
              (-Xloggc:filename), pid (process id) and time when the
              first rotation file created:<br>
                     &lt;filename&gt;-pid&lt;pid&gt;-YYYY-MM-DD_HH-MM-SS<br>
                2) If rotate in same file, file name is as in 1), if
              rotate in multiple files, .&lt;i&gt; will append to above
              file name.<br>
                3) every time file rotated, file name and time when file
              created will be logged to head/tail, this is same as first
              version.<br>
                4) current file has name 
              &lt;filename&gt;-pid&lt;pid&gt;-YYYY-MM-DD_HH-MM-SS.&lt;i&gt;.current<br>
                     This is similar to first version.<br>
                     By adapting such name format we will never loss
              logs in case apps stops and restart, the log files will
              not be overwritten since time stamp in file names.<br>
                 5) If open file failed, turn off gc log rotation. <br>
                      If due to some reason, file operation failed (such
              as permission changed etc), with log file opened, logging
              still works, but at <br>
                      saving and renaming, the file operation will fail,
              this will lead not all files saved.<br>
              <br>
                   <a moz-do-not-send="true"
                class="moz-txt-link-freetext"
                href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev01">http://cr.openjdk.java.net/~minqi/7164841/webrev01</a><br>
                   <br>
                   Tested with jtreg, JPRT.<br>
              <br>
              Thanks<br>
              Yumin<br>
              <br>
              <div class="moz-cite-prefix">On 8/15/2013 8:35 AM, Yumin
                Qi wrote:<br>
              </div>
              <blockquote cite="mid:520CF52B.6050000@oracle.com"
                type="cite">
                <meta http-equiv="content-type" content="text/html;
                  charset=UTF-8">
                Hi, <br>
                <br>
                  Can I have your review for this small changes?<br>
                  <a moz-do-not-send="true"
                  href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev00/">http://cr.openjdk.java.net/~minqi/7164841/webrev00/</a><br>
                <br>
                   This is for a enhancement to add head/tail message to
                the logging files to assist reading GC output.<br>
                   1. modified prompt message if invalid arguments used
                for log rotating;<br>
                   2. add time and file name message to log file
                head/tail.<br>
                   3. for easily identify which log file is current, use
                file name like &lt;filename&gt;.n.current, after it
                reaches maximum size, rename it to &lt;filename&gt;.n<br>
                        On Windows, there is no F_OK (existing test)
                definition, F_OK is defined as "0" and for _access of
                VC++, it just describes:<br>
                         <br>
                <table style="border-collapse: collapse; padding: 0px;
                  width: 1306px; border: 1px solid rgb(187, 187, 187);
                  color: rgb(0, 0, 0); font-family: 'Segoe UI', 'Lucida
                  Grande', Verdana, Arial, Helvetica, sans-serif;
                  font-size: 13px; font-style: normal; font-variant:
                  normal; font-weight: normal; letter-spacing: normal;
                  line-height: 17px; orphans: auto; text-align: start;
                  text-indent: 0px; text-transform: none; white-space:
                  normal; widows: auto; word-spacing: 0px;
                  -webkit-text-stroke-width: 0px;">
                  <tbody>
                    <tr>
                      <th style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px;
                        background-color: rgb(237, 237, 237); color:
                        rgb(112, 112, 112); text-align: left;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;"><span class="parameter"
                            style="font-style: italic;">mode</span><span
                            class="Apple-converted-space"> </span>value</p>
                      </th>
                      <th style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px;
                        background-color: rgb(237, 237, 237); color:
                        rgb(112, 112, 112); text-align: left;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">Checks file for</p>
                      </th>
                    </tr>
                    <tr>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">00</p>
                      </td>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">Existence only</p>
                      </td>
                    </tr>
                    <tr>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">02</p>
                      </td>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">Write-only</p>
                      </td>
                    </tr>
                    <tr>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">04</p>
                      </td>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">Read-only</p>
                      </td>
                    </tr>
                    <tr>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">06</p>
                      </td>
                      <td style="border: 1px solid rgb(187, 187, 187);
                        margin: 10px; padding: 10px 8px; color: rgb(42,
                        42, 42); vertical-align: top;">
                        <p style="color: rgb(42, 42, 42); margin-top:
                          0px; margin-bottom: 0px; padding-bottom: 0px;
                          line-height: 18px;">Read and write</p>
                      </td>
                    </tr>
                  </tbody>
                </table>
                <br>
                <a moz-do-not-send="true"
                  href="http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx">http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx</a><br>
                The definition are consistent with unistd.h.       <br>
                <br>
                    Test: JPRT and jtreg.<br>
                <br>
                   Thanks<br>
                   Yumin<br>
              </blockquote>
              <br>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>