RFR (XS) 8213992: Rename and make DieOnSafepointTimeout the diagnostic option
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Tue Nov 20 16:20:23 UTC 2018
This looks good to me. I like that it's a diagnostic option so we can
use it in product mode.
Coleen
On 11/20/18 11: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
>
>
More information about the hotspot-dev
mailing list