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

[Aleksey Shipilev]

Pushed. Thanks everyone!

-Aleksey

On 11/20/18 10:10 PM, David Holmes wrote:
> Okay I guess we're redefining what "diagnostic" means :)
> 
> Cheers,
> David
> 
> On 21/11/2018 2:17 am, Aleksey Shipilev wrote:
>> On 11/19/18 3:43 PM, Daniel D. Daugherty wrote:
>>> 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.
>>
>> Thanks. No further comments? David, are you happy? I am pushing this:
>>
>> diff -r 0811cc0ab16e src/hotspot/share/runtime/globals.hpp
>> --- a/src/hotspot/share/runtime/globals.hpp     Tue Nov 20 16:18:12 2018 +0100
>> +++ b/src/hotspot/share/runtime/globals.hpp     Tue Nov 20 17:17:35 2018 +0100
>> @@ -496,12 +496,12 @@
>>                                                                               \
>>     product(bool, SafepointTimeout, false,                                    \
>>             "Time out and warn or fail after SafepointTimeoutDelay "          \
>>             "milliseconds if failed to reach safepoint")                      \
>>                                                                               \
>> -  develop(bool, DieOnSafepointTimeout, false,                               \
>> -          "Die upon failure to reach safepoint (see SafepointTimeout)")     \
>> +  diagnostic(bool, AbortVMOnSafepointTimeout, false,                        \
>> +          "Abort upon failure to reach safepoint (see SafepointTimeout)")   \
>>                                                                               \
>>     /* 50 retries * (5 * current_retry_count) millis = ~6.375 seconds */      \
>>     /* typically, at most a few retries are needed                    */      \
>>     product(intx, SuspendRetryCount, 50,                                      \
>>             "Maximum retry count for an external suspend request")            \
>> diff -r 0811cc0ab16e src/hotspot/share/runtime/safepoint.cpp
>> --- a/src/hotspot/share/runtime/safepoint.cpp   Tue Nov 20 16:18:12 2018 +0100
>> +++ b/src/hotspot/share/runtime/safepoint.cpp   Tue Nov 20 17:17:35 2018 +0100
>> @@ -976,13 +976,13 @@
>>         }
>>         ls.print_cr("# SafepointSynchronize::begin: (End of list)");
>>       }
>>     }
>>
>> -  // To debug the long safepoint, specify both DieOnSafepointTimeout &
>> +  // To debug the long safepoint, specify both AbortVMOnSafepointTimeout &
>>     // ShowMessageBoxOnError.
>> -  if (DieOnSafepointTimeout) {
>> +  if (AbortVMOnSafepointTimeout) {
>>       fatal("Safepoint sync time longer than " INTX_FORMAT "ms detected when executing %s.",
>>             SafepointTimeoutDelay, VMThread::vm_safepoint_description());
>>     }
>>   }
>>
>>
>> Thanks,
>> -Aleksey
>>
>>