RFR: 8027429: Add diagnostic command VM.info to get hs_err print-out

[David Holmes]

On 29/10/2015 11:36 PM, Mattis Castegren wrote:
> Some background: We have this command in JRockit. The information you gather when you crash to give a good summary of what system you run on is pretty much exactly the information you need to get a good summary on a system that has not crashed. The JRockit command is extremely useful for support, and saves a lot of work going back and forth asking about system information.
>
> Also, if we write something new in the hs_err file, like if there has been any out of memory errors, we often would want the same information in the VM.info output. From my experience in Sustaining/Support, I can't think of any information you would want in VM.info that you wouldn't also want in the hs_err file and the other way around, apart from details about the crash (obviously).
>
> I don't see a reason to exactly enumerate what information VM.info should provide. From a sustaining/support perspective, we want a one-stop command to gather as much useful information as possible, which is the same idea we have for the hs_err file.

The reason to enumerate what is required is to see where that 
information already exists and can be collected from. The VMError report 
has to be very careful about what it does and how it does it because of 
the fact we may have crashed and the general process state is 
indeterminate. A Dcmd can simply gather whatever information is required 
from the available sources in whatever order or format desired.

I have no problem with this command, just how it has been proposed to 
implement it.

David

> Kind Regards
> /Mattis
>
> -----Original Message-----
> From: David Holmes
> Sent: den 29 oktober 2015 13:08
> To: Coleen Phillimore; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR: 8027429: Add diagnostic command VM.info to get hs_err print-out
>
> On 29/10/2015 10:02 PM, Coleen Phillimore wrote:
>>
>>
>> On 10/29/15 7:58 AM, David Holmes wrote:
>>> On 29/10/2015 9:26 PM, Coleen Phillimore wrote:
>>>>
>>>> So I actually disagree.  I don't think there should be an additional
>>>> separate mechanism to get the same information that we get with
>>>> hs_err reporting.  I've wanted to have a feature like this for a long time.
>>>>
>>>> I pre-reviewed this change and I thought it looked good in general.
>>>> I didn't see the thread iteration problem that Fred refers to below,
>>>> but I think the individual problems can be fixed.
>>>>
>>>> The last thing I want is this code to be copied somewhere else.
>>>
>>> Factored as needed not copied. VMError is not an "info" reporting
>>> mechanism.
>>
>> If you look at the things that are reported in each "STEP", there's a
>> small amount of code and the order is important.
>
> The order is less important in an "info" request I would think.
>
>> I'd like the vm info to use the same order and report what it can do
>> safely.   Refactoring 5 lines of code into functions doesn't make sense.
>
> I need to consider exactly what it is the "info" needs to report in more detail. There are existing facilities (system properties, management
> APIs) for various bits of runtime information, which VMError can't utilize but a Dcmd can.
>
> David
>
>> Coleen
>>
>>>
>>> David
>>>
>>>> Coleen
>>>>
>>>> On 10/28/15 8:48 PM, David Holmes wrote:
>>>>> I agree with Fred, this kind of info reporting should not be
>>>>> piggy-backed onto VMError handling for the reasons stated (and the
>>>>> error handling logic is complicated enough as it is!). For things
>>>>> like thread lists there are already safe management functions that
>>>>> can/should be used.
>>>>>
>>>>> Thanks,
>>>>> David H.
>>>>>
>>>>> On 29/10/2015 3:29 AM, Frederic Parain wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> I haven't review all the code yet, but I'm concerned with the fact
>>>>>> that the diagnostic command is calling VMError::report().
>>>>>> This method has been implemented to be executed in the particular
>>>>>> context of fatal errors, and its usage while the VM is running
>>>>>> normally seems dangerous.
>>>>>>
>>>>>> For instance, VMError::report() consciously avoids grabbing locks
>>>>>> because of the risk of deadlock during the error reporting.
>>>>>> The consequence is that some data structures are browsed in an
>>>>>> unsafe way. One example: VMError::report() calls
>>>>>> Threads::print_on_error() which iterates over the thread list
>>>>>> *without owning the Threads_lock*.
>>>>>>
>>>>>> The implementation of the diagnostic command seems also to exclude
>>>>>> a lot of reporting from the initial VMError::report() method. Have
>>>>>> you considered implementing a new MT-safe reporting method rather
>>>>>> than trying to modify the special VMError::report() methods? (Note
>>>>>> that some code factorization between VMError::report() and this
>>>>>> new method should be possible).
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Fred
>>>>>>
>>>>>> On 28/10/2015 17:18, david buck wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>> Please review my change for this small enhancement.
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8027429
>>>>>>> webrev: http://cr.openjdk.java.net/~dbuck/8027429_jdk9_01/
>>>>>>>
>>>>>>> Cheers,
>>>>>>> -Buck
>>>>>>
>>>>
>>