<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Mikael,<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp.frames.html">http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp.frames.html</a><br>
<br>
I don't mean to expand this task but the comment after the parameter<br>
"do_code_roots" caught my eye.<br>
<br>
<blockquote type="cite">
<pre>3736 gch->gen_process_strong_roots(_cmsGen->level(),
3737 true, // younger gens are roots
3738 true, // activate StrongRootsScope
3739 SharedHeap::ScanningOption(roots_scanning_options()),
3740 ¬Older,
<span class="changed">3741 true, <font color="#ff0000">// walk all of code cache if (so & SO_AllCodeCache)</font></span>
3742 NULL,
3743 &klass_closure);</pre>
</blockquote>
<br>
Did you consider whether the "do_code_roots" is needed any more?<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0/src/share/vm/memory/sharedHeap.cpp.frames.html">http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0/src/share/vm/memory/sharedHeap.cpp.frames.html</a><br>
<br>
<blockquote type="cite">
<pre><span class="changed"> 208 if (so & SO_ScavengeCodeCache) {</span>
209 assert(code_roots != NULL, "must supply closure for code cache");
210
211 // We only visit parts of the CodeCache when scavenging.
212 CodeCache::scavenge_root_nmethods_do(code_roots);
<span class="changed"> 213 }</span>
<span class="changed"> 214 if (so & SO_AllCodeCache) {</span>
<span class="changed"> 215 assert(code_roots != NULL, "must supply closure for code cache");</span>
<span class="changed"> 216 </span>
217 // CMSCollector uses this to do intermediate-strength collections.
218 // We scan the entire code cache, since CodeCache::do_unloading is not called.
219 CodeCache::blobs_do(code_roots);
220 }</pre>
</blockquote>
<br>
The new code allows for scavenge_root_nmethods_do() and blobs_do()
both to be called<br>
(depending on the contents of "so"). That could not happen with the
old code. Is that<br>
OK? I didn't read enough code to know if blobs_do would also
process the same roots<br>
again as scavenge_root_nmethods_do().<br>
<br>
That's all. Rest looks good. Nice change.<br>
<br>
Jon<br>
<br>
<br>
<br>
<br>
<br>
<div class="moz-cite-prefix">On 1/21/2014 5:13 AM, Mikael Gerdin
wrote:<br>
</div>
<blockquote cite="mid:19322104.tGMWz1WVDH@mgerdin03" type="cite">
<pre wrap="">Hi all,
as a part of implementing JEP-156 (G1 class unloading) we are doing some
cleanups in order to refactor the strong root processing code.
Currently there are two "dimensions" in which SharedHeap::process_strong_roots
is configured. One is the ScanOption enum, the other is the is_scavenging
booelan. The semantic meaning of is_scavenging can easily be folded into the
ScanOption enum by:
* Introducing a SO_AllCodeCache/SO_ScavengeCodeCache distinction (in a way
similar to SO_AllClasses/SO_SystemClasses)
* Noting that passing a CLD closure to Threads::oops_do is only needed when we
want to determine the precise liveness of classes, this is already signaled by
SO_SystemClasses.
Bug link: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8032379">https://bugs.openjdk.java.net/browse/JDK-8032379</a>
Webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0">http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0</a>
Thanks
/Mikael
</pre>
</blockquote>
<br>
</body>
</html>