I looked a the code and it seems as though obj_is_alive() started out as a purely debugging/verification<br>method and subsequently as a result of an earlier JMap related bug fix, started being used for this other purpose.<br>
We realized at some subsequent point that this would miss some objects in perm gen, especially in as<br>much as the identification by JMap was concerned and the comments in obj_is_alive() state as much.<br><br>I think though that unfortunately when there is ambiguity about a perm gen object's<br>
liveness, we cannot safely return the answer that it is live because, if it is not, then it may be<br>holding references to other potentially dead objects that the closure might want to mess with. I am not sure what the<br>
right fix here is unless we have some means of restricting what the closure passed to safe_object_iterate()<br>does. In general, it would be dangerous to declare that an arbitrary perm gen object is live without being<br>
certain of it (when class unloading is not enabled), but it may be safe to do so under restricted circumstances<br>-- circumstances perhaps that hold in the way jmap makes use of safe_object_iterate().<br><br>It would seem to me almost as though you want two versions of obj_is_alive() -- (as I indicated below) --<br>
the more conservative one (as is currently the case) for the heap verification, and a second version <br>which corresponds to Kris' suggestion -- for the very restricted case of how the heap dump uses<br>safe_object_iterate() -- for use by safe_object_iterate().<br>
<br>Then we will need to make sure to test both verification with CMS perm gen sweeping disabled<br>as well as Jmap.<br><br>The other alternative is to bite the bullet re the performance impact of doing so,<br>and enable CMS perm gen sweeping / class unloading always, so we can simplify<br>
safe_object_iterate() quite a bit (the only issue to deal with then is for the case when<br>a sweep is in progress when a heap dump wants to occur and we just use the<br>live map in that case).<br><br>Or, may be one wants to let this Jmap anomaly remain and chalk it up as a shortcoming<br>
of jmap when applied to CMS heaps in that they under-report the objects in the perm gen.<br><br>Sorry that i could not be of more help, but may be my ramblings will help inform folks of<br>what may be a proper course here (subject to relative costs and benefits).<br>
<br>thanks!<br>-- ramki<br><br>
<div class="gmail_quote">On Wed, Nov 16, 2011 at 9:57 AM, Srinivas Ramakrishna <span dir="ltr"><<a href="mailto:ysr1729@gmail.com" target="_blank">ysr1729@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It depends on where this is being used. I haven't looked at the code, but if it was being used for verification,<br>
the old answer would be considered more conservative. If on the other had for the identification of object<br>
liveness then the new answer would be more correct. I'll try and take a look at it later today and get<br>
back with comments.<br>
<br>
thanks!<br>
-- ramki<div><div></div><div><br><br><div class="gmail_quote">On Wed, Nov 16, 2011 at 4:55 AM, Krystal Mok <span dir="ltr"><<a href="mailto:rednaxelafx@gmail.com" target="_blank">rednaxelafx@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi all,<div><br></div><div>Comments on bug 7066603, inline below:<br><br><div class="gmail_quote">On Wed, Nov 16, 2011 at 4:05 AM, Tom Rodriguez <span dir="ltr"><<a href="mailto:tom.rodriguez@oracle.com" target="_blank">tom.rodriguez@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>On Nov 14, 2011, at 2:00 PM, Martin Hare Robertson wrote:<br></div><div>> The biggest unresolved problem was reported under bug 7066603. Sadly this seems to have been marked as low priority.<br>
<br></div>It seems to have gotten stuck over in the wrong category. I've moved into the proper category and updated it. It appears CMS isn't visiting some regular Java objects held in perm so they are missing from the dump. The recent changes to move Strings and Classes out of perm make this problem disappear in JDK7. So it only affects JDK6 at this point. I believe the bug is in the obj_is_alive logic that CMS uses to visit only live objects in perm.<br>
</blockquote><div><br></div><div>Looking at the repro example in bug 7066603, I agree that the problem seem to lie in obj_is_alive logic.</div><div><br></div><div>A quick patch to HotSpot 20.0 [1]:</div><div><br></div><div>
<div><font face="'courier new', monospace">diff -r f0f676c5a2c6 src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp</font></div><div><font face="'courier new', monospace">--- a/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp Tue Mar 15 19:30:16 2011 -0700</font></div>
<div><font face="'courier new', monospace">+++ b/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp Wed Nov 16 20:38:29 2011 +0800</font></div><div><font face="'courier new', monospace">@@ -1143,7 +1143,7 @@</font></div>
<div><font face="'courier new', monospace"> return (dead_map->sizeInBits() == 0) // bit_map has been allocated</font></div><div><font face="'courier new', monospace"> || !dead_map->par_isMarked((HeapWord*) p);</font></div>
<div><font face="'courier new', monospace"> } else {</font></div><div><font face="'courier new', monospace">- return false; // We can't say for sure if it's live, so we say that it's dead.</font></div>
<div><font face="'courier new', monospace">+ return true; // We can't say for sure if it's live, so we say that it's alive.</font></div><div><font face="'courier new', monospace"> }</font></div>
<div><font face="'courier new', monospace"> }</font></div><div><font face="'courier new', monospace"> }</font></div></div><div><br></div><div>With this patch, re-run the repro, and jhat doesn't complain any more.</div>
<div><br></div><div>I'm not sure yet if this is the right change. It's interesting to find that in the repro example, all objects missing from the heap dump seem to be Strings referenced by objects in PermGen; the roots are most probably static fields.</div>
<div><br></div><div>Regards,</div><div>Kris Mok</div><div><br></div><div>[1]: <a href="http://hg.openjdk.java.net/hsx/hsx20/master/file/f0f676c5a2c6/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp" target="_blank">http://hg.openjdk.java.net/hsx/hsx20/master/file/f0f676c5a2c6/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.cpp</a></div>
<div> </div><blockquote class="gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
tom</blockquote></div></div>
</blockquote></div><br>
</div></div></blockquote></div><br>