<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi Derek,<br>
<br>
This version looks good to me.<br>
<br>
Thanks,<br>
StefanK<br>
<br>
On 16/02/16 19:42, Derek White wrote:<br>
</div>
<blockquote cite="mid:56C36DA2.3030102@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">2nd webrev puts the calls to
ensure_string_alive() back to their original location (to avoid
locking issues mentioned by Stefan).<br>
<br>
RFE: <a moz-do-not-send="true" class="issue-link"
data-issue-key="JDK-8149837"
href="https://bugs.openjdk.java.net/browse/JDK-8149837"
id="key-val" rel="4865448">JDK-8149837</a><br>
Webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/8149837/webrev.02/">http://cr.openjdk.java.net/~drwhite/8149837/webrev.02/</a><br>
<br>
Thanks for looking!<br>
<br>
- Derek<br>
<br>
On 2/15/16 4:57 PM, Derek White wrote:<br>
</div>
<blockquote cite="mid:56C249DB.2000003@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Stefan,<br>
<br>
On 2/15/16 3:55 PM, Stefan Karlsson wrote:<br>
</div>
<blockquote cite="mid:56C23B57.1090505@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Derek,<br>
<br>
I don't think this change will work, unless something
changed in this area recently. We used to have the code the
way your patch is written, but unfortunately we end up with
a lock reordering problem with the StringTable_lock and one
of the locks taken when executing the
G1SATBCardTableModRefBS::enqueue. Unfortunately, there's no
comment in the code that shows this intricate problem.<br>
<br>
This is the problematic code:<br>
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
<pre> 241
242 // Grab the StringTable_lock before getting the_table() because it could
243 // change at safepoint.
244 oop added_or_found;
245 {
246 MutexLocker ml(StringTable_lock, THREAD);
247 // Otherwise, add to symbol to table
248 added_or_found = the_table()->basic_add(index, string, name, len,
249 hashValue, CHECK_NULL);
250 }
<span class="removed"> 251 </span>
<span class="removed"> 252 ensure_string_alive(added_or_found);</span>
253
254 return added_or_found;
255 }</pre>
We hold StringTable_lock, while calling this chain:
basic_add ->
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
lookup_in_main_table ->
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
ensure_string_alive -> G1SATBCardTableModRefBS::enqueue.<br>
<br>
I'm not sure, but I think this was the code that took the
lock:<br>
void PtrQueueSet::enqueue_complete_buffer(void** buf, size_t
index) {<br>
MutexLockerEx x(_cbl_mon,
Mutex::_no_safepoint_check_flag);<br>
<br>
and if that's the case you might be able to provoke the
problem by setting the
G1SATBBufferEnqueueingThresholdPercent flag.<br>
<br>
Have you run this patch with fastdebug on a larger set of
benchmarks?<br>
</div>
</blockquote>
OK, I was wondering if there was something deeper that I wasn't
seeing. I haven't tried with a larger benchmark case.<br>
<blockquote cite="mid:56C23B57.1090505@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
I think we should hold this patch until we have figured out
if this is still a problem or not.<br>
</div>
</blockquote>
<br>
OK, I can also go back to my plan A, which left the calls to <span
class="removed">ensure_string_alive() in the same place, but
made the calls conditional on whether or not the string
returned from the "intern" was the same as the string passed
in.<br>
<br>
Thanks for the quick response!<br>
</span>
<blockquote cite="mid:56C23B57.1090505@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Thanks,<br>
StefanK<br>
<br>
On 2016-02-15 20:52, Derek White wrote:<br>
</div>
<blockquote cite="mid:56C22C86.3090407@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
This small change only does a STAB read barrier if we really
read a string from the string intern table.<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
This is a small optimization, and the old code may
contribute to the malloc failures in <a
moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8134992"
title="vm/gc/compact/Compact_InternedStrings_Strings
failed due to a malloc() failure" class="issue-link"
data-issue-key="JDK-8134992">JDK-8134992</a>
"vm/gc/compact/Compact_InternedStrings_Strings failed due to
a malloc() failure" <br>
<br>
RFE:
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
<a moz-do-not-send="true" class="issue-link"
data-issue-key="JDK-8149837"
href="https://bugs.openjdk.java.net/browse/JDK-8149837"
id="key-val" rel="4865448">JDK-8149837</a><br>
Webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/8149837/webrev.01/">http://cr.openjdk.java.net/~drwhite/8149837/webrev.01/</a><br>
<br>
Tested: jprt<br>
<br>
Thanks! <br>
- Derek<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>