RFR: [11u, 12u] 8219584: Try to dump error file by thread which causes safepoint timeout

[Doerr, Martin]

Hi Götz,

an incremental webrev sounds like a good idea. I'll try to remember that next time.
Thank you for the review.

Best regards,
Martin


-----Original Message-----
From: Lindenmaier, Goetz 
Sent: Montag, 18. März 2019 12:55
To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-dev at openjdk.java.net
Cc: jdk-updates-dev at openjdk.java.net
Subject: RE: RFR: [11u, 12u] 8219584: Try to dump error file by thread which causes safepoint timeout

Hi, 

the change looks good, the adaptions make sense.

I'd appreciate a webrev next time ... maybe an incremental
one in this case as the other applies clean (checked :)).

Best regards,
  Goetz.

> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net> On
> Behalf Of Doerr, Martin
> Sent: Freitag, 15. März 2019 13:03
> To: hotspot-runtime-dev at openjdk.java.net
> Cc: jdk-updates-dev at openjdk.java.net
> Subject: [CAUTION] RFR: [11u, 12u] 8219584: Try to dump error file by thread
> which causes safepoint timeout
> 
> Hi,
> 
> I'd like to backport this change to 11u and 12u for 2 reasons:
> 1. This feature can detect what causes safepoint timeouts.
> 2. Enable other backports. JDK-8220374 contains a new test which requires
> this change.
> 
> https://bugs.openjdk.java.net/browse/JDK-8219584
> 
> 
> It applies cleanly, but I'd like to make a functional adaptation because of
> "8203469: Faster safepoints" which is only in jdk13.
> The timeout condition changed and it should be the same as a few lines above
> in the code.
> 
> Please review this adaptation:
> 
> 11u/12u:
> diff -r 20d87ac7aa4d src/hotspot/share/runtime/safepoint.cpp
> --- a/src/hotspot/share/runtime/safepoint.cpp   Fri Mar 08 11:23:30 2019
> +0100
> +++ b/src/hotspot/share/runtime/safepoint.cpp   Wed Mar 13 17:44:48 2019
> +0100
> @@ -997,7 +997,10 @@
>    if (AbortVMOnSafepointTimeout) {
>      // Send the blocking thread a signal to terminate and write an error file.
>      for (JavaThreadIteratorWithHandle jtiwh; JavaThread *cur_thread =
> jtiwh.next(); ) {
> -      if (cur_thread->safepoint_state()->is_running()) {
> +      ThreadSafepointState *cur_state = cur_thread->safepoint_state();
> +      if (cur_thread->thread_state() != _thread_blocked &&
> +          ((reason == _spinning_timeout && cur_state->is_running()) ||
> +             (reason == _blocking_timeout && !cur_state->has_called_back()))) {
>          if (!os::signal_thread(cur_thread, SIGILL, "blocking a safepoint")) {
>            break; // Could not send signal. Report fatal error.
>          }
> 
> 
> In addition, jdk 11u does not support compiler availability checks,
> so I'd like to add the following part of "8207965: C2-only debug build fails".
> Please review this addition for 11u:
> 
> --- a/test/hotspot/jtreg/TEST.ROOT      Fri Mar 08 11:23:30 2019 +0100
> +++ b/test/hotspot/jtreg/TEST.ROOT      Thu Mar 14 14:30:59 2019 +0100
> @@ -62,6 +62,8 @@
>      vm.cds.custom.loaders \
>      vm.cds.archived.java.heap \
>      vm.graal.enabled \
> +    vm.compiler1.enabled \
> +    vm.compiler2.enabled \
>      docker.support
> 
> # Minimum jtreg version
> diff -r 4840f7c74108 test/jdk/TEST.ROOT
> --- a/test/jdk/TEST.ROOT        Fri Mar 08 11:23:30 2019 +0100
> +++ b/test/jdk/TEST.ROOT        Thu Mar 14 13:51:09 2019 +0100
> @@ -39,6 +39,8 @@
>      java.runtime.name \
>      vm.gc.Z \
>      vm.graal.enabled \
> +    vm.compiler1.enabled \
> +    vm.compiler2.enabled \
>      vm.cds \
>      vm.hasSA \
>      vm.hasSAandCanAttach \
> diff -r 4840f7c74108 test/jtreg-ext/requires/VMProps.java
> --- a/test/jtreg-ext/requires/VMProps.java      Fri Mar 08 11:23:30 2019 +0100
> +++ b/test/jtreg-ext/requires/VMProps.java      Thu Mar 14 13:51:09 2019
> +0100
> @@ -92,6 +92,8 @@
>          map.put("vm.cds.archived.java.heap", vmCDSForArchivedJavaHeap());
>          // vm.graal.enabled is true if Graal is used as JIT
>          map.put("vm.graal.enabled", isGraalEnabled());
> +        map.put("vm.compiler1.enabled", isCompiler1Enabled());
> +        map.put("vm.compiler2.enabled", isCompiler2Enabled());
>          map.put("docker.support", dockerSupport());
>          map.put("release.implementor", implementor());
>          vmGC(map); // vm.gc.X = true/false
> @@ -390,6 +392,23 @@
>          return Compiler.isGraalEnabled() ? "true" : "false";
>      }
> 
> +    /**
> +     * Check if Compiler1 is present.
> +     *
> +     * @return true if Compiler1 is used as JIT compiler, either alone or as part
> of the tiered system.
> +     */
> +    protected String isCompiler1Enabled() {
> +        return Compiler.isC1Enabled() ? "true" : "false";
> +    }
> +
> +    /**
> +     * Check if Compiler2 is present.
> +     *
> +     * @return true if Compiler2 is used as JIT compiler, either alone or as part
> of the tiered system.
> +     */
> +    protected String isCompiler2Enabled() {
> +        return Compiler.isC2Enabled() ? "true" : "false";
> +    }
> 
>     /**
>       * A simple check for docker support
> 
> 
> Best regards,
> Martin