<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Jon,<div class=""><br class=""><div class=""><div><blockquote type="cite" class=""><div class="">On 20 Apr 2015, at 20:15, Jon Masamitsu <<a href="mailto:jon.masamitsu@oracle.com" class="">jon.masamitsu@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class="">
Per,<br class="">
<br class="">
In place of<br class="">
<br class="">
It is not possible to combine the ParNew young collector with <font color="#ff0000" class="">anything other than the CMS collector</font><br class="">
<br class="">
let me suggest<br class="">
<br class="">
It is not possible to combine the ParNew young collector with <font color="#ff0000" class="">any collector other than CMS</font><br class=""></div></div></blockquote><div><br class=""></div><div>Yep, that sounds better. Will fix.</div><br class=""><blockquote type="cite" class=""><div class=""><div bgcolor="#FFFFFF" text="#000000" class="">
<br class="">
Otherwise, still looks good.<br class=""></div></div></blockquote><div><br class=""></div><div>Thanks Jon!</div><div><br class=""></div><div>/Per</div><br class=""><blockquote type="cite" class=""><div class=""><div bgcolor="#FFFFFF" text="#000000" class="">
<br class="">
Jon<br class="">
<br class="">
<div class="moz-cite-prefix">On 04/20/2015 01:28 AM, Per Liden
wrote:<br class="">
</div>
<blockquote cite="mid:5534B89B.7080308@oracle.com" type="cite" class="">Hi
Bengt,
<br class="">
<br class="">
On 2015-04-20 09:45, Bengt Rutisson wrote:
<br class="">
<blockquote type="cite" class="">
<br class="">
Hi Per,
<br class="">
<br class="">
On 2015-04-17 16:05, Per Liden wrote:
<br class="">
<blockquote type="cite" class="">Hi,
<br class="">
<br class="">
When no Use*GC flag is specified on the command-line we make a
default
<br class="">
GC selection. In case we select SerialGC we fail to set
UseSerialGC to
<br class="">
true (unlike when we select ParallelGC where we set
UseParallelGC).
<br class="">
This means we can run with all Use*GC set to false (which
implicitly
<br class="">
means we're using serial). It also means we can't use
UseSerialGC as
<br class="">
the sole indicator that we're using serial.
<br class="">
<br class="">
This patch makes sure that we always set the corresponding
Use*GC for
<br class="">
the GC we have selected.
<br class="">
<br class="">
A typical case where the VM selects serial but fails to set
<br class="">
UseSerialGC is when running a client VM without specifying a
Use*GC
<br class="">
option on the command-line.
<br class="">
<br class="">
Testing: Added a new jtreg test, passes jprt
<br class="">
<br class="">
Webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~pliden/8068582/webrev.0/">http://cr.openjdk.java.net/~pliden/8068582/webrev.0/</a>
<br class="">
Bug: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8068582">https://bugs.openjdk.java.net/browse/JDK-8068582</a>
<br class="">
</blockquote>
<br class="">
The changes look good, but there are a couple of comments in the
code
<br class="">
that would be nice to clean up now that you enforce that
UseSerialGC is
<br class="">
properly set up:
<br class="">
<br class="">
genCollectedHeap.hpp:
<br class="">
<br class="">
virtual bool can_elide_initializing_store_barrier(oop
new_obj) {
<br class="">
// We wanted to assert that:-
<br class="">
// assert(UseSerialGC || UseConcMarkSweepGC,
<br class="">
// "Check can_elide_initializing_store_barrier() for
this
<br class="">
collector");
<br class="">
// but unfortunately the flag UseSerialGC need not
necessarily always
<br class="">
// be set when DefNew+Tenured are being used.
<br class="">
return is_in_young(new_obj);
<br class="">
}
<br class="">
<br class="">
<br class="">
arguments.cpp:
<br class="">
<br class="">
if (UseParNewGC && !UseConcMarkSweepGC) {
<br class="">
// !UseConcMarkSweepGC means that we are using serial old
gc.
<br class="">
Unfortunately we don't
<br class="">
// set up UseSerialGC properly, so that can't be used in
the check
<br class="">
here.
<br class="">
jio_fprintf(defaultStream::error_stream(),
<br class="">
"It is not possible to combine the ParNew young
collector with
<br class="">
the Serial old collector.\n");
<br class="">
return false;
<br class="">
}
<br class="">
<br class="">
<br class="">
In both these cases I think we can just remove the comments. The
<br class="">
suggested assert in can_elide_initializing_store_barrier() is
not
<br class="">
interesting to add in my opinion since it is code in
GenCollectedHeap,
<br class="">
which we know is only used by Serial and CMS.
<br class="">
</blockquote>
<br class="">
Good catch. I will remove both of these.
<br class="">
<br class="">
I'll also adjust the error message there from:
<br class="">
<br class="">
"It is not possible to combine the ParNew young collector with
the Serial old collector.
<br class="">
<br class="">
to:
<br class="">
<br class="">
"It is not possible to combine the ParNew young collector with
anything other than the CMS collector."
<br class="">
<br class="">
Because it doesn't makes sense to mention SerialOld when you say
e.g. -XX:+UseParNewGC -XX:+UseParallelOldGC
<br class="">
<br class="">
Here's an updated webrev:
<br class="">
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~pliden/8068582/webrev.1/">http://cr.openjdk.java.net/~pliden/8068582/webrev.1/</a>
<br class="">
<br class="">
Diff against previous webrev:
<br class="">
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~pliden/8068582/webrev.1-diff/">http://cr.openjdk.java.net/~pliden/8068582/webrev.1-diff/</a>
<br class="">
<br class="">
Thanks for reviewing!
<br class="">
<br class="">
/Per
<br class="">
<br class="">
<blockquote type="cite" class="">
<br class="">
Thanks,
<br class="">
Bengt
<br class="">
<br class="">
<blockquote type="cite" class="">
<br class="">
/Per
<br class="">
</blockquote>
<br class="">
</blockquote>
</blockquote>
<br class="">
</div>
</div></blockquote></div><br class=""></div></div></body></html>