[PATCH] Added support for fractional values of SafepointTimeoutDelay

[David Holmes]

Hi Wojciech,

Welcome to OpenJDK!

I have filed:

https://bugs.openjdk.org/browse/JDK-8305506

for this issue.  Are you able to create a Pull Request on github for this?

Thanks,
David

On 3/04/2023 5:35 pm, Wojciech KUDLA wrote:
> Hi everyone,
> 
> My organization uses Java to run latency-sensitive workloads; we often engage in performance troubleshooting of the compiled code or the VM itself. We typically avoid STW altogether but when a safepoint happens it's always a sub-millisecond pause. The SafepointTimeoutDelay is great for quickly identifying threads that failed to park themselves in a timely manner and are causing long time-to-safepoint issues but the functionality works with millisecond granularity so doesn't cover our use cases at all. We have a very small patch that introduces sub-millisecond granularity that we'd like to share with the community.
> We have just signed the OCA and this would be our first contribution so we'd appreciate if someone could sponsor this.
> 
> The change is very straightforward but to preserve backwards compatibility we decided to replace the type of SafepointTimeoutDelay with double which I imagine might the topic of various opinions clashing. We're very happy to engage in these discussions and shape the patch to the community's standards.
> Here it is in its entirety:
> 
> ---
>   src/hotspot/share/runtime/globals.hpp             | 7 ++++---
>   src/hotspot/share/runtime/safepoint.cpp           | 4 ++--
>   src/hotspot/share/utilities/globalDefinitions.hpp | 3 +++
>   3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp
> index 483287e6262..d694758829b 100644
> --- a/src/hotspot/share/runtime/globals.hpp
> +++ b/src/hotspot/share/runtime/globals.hpp
> @@ -1289,9 +1289,10 @@ const int ObjectAlignmentInBytes = 8;
>             "(0 means none)")                                                 \
>             range(0, max_jint)                                                \
>                                                                               \
> -  product(intx, SafepointTimeoutDelay, 10000,                               \
> -          "Delay in milliseconds for option SafepointTimeout")              \
> -          range(0, max_intx LP64_ONLY(/MICROUNITS))                         \
> +  product(double, SafepointTimeoutDelay, 10000,                             \
> +          "Delay in milliseconds for option SafepointTimeout; "             \
> +          "supports sub-millisecond resolution with fractional values.")    \
> +          range(0, max_jlongDouble LP64_ONLY(/MICROUNITS))                  \
>                                                                               \
>     product(bool, UseSystemMemoryBarrier, false, EXPERIMENTAL,                \
>             "Try to enable system memory barrier")                            \
> diff --git a/src/hotspot/share/runtime/safepoint.cpp b/src/hotspot/share/runtime/safepoint.cpp
> index 2ff593a0143..7a27aaf804c 100644
> --- a/src/hotspot/share/runtime/safepoint.cpp
> +++ b/src/hotspot/share/runtime/safepoint.cpp
> @@ -379,7 +379,7 @@ void SafepointSynchronize::begin() {
>     if (SafepointTimeout) {
>       // Set the limit time, so that it can be compared to see if this has taken
>       // too long to complete.
> -    safepoint_limit_time = SafepointTracing::start_of_safepoint() + (jlong)SafepointTimeoutDelay * (NANOUNITS / MILLIUNITS);
> +    safepoint_limit_time = SafepointTracing::start_of_safepoint() +
> + (jlong)SafepointTimeoutDelay * NANOSECS_PER_MILLISEC;
>       timeout_error_printed = false;
>     }
>   
> @@ -795,7 +795,7 @@ void SafepointSynchronize::print_safepoint_timeout() {
>           os::naked_sleep(3000);
>         }
>       }
> -    fatal("Safepoint sync time longer than " INTX_FORMAT "ms detected when executing %s.",
> +    fatal("Safepoint sync time longer than " JDOUBLE_FORMAT_P(6) "ms
> + detected when executing %s.",
>             SafepointTimeoutDelay, VMThread::vm_operation()->name());
>     }
>   }
> diff --git a/src/hotspot/share/utilities/globalDefinitions.hpp b/src/hotspot/share/utilities/globalDefinitions.hpp
> index 41ff5150243..1570fde7477 100644
> --- a/src/hotspot/share/utilities/globalDefinitions.hpp
> +++ b/src/hotspot/share/utilities/globalDefinitions.hpp
> @@ -151,6 +151,9 @@ class oopDesc;
>   #define UINTX_FORMAT_X           "0x%"        PRIxPTR
>   #define UINTX_FORMAT_W(width)    "%"   #width PRIuPTR
>   
> +// Format jdouble with defined precision #define
> +JDOUBLE_FORMAT_P(precision) "%." #precision "f"
> +
>   // Format jlong, if necessary
>   #ifndef JLONG_FORMAT
>   #define JLONG_FORMAT             INT64_FORMAT
> --
> 
> Kind regards
> 
> Wojciech Kudla
> HSBC Bank plc
> 
> 
> PUBLIC
> 
> -----------------------------------------
> SAVE PAPER - THINK BEFORE YOU PRINT!
> 
> This E-mail is confidential.
> 
> It may also be legally privileged. If you are not the addressee you may not copy,
> forward, disclose or use any part of it. If you have received this message in error,
> please delete it and all copies from your system and notify the sender immediately by
> return E-mail.
> 
> Internet communications cannot be guaranteed to be timely secure, error or virus-free.
> The sender does not accept liability for any errors or omissions.