<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 10/2/15 8:54 AM, Thomas Schatzl
wrote:<br>
</div>
<blockquote cite="mid:1443790456.2160.25.camel@oracle.com"
type="cite">
<pre wrap="">Hi all,
can I have reviews for this very-small fix for a bug introduced in
JDK-7097567?
Instead of using
CollectorState::last_young_gc() && !CollectorState::in_marking_window()
to determine when to update the survivor rate group predictors (which
means "only the gc between GC cleanup and mixed GC"), G1 needs to do
this for all young GCs using
CollectorState::last_gc_was_young() && !
CollectorState::in_marking_window()
which means just during young-only GCs outside of marking.
The original code in JDK-7097567 looks like
- bool propagate = _last_gc_was_young && !_in_marking_window;
+ bool propagate = collector_state()->should_propagate();
(<a class="moz-txt-link-freetext" href="http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/23cc50392e04/src/share/vm/gc/g1/g1CollectorPolicy.hpp">http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/23cc50392e04/src/share/vm/gc/g1/g1CollectorPolicy.hpp</a>)
where CollectorState::should_propagate() has been wrongly changed to:
bool should_propagate() { // XXX should have a more suitable state name or abstraction for this
return (_last_young_gc && !_in_marking_window);
}
</pre>
</blockquote>
Ahh, when I ported Ramki's original patch he had:<br>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
return (_last_young_gc_full && !_in_marking_window);<br>
<br>
But the variable "_last_young_gc_full" no longer exists. I
miss-updated that to "_last_young_gc", and didn't catch it in the
diff.<br>
<br>
I really need to rename these variables like I've been threatening
to. They are very confusing!<br>
<br>
(r)eviewed.<br>
<blockquote cite="mid:1443790456.2160.25.camel@oracle.com"
type="cite">
<pre wrap="">
(<a class="moz-txt-link-freetext" href="http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/23cc50392e04/src/share/vm/gc/g1/g1CollectorState.hpp">http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/23cc50392e04/src/share/vm/gc/g1/g1CollectorState.hpp</a>)
It is based on the changes for 8138750 which is also currently out for
review.
CR:
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8138752">https://bugs.openjdk.java.net/browse/JDK-8138752</a>
Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tschatzl/8138752/webrev/">http://cr.openjdk.java.net/~tschatzl/8138752/webrev/</a>
Testing:
local compilation
Thanks,
Thomas
</pre>
</blockquote>
<br>
</body>
</html>