review request for CR 6361589

[David Holmes - Sun Microsystems]

Hi Yumin,

Can you update the thread hierarchy comments in thread.hpp:

   49 // Class hierarchy
   50 // - Thread
   51 //   - VMThread
   52 //   - JavaThread
   53 //   - WatcherThread

This seems incomplete anyway so it would be good to update it.

I would also find it useful to add more comments to the NamedThread 
class explaining the role of this new field. It really has nothing to do 
with being a "named thread" - but then NamedThread should really have 
been called something more general. Pity we can't rename it - can we? :)

In vmError.cpp:

506      // printing Java thread stack trace if it is involved in GC crash
  507
  508      if (_verbose && (_thread->is_Named_thread())) {

Can/Should this be a new STEP?

David

Yumin Qi said the following on 12/11/09 08:50:
> 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
>>>     
>>
>>   
>