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