<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Poonam,<br>
<br>
Thanks for the changes.<br>
<br>
What do you think about deleting this part of the<br>
change (line 2759) from the flag processing?<br>
It feels like we're fixing something in two places where<br>
as we only really need the change in <br>
Arguments::set_cms_and_parnew_gc_flags().<br>
<br>
<pre><span class="new"></span><span class="new"><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~poonam/8085965/webrev.01/src/share/vm/runtime/arguments.cpp.frames.html">http://cr.openjdk.java.net/~poonam/8085965/webrev.01/src/share/vm/runtime/arguments.cpp.frames.html</a>
2759 FLAG_SET_CMDLINE(bool, CMSClassUnloadingEnabled, false);</span></pre>
<br>
Jon<br>
<br>
<div class="moz-cite-prefix">On 06/11/2015 11:12 AM, Poonam Bajaj
Parhar wrote:<br>
</div>
<blockquote cite="mid:5579CF72.9070307@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Hello Jon,<br>
<br>
Here's the updated webrev:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epoonam/8085965/webrev.01/">http://cr.openjdk.java.net/~poonam/8085965/webrev.01/</a><br>
<br>
Thanks,<br>
Poonam<br>
<br>
<div class="moz-cite-prefix">On 6/11/2015 6:45 AM, Poonam Bajaj
Parhar wrote:<br>
</div>
<blockquote cite="mid:557990ED.4010105@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hello Jon,<br>
<br>
Thanks for taking a look at the code changes:<br>
<br>
<div class="moz-cite-prefix">On 6/10/2015 6:42 PM, Jon Masamitsu
wrote:<br>
</div>
<blockquote cite="mid:5578E7A3.4040406@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Poonam,<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epoonam/8085965/webrev/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp.frames.html">http://cr.openjdk.java.net/~poonam/8085965/webrev/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp.frames.html</a><br>
<br>
I think it reads more easily if it is coded like this <br>
<br>
if (!ClassUnloading) {<br>
_should_unload_classes = false;<br>
} else if ((_full_gc_requested &&
ExplicitGCInvokesConcurrentAndUnloadsClasses) { <br>
_should_unload_classes = true<br>
} else if (CMSClassUnloadingEnabled) { <br>
// Condition 2.a above 2678<br>
// Disjuncts 2.b.(i,ii,iii) above <br>
_should_unload_classes =
(concurrent_cycles_since_last_unload() >= <br>
CMSClassUnloadingMaxInterval) <br>
||
_cmsGen->is_too_full();<br>
}<br>
<br>
I might not have cut-and-pasted it right but I think you can
see what I mean.<br>
<br>
</blockquote>
I understand, will make this change.<br>
<br>
<blockquote cite="mid:5578E7A3.4040406@oracle.com" type="cite">
Also fix the block comment above to say that ClassUnloading
overides the other<br>
conditions.<br>
<br>
</blockquote>
yes.<br>
<br>
<blockquote cite="mid:5578E7A3.4040406@oracle.com" type="cite">
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epoonam/8085965/webrev/src/share/vm/runtime/arguments.cpp.frames.html">http://cr.openjdk.java.net/~poonam/8085965/webrev/src/share/vm/runtime/arguments.cpp.frames.html</a><br>
<br>
<br>
This catches the case where -Xnoclassgc is used but does not
catch if -XX:-ClassUnloading is used. Yes?<br>
<pre>2752 } else if (match_option(option, "-Xnoclassgc")) {
2753 FLAG_SET_CMDLINE(bool, ClassUnloading, false);
<span class="new">2754 FLAG_SET_CMDLINE(bool, CMSClassUnloadingEnabled, false);</span></pre>
<br>
Instead if you add in "share/vm/runtime/arguments.cpp" to the
method<br>
<br>
1262 void Arguments::set_cms_and_parnew_gc_flags() ;<br>
<br>
if (!ClassUnloading) {<br>
<span class="new">FLAG_SET_CMDLINE(bool,
CMSClassUnloadingEnabled, false);</span> <br>
}<br>
<br>
</blockquote>
Oops! I missed it; will add it.<br>
<br>
Thanks,<br>
Poonam<br>
<br>
<br>
<blockquote cite="mid:5578E7A3.4040406@oracle.com" type="cite">
That should catch both -Xnoclassgc and -XX:-ClassUnloading<br>
<br>
Jon<br>
<br>
<div class="moz-cite-prefix">On 6/10/2015 2:20 PM, Poonam
Bajaj Parhar wrote:<br>
</div>
<blockquote cite="mid:5578AA10.8030302@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
Please review the code changes to fix:<br>
<a moz-do-not-send="true" id="key-val" rel="4786055"
href="https://bugs.openjdk.java.net/browse/JDK-8085965">JDK-8085965:</a>
VM hangs in C2Compiler <br>
<br>
Problem and fix: <br>
In JDK8, CMSClassUnloadingEnabled option which is used to
control the class-unloading in CMS, was enabled by default.
But if the user specifies -Xnoclassgc or -XX:-ClassUnloading
on the command line then classes get unloaded but updating
of subklasses/siblings links gets skipped by the
ClassUnloading check in the following function:<br>
<br>
<span id="mainframespan"><tt><i>void
Klass::clean_weak_klass_links(BoolObjectClosure*
is_alive, bool </i> <br>
<i>clean_alive_klasses) {</i> <br>
<i> if (!ClassUnloading) {</i> <br>
<i> return;</i> <br>
<i> }<br>
<br>
</i></tt></span>which corrupts the class hierarchy links
causing hangs and crashes. <br>
<br>
This fix honors -XX:-ClassUnloading and -Xnoclassgc options
and disables class unloading with CMS.<br>
<br>
Webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epoonam/8085965/webrev/">http://cr.openjdk.java.net/~poonam/8085965/webrev/</a><br>
<br>
Thanks,<br>
Poonam<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>