review request for CR 6361589

Tom Rodriguez Thomas.Rodriguez at Sun.COM
Wed Dec 9 15:05:34 PST 2009


Overall this looks pretty nice.

java_thread_gc isn't a very clear name.  Maybe collected_thread or processed_thread?  The comment should describe it's purpose more explicitly.

Is this assert really needed?

+    assert(thread != this, "Should not be java thread itself");

I think you should add is_NamedThread() instead of distinguishing all these cases individually.

If VMThread is now a NamedTread why don't you call set_name instead of having it override name again?

vmError.cpp:

I think the printing output should look more like this:

+         st->print_cr("JavaThread " PTR_FORMAT " was being processed during the crash, id = " UINTX_FORMAT, jt, jt->osthread()->thread_id());

tom

On Dec 9, 2009, at 2:49 PM, Yumin Qi wrote:

> Fixed 6361589: Print out stack trace for target thread of GC crash
> 
> http://cr.openjdk.java.net/~minqi/6361589
> 
> Comments: This is a facility to assist for JVM crash analysis. To implement this function, it needs record JavaThread which may involve crash somewhere in GC thread. The place chosen here is NamedThread from which all GC threads but VMThread derive. I change VMThread's base from Thread to NamedThread, so the recording of JavaThread covers for GC threads and VMThread. Once the JavaThread recorded in GC thread seen during error report, its stack trace be printed out. To test this function, add a debug flag CrashGCForDumpingJavaThread, once it is set, it will crash immediately in VMThread or GC Thread to core dump. Note this is for test only purpose, no defects in involved JavaThread.
> 
> 
> Thanks
> Yumin



More information about the hotspot-runtime-dev mailing list