<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix"><br>
Hi Kim,<br>
<br>
On 13/04/15 04:47, Kim Barrett wrote:<br>
</div>
<blockquote
cite="mid:3883505F-5492-4255-81C3-8E22BA4CD795@oracle.com"
type="cite">
<pre wrap="">Please review this change to the filtering of SATB queue contents.
The problem is that a SATB queue may contain a stale reference to an
eagerly reclaimed humongous object. Assert-conditional verification
during full queue processing that all entries are oop's can now fail.
We deal with this by eliminating the pre-filtering verification
(removed call to ObjPtrQueue::verify_oops_in_buffer and removed that
now unused function). Instead, ObjPtrQueue::filter has been revised
to be more careful about oop-ness assumptions, and to assert the
oop-ness of entries that are retained.
This is related to
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8073717">https://bugs.openjdk.java.net/browse/JDK-8073717</a>
which is another failure due to unexpected stuff in SATB queues as a
result of eager reclaim of a humongous object.
As an aside, ObjPtrQueue is now poorly named. A followup CR will be
filed to rename it to something like SATBQueue.
CR:
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8075466">https://bugs.openjdk.java.net/browse/JDK-8075466</a>
Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8075466/webrev.00/">http://cr.openjdk.java.net/~kbarrett/8075466/webrev.00/</a></pre>
</blockquote>
<br>
Nice refactoring and fix! Some minor comments:<br>
<br>
satbQueue.cpp<br>
<br>
What's the advantage of casting to HeapWord** and HeapWord* in
ObjPtrQueue::filter() rather than just working with void** and void*
when popping from and pushing to the "buf"?<br>
<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
83 if (entry >= region->next_top_at_mark_start()) return
false;<br>
<br>
Please add some brackets to this if statement.<br>
<br>
if (entry >= region->next_top_at_mark_start()) {<br>
return false;<br>
}<br>
<br>
<br>
85 // Can obj really have it's mark word set? It's not in young
gen...<br>
86 assert(((oop)entry)->is_oop(true /* ignore mark word */),<br>
87 err_msg("Invalid oop in SATB buffer: " PTR_FORMAT,
p2i(entry)));<br>
<br>
What does the comment mean? This is concurrent marking so anyone can
be manipulating the mark work. Locking etc. I think that is why the
assert avoids checking the mark word.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
<blockquote
cite="mid:3883505F-5492-4255-81C3-8E22BA4CD795@oracle.com"
type="cite">
<pre wrap="">
Testing:
JPRT
local RefWorkload with -XX:+UseG1GC
Aurora GC Nightly and VM quicktests with -XX:+UseG1GC
</pre>
</blockquote>
<br>
</body>
</html>