review request for CR 6361589

[Tom Rodriguez]

I think the comment should be:

+  // log JavaThread being processed by oops_do

Could you add a vmStructs record for the new field in case we want to look at it later?  Otherwise it looks good.

tom

On Dec 10, 2009, at 2:50 PM, Yumin Qi wrote:

> Tom,
> 
> Thanks, changed as pointed.
> Sorry I forget to include GC group.
> 
> Can someone else give a look?
> 
> http://cr.openjdk.java.net/~minqi/6361589
> 
> 
> 
> Thanks
> Yumin
> 
> Tom Rodriguez wrote:
>> 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
>>>    
>> 
>>  
>