<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>