RFR: 8271931: Make AbortVMOnVMOperationTimeout more resilient to OS scheduling

[Albert Mingkun Yang]

On Thu, 5 Aug 2021 10:54:42 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Note the fatal msg is different: on VM thread we know the VM-op has completed, but on TimeoutTask the VM-op status is unknown.
>> 
>> How about sth like this?
>> 
>> 
>> bool check_timeout(jlong& delay) {
>>   delay = nanos_to_millis(os::javaTimeNanos() - _arm_time);
>>   return delay > AbortVMOnVMOperationTimeoutDelay
>> }
>> 
>> void disarm() {
>>   Atomic::release_store_fence(&_armed, 0);
>>   
>>   jlong delay;
>>   if (check_timeout(delay)) {
>>     fatal("%s VM operation took too long: completed in " JLONG_FORMAT " ms (timeout: " INTX_FORMAT " ms)",
>>         _cur_vm_operation->name(), delay, AbortVMOnVMOperationTimeoutDelay);
>>   }
>> }
>
> Huh, I missed this difference. Yeah, that goes in the right direction. Can we just pass the `const char*` with the format string to checking method? So we don't do the out-parameter, and the code ends up being:
> 
> 
> bool maybe_fail_with(const char* fail_msg) {
>   jlong delay = nanos_to_millis(os::javaTimeNanos() - _arm_time);
>   if (delay > AbortVMOnVMOperationTimeoutDelay) {
>      fatal(fail_msg, delay, AbortVMOnVMOperationTimeoutDelay);
>   }
> }
> 
> void disarm() {
>   Atomic::release_store_fence(&_armed, 0);
>   maybe_fail_with("VM operation took too long: completed in " JLONG_FORMAT " ms (timeout: " INTX_FORMAT " ms)");
> }

I actually think having the format string and the variables in the same scope/statement is better, because one can match the format specifier with the corresponding variable easily.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5016