RFR (XS) 8213992: Rename and make DieOnSafepointTimeout the diagnostic option

[Daniel D. Daugherty]

On 11/19/18 6:23 AM, Aleksey Shipilev wrote:
> On 11/19/18 6:08 AM, David Holmes wrote:
>>> David suggested making the option "product", but I don't like it very much: "product" option
>>> suggests this is a production-grade feature and it comes with expectations of support, which is
>>> interesting in itself when product feature crashes the VM. "diagnostic" keeps this mode available in
>>> product builds without the attached notion of support. Users that want fail-fast VM crash can then
>>> use that option on "we know what we are doing" basis.
>> I feel a little uncomfortable that "diagnostic" is being used as a synonym for "unsupported
>> product". If you really want to fail-fast in production then you're looking for a production flag
>> not a "diagnostic" one IMHO. I won't fight it but I'd like to know what others think.
> Yup, let's hear it. I don't want to bikeshed this too much :)

If I was doing the work on this bug, I would have made it a diagnostic 
option
also. I have a bias towards adding options as diagnostic for two reasons:

1) Easier to get rid of the option if it's a bad idea.
2) You have to use the '-XX:UnlockDiagnosticVMOption' option to use it
    which makes unintentional use more difficult.

Dan


> To re-iterate: I don't really want to introduce the hard-to-retract product option. Rather, I would
> like to expose the flag that we are using for debugging into release builds, so it could both used
> by us for our release-mode debugging, and by power users who know what they are doing. It seems
> "diagnostic" hits the spot here. Also, symmetry against other Abort* flags...
>
>>> Fix:
>>>
>>> diff -r 9ad663e63da5 -r 132db6e99f77 src/hotspot/share/runtime/globals.hpp
>>> --- a/src/hotspot/share/runtime/globals.hpp    Fri Nov 16 12:02:08 2018 +0100
>>> +++ b/src/hotspot/share/runtime/globals.hpp    Fri Nov 16 13:35:16 2018 +0100
>>> @@ -498,7 +498,7 @@
>>>              "Time out and warn or fail after SafepointTimeoutDelay "          \
>>>              "milliseconds if failed to reach safepoint")                      \
>>>                                                                                \
>>> -  develop(bool, DieOnSafepointTimeout, false,                               \
>>> +  diagnostic(bool, AbortVMOnSafepointTimeout, false,                        \
>>>              "Die upon failure to reach safepoint (see SafepointTimeout)")     \
>> s/Die/Abort
>>
>> Thanks.
> Done in my local patch queue, thanks!
>
> -Aleksey
>
>