<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">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 href="mailto:jiangli.zhou@oracle.com">jiangli.zhou@oracle.com</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 href="mailto:dmitry.dmitriev@oracle.com">dmitry.dmitriev@oracle.com</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 href="http://cr.openjdk.java.net/~jiangli/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></body></html>