<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Kim,<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00/src/share/vm/gc/g1/dirtyCardQueue.hpp.frames.html">http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00/src/share/vm/gc/g1/dirtyCardQueue.hpp.frames.html</a><br>
<br>
<pre><span class="changed"> 55 // Apply the closure to all active elements, from index to <font color="#ff0000">size</font>. If</span></pre>
<br>
Change "size" to "_sz"? It's hard to understand what "size" is from
only the<br>
signature of apply_closure(). Using "_sz" gives a (weak) hint that
the upper limit<br>
is a field in the class.<br>
<br>
<pre> 69 static bool apply_closure_to_buffer(CardTableEntryClosure* cl,</pre>
Would "apply_closure_to_node" now be a better name?<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00/src/share/vm/gc/g1/satbMarkQueue.cpp.frames.html">http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00/src/share/vm/gc/g1/satbMarkQueue.cpp.frames.html</a><br>
<br>
<blockquote type="cite">
<pre><span class="changed"> 129 if (retain_entry(entry, g1h)) {</span>
<span class="changed"> 130 // Found keeper. Search high to low for an entry to discard.</span>
<span class="changed"> 131 while ((src < --dst) && retain_entry(*dst, g1h)) { }</span>
<span class="changed"> 132 if (src >= dst) break; // Done if no discard found.</span></pre>
</blockquote>
<br>
If I'm at line 132 then "src" points to a keeper and is the lowest<br>
(addressed) keeper. If that's true, then if "dst" is less than
"src"<br>
as seems to be allowed at line 132, then the index calculation<br>
<pre><blockquote type="cite"><pre><span class="changed"> 137 _index = pointer_delta(dst, buf, 1);</span></pre></blockquote></pre>
<br>
seems like it will be off (too small).<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00/src/share/vm/gc/g1/ptrQueue.cpp.frames.html">http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00/src/share/vm/gc/g1/ptrQueue.cpp.frames.html</a><br>
<br>
<pre><span class="changed"> 209 BufferNode* node = BufferNode::make_node_from_buffer(_buf, _index);</span></pre>
Move 209 above 190<br>
<br>
<pre> 190 if (_lock) {
and delete 222?
</pre>
<br>
<pre><span class="changed"> 222 BufferNode* node = BufferNode::make_node_from_buffer(_buf, _index);
</span><span class="changed"></span>
</pre>
Jon<br>
<br>
<br>
<pre><span class="changed"></span></pre>
<div class="moz-cite-prefix">On 03/01/2016 08:00 PM, Kim Barrett
wrote:<br>
</div>
<blockquote
cite="mid:DCFB42AE-4935-43FC-B7F7-C7FCD3DD9DCD@oracle.com"
type="cite">
<blockquote type="cite">
<pre wrap="">On Mar 1, 2016, at 9:56 PM, Kim Barrett <a class="moz-txt-link-rfc2396E" href="mailto:kim.barrett@oracle.com"><kim.barrett@oracle.com></a> wrote:
Please review this change to PtrQueue and its derivatives to maintain
the index in BufferNode where needed. This allowed the removal of
code to fill inactive leading portions of buffers with NULL and to
remove code to skip over NULL entries.
Removed unused DirtyCardQueueSet::apply_closure_to_all_completed_buffers,
rather than fixing it's BufferNode manipulation.
Further changed SATBMarkQueue::filter to use two-fingered compaction,
which may further reduce the number of writes to the buffer during
filtering. For example, using specjbb2015, with over 2.5M buffers
processed, the number of writes using the new two-fingered compaction
(12M) was a factor of 50 fewer than needed by the (non-NULLing)
sliding algorithm (60M), and a factor of 250 fewer than the original
sliding algorithm (330M).
</pre>
</blockquote>
<pre wrap="">
Oops, the relative factors are correct, but the values for the sliding write counts
are wrong. Should be 12M (two-fingered), 600M (new slide) and 3300M (old slide).
</pre>
<blockquote type="cite">
<pre wrap=""> On average, filtering a buffer removed
about 75% of the entries in that test.
CR:
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8150676">https://bugs.openjdk.java.net/browse/JDK-8150676</a>
Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00">http://cr.openjdk.java.net/~kbarrett/8150676/webrev.00</a>
Testing:
JPRT
Aurora ad-hoc defaults + GC nightly + Runtime nightly
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>