<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<div class="moz-cite-prefix">On 2013-11-05 01:33, Jon Masamitsu
wrote:<br>
</div>
<blockquote cite="mid:52783CF3.6040006@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<br>
<div class="moz-cite-prefix">On 11/4/2013 1:02 AM, Bengt Rutisson
wrote:<br>
</div>
<blockquote cite="mid:527762B7.4060605@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<br>
Hi Jon,<br>
<br>
<div class="moz-cite-prefix">On 2013-11-02 01:02, Jon Masamitsu
wrote:<br>
</div>
<blockquote cite="mid:52744109.5030302@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
<big>8024483 - assertion failure: (!mirror_alive ||
loader_alive) failed<br>
<br>
The default value of _should_unload_classes in CMSCollector
is false.<br>
The default value of CMSClassUnloadingEnabled is true
(changed in jdk8)<br>
and is used to set the root scanning options. This
inconsistency leads to<br>
dead class loaders without unloading classes.<br>
<br>
The fix for 8024954 fixes (or works around) this
inconsistency by resetting <br>
the root scanning options based on _should_unload_classes.<br>
<br>
Additionally there is a missing call to
update_should_unload_classes() in<br>
the CMS foreground collector. Adding the call allows the
test for<br>
</big><big>CMSClassUnloadingEnabled to pass but may not be a
complete fix.<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/8024483/webrev.00/">http://cr.openjdk.java.net/~jmasa/8024483/webrev.00/</a><br>
</big></blockquote>
<br>
Looks good.<br>
<br>
<blockquote cite="mid:52744109.5030302@oracle.com" type="cite"><big>
<br>
</big><big> <br>
8024954: CMS: CMSClassUnloadingMaxInterval is not
implemented correctly<br>
<br>
For 8024954 the classes used as roots was being set at
initialization based on<br>
the value of <span class="removed">CMSClassUnloadingEnabled</span>.
For a value of CMSClassUnloadingMaxInterval<br>
greater than 1, the roots need to be set dynamically <br>
(done in
CMSCollector::setup_cms_unloading_and_verification_state
with this fix).<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejmasa/8024954/webrev.01/">http://cr.openjdk.java.net/~jmasa/8024954/webrev.01/</a></big>
</blockquote>
<br>
<br>
Looks good. One minor question. In CMSCollector::CMSCollector()
you left this line:<br>
<br>
792 add_root_scanning_option(SharedHeap::SO_SystemClasses);<br>
<br>
But in
CMSCollector::setup_cms_unloading_and_verification_state() we
now always set up the state correctly to be either SO_AllClasses
or SO_SystemClasses. So, do we need that initilization in
CMSCollector::CMSCollector()?<br>
</blockquote>
<br>
I tested both with and without it and it is not needed as far as I
can tell.<br>
I left it in because I know that SO_None is always wrong (as set
by the<br>
constructor). I didn't want to try and figure out what the best
default<br>
was based on the other flags because that was the source of this
bug.<br>
If you think leaving it initialized to SO_None makes the code more
readable,<br>
I can remove it. <br>
</blockquote>
<br>
I prefer to just use SO_None as default since I think that setting
it to SharedHeap::SO_SystemClasses just gives the false impression
that this is what we want to use. But I'm also ok with the change as
it is. So, I'll leave it up to you to decide.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:52783CF3.6040006@oracle.com" type="cite"> <br>
Jon<br>
<br>
<blockquote cite="mid:527762B7.4060605@oracle.com" type="cite"> <br>
Thanks,<br>
Bengt<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>