<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hello Jiangli,<br>
    <br>
    Thank you. About extra spaces - I just highlight changed lines in
    webrev and see that some lines have additional spaces at the end. :)<br>
    <br>
    Dmitry<br>
    <br>
    <div class="moz-cite-prefix">On 07.08.2015 3:40, Jiangli Zhou wrote:<br>
    </div>
    <blockquote
      cite="mid:30B0248C-3BEA-4FAE-861F-DAAE19F56B45@oracle.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html;
        charset=windows-1252">
      Hi Dmitry,
      <div><br>
      </div>
      <div>I’ve added shared string unmapping to handle the string data
        verification failure. I also added '<span style="font-size:
          11px;">free_archive_regions()’ call
          to FileMapInfo::unmap_string_regions().</span><span
          style="font-size: 11px;"> My testing revealed some issues with
          the new archive region free code when I forced string
          verification failure. I’m working with Tom on the issues.</span></div>
      <div><span style="font-size: 11px;"><br>
        </span></div>
      <div><span style="font-size: 11px;">Thanks,</span></div>
      <div><span style="font-size: 11px;">Jiangli</span></div>
      <div><font face="Menlo"><span style="font-size: 11px;"><br>
          </span></font></div>
      <div><font face="Menlo"><span style="font-size: 11px;"><br>
          </span></font>
        <div>
          <div>On Aug 6, 2015, at 3:19 PM, Jiangli Zhou <<a
              moz-do-not-send="true"
              href="mailto:jiangli.zhou@oracle.com"><a class="moz-txt-link-abbreviated" href="mailto:jiangli.zhou@oracle.com">jiangli.zhou@oracle.com</a></a>>
            wrote:</div>
          <br class="Apple-interchange-newline">
          <blockquote type="cite">Hi Dmitry,<br>
            <br>
            Thank you for the feedback! You are definitely right about
            freeing the archived regions in those failure cases. I
            should have thought it more thoroughly, instead of just
            fixing the case that I ran into. Will exam all cases.<br>
            <br>
            I’ll change the test to incorporate your suggestion. Will
            fix the extra spaces before committing. I have one question,
            how do you detect the extra spaces from the webrev? :)<br>
            <br>
            Thanks,<br>
            Jiangli<br>
            <br>
            On Aug 6, 2015, at 2:59 PM, Dmitry Dmitriev <<a
              moz-do-not-send="true"
              href="mailto:dmitry.dmitriev@oracle.com"><a class="moz-txt-link-abbreviated" href="mailto:dmitry.dmitriev@oracle.com">dmitry.dmitriev@oracle.com</a></a>>
            wrote:<br>
            <br>
            <blockquote type="cite">Hello Jiangli,<br>
              <br>
              I have few comments/questions.<br>
              <br>
              src/share/vm/memory/filemap.cpp module:<br>
              1) Should free_archive_regions also called when
              verify_string_regions() returns false on line 717?<br>
              2) The same question about unmap_string_regions(). Region
              must be freed when unmap_string_regions() is called?<br>
              3) Extra space at the end of the line 711.<br>
              <br>
              test/runtime/SharedArchiveFile/SharedStringsRunAuto.java<br>
              1) Unneeded second creating of OutputAnalyzer on line 61.
              Also, probably will be better to use same scheme for
              OutputAnalyzer? On lines 46-49 you not use local variable,
              but on lines 61-63 use local variable.<br>
               59         OutputAnalyzer output = new
              OutputAnalyzer(pb.start());<br>
               60 <br>
               61         output = new OutputAnalyzer(pb.start());<br>
              2) Extra space at the end of the lines 25,26,30, 51<br>
              <br>
              Thanks,<br>
              Dmitry<br>
              <br>
              On 07.08.2015 0:32, Jiangli Zhou wrote:<br>
              <blockquote type="cite">Hi,<br>
                <br>
                Here is the runtime part of the bug fix that calls the
                new free_archive_regions() when shared string mapping
                fails. I also added a jtreg test to test shared strings
                with -Xshare:auto.<br>
                <br>
                 <a moz-do-not-send="true"
                  href="http://cr.openjdk.java.net/%7Ejiangli/8131734/webrev.00/">http://cr.openjdk.java.net/~jiangli/8131734/webrev.00/</a><br>
                <br>
                Test:<br>
                - Tested by explicitly making the shared string mapping
                fail on linux-x64, -Xshare:auto runs without crash with
                the fix<br>
                - Tested with the new SharedStringsRunAuto test<br>
                - Tested with XX:+PrintNMTStatistics
                -XX:NativeMemoryTracking=detail<br>
                <br>
                Thanks,<br>
                Jiangli<br>
              </blockquote>
              <br>
            </blockquote>
            <br>
          </blockquote>
        </div>
        <br>
      </div>
    </blockquote>
    <br>
  </body>
</html>