<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 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>
</body>
</html>