I tend to agree with David that it's cleaner to have collect_as_vm_thread() return an indication of<br>whether GC was done or not, and for the caller to then make the heap parsable if GC was skipped<br>and the plan is to subsequently walk the heap regardless.<br>
<br>I also appreciate that what Poonam proposed may be the short-term expedient fix, and in that<br>case the fix looks OK to me as a temporary solution (i can imagine that with very large Edens and<br>many, many threads on a highly multi-core server just walking the threads twice is an unnecessary<br>
cost); reviewed.<br><br>thanks.<br>- ramki (openjdk: ysr)<br><br><div class="gmail_quote">On Thu, Nov 10, 2011 at 11:36 PM, David Holmes <span dir="ltr"><<a href="mailto:david.holmes@oracle.com">david.holmes@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">As a short-term quick-fix I'm okay with this but I defer to the GC folks. If similar code is already being used then this seems reasonable.<br>
<br>
David<div><div></div><div class="h5"><br>
<br>
On 11/11/2011 5:14 PM, <a href="mailto:poonam.bajaj@oracle.com" target="_blank">poonam.bajaj@oracle.com</a> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div><div></div><div class="h5">
Hi David,<br>
<br>
On 11/11/2011 11:48 AM, David Holmes wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Hi Poonam,<br>
<br>
I've cc'ed GC-dev as this is more their area. It seems very wasteful<br>
to potentially ensure_parsability and also do a full GC. And the check<br>
for the active GC_Locker is somewhat arbitrary as it could change the<br>
instant you've queried it.<br>
<br>
</blockquote>
Thanks for adding GC-dev.<br>
<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
It seems to me that a more complete solution here would enable<br>
collect_as_vm_thread to indicate whether or not GC occurred. Which in<br>
turn requires that do_full_collection report that info - a RFE that is<br>
already noted in the G1 code at least:<br>
<br>
void G1CollectedHeap::do_full_<u></u>collection(bool clear_all_soft_refs) {<br>
// do_collection() will return whether it succeeded in performing<br>
// the GC. Currently, there is no facility on the<br>
// do_full_collection() API to notify the caller than the collection<br>
// did not succeed (e.g., because it was locked out by the GC<br>
// locker). So, right now, we'll ignore the return value.<br>
bool dummy = do_collection(true, /* explicit_gc */<br>
clear_all_soft_refs,<br>
0 /* word_size */);<br>
}<br>
<br>
</blockquote>
Yes, that would be the ideal solution.<br>
<br>
What is the time frame of having this RFE implemented for other<br>
collectors? This bug was discovered during the FMW stress testing and<br>
needs to fixed as soon as we can.<br>
<br>
The solution I proposed is being used by other VM operations as well<br>
e.g. VM_GC_HeapInspection and was somehow missed for VM_HeapDumper.. So,<br>
I guess until we have the RFE implemented that allows us to have the<br>
result whether a GC succeeded or not, this could be a short term solution.<br>
<br>
<br>
Thanks,<br>
Poonam<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
David<br>
-----<br>
<br>
On 11/11/2011 3:20 PM, <a href="mailto:poonam.bajaj@oracle.com" target="_blank">poonam.bajaj@oracle.com</a> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Hi,<br>
<br>
Can I get a couple of reviews for this change:<br>
<br>
CR 7110428: Crash during HeapDump operation<br>
<a href="http://monaco.sfbay.sun.com/detail.jsf?cr=7110428" target="_blank">http://monaco.sfbay.sun.com/<u></u>detail.jsf?cr=7110428</a><br>
<br>
The crash happens when VM thread traverses the heap during heapdump<br>
operation and it finds the heap in non-parseable state. The heap can be<br>
left in non-parseable state if due to some reason GC is skipped and we<br>
didn't call ensure_parsability() either before dumping the heap. The<br>
solution is to always call ensure_parsability() In<br>
VM_HeapDumper::doit(), whether or not a GC is requested before the heap<br>
dumping.<br>
<br>
<a href="http://cr.openjdk.java.net/%7Epoonam/7110428/webrev.00/" target="_blank">http://cr.openjdk.java.net/~<u></u>poonam/7110428/webrev.00/</a><br>
<br>
<br>
Thanks,<br>
Poonam<br>
<br>
<br>
</blockquote></blockquote>
<br>
--<br>
Best regards, Poonam<br>
<br></div></div>
Oracle <<a href="http://www.oracle.com" target="_blank">http://www.oracle.com</a>><div class="im"><br>
Poonam Bajaj | Principal Member of Technical Staff<br></div>
Phone: <a href="tel:%2B91%2080%2066937451" value="+918066937451" target="_blank">+91 80 66937451</a> <tel:+91%2080%<a href="tel:2066937451" value="+12066937451" target="_blank">2066937451</a>> | Mobile: +91<br>
9844511366 <tel:+91%209844511366><div class="im"><br>
Oracle JVM Sustaining Engineering<br>
<br>
ORACLE India Bangalore<br></div>
Green Oracle <<a href="http://www.oracle.com/commitment" target="_blank">http://www.oracle.com/<u></u>commitment</a>> Oracle is committed to<div class="im"><br>
developing practices and products that help protect the environment<br>
<br>
</div></blockquote>
</blockquote></div><br>