OpenJDK › OpenJDK Hotspot › OpenJDK Hotspot Compiler Development List

RFR: 8262011: [JVMCI] allow printing to tty from unattached libgraal thread

Classic

List

Threaded

5 messages Options

Doug Simon

RFR: 8262011: [JVMCI] allow printing to tty from unattached libgraal thread

Currently, `HotSpotJVMCIRuntime.writeDebugOutput` does nothing if the current thread is not attached to HotSpot (i.e., `Thread::current_or_null() == NULL`). This means crucial debug info can be lost. For reference, an unattached libgraal thread is a thread started from within libgraal that has not yet attached itself to the VM (e.g., before [this line](https://github.com/oracle/graal/blob/e4b9ab931940e1946f96f2015b937ba100384573/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java#L42)) or has already detached itself (e.g., after [this line](https://github.com/oracle/graal/blob/e4b9ab931940e1946f96f2015b937ba100384573/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java#L46)).

The reason for the current behavior is that `HotSpotJVMCIRuntime.writeDebugOutput` passes a Java byte array to C++ code and the C++ code calls back into Java to decode the byte array into a native buffer. These call backs require the current thread to be attached to the VM.

This PR moves the Java-to-native-buffer decoding into Java and thus avoids the requirement for the current thread to be attached to the VM.

Tested in libgraal by patching Graal as follows:
diff --git a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java
index 36064767c95..352395dd59b 100644
--- a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java
+++ b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java
@@ -43,7 +43,14 @@ public class GraalServiceThread extends Thread {
try {
runnable.run();
} finally {
+ String debug = System.getenv("GraalServiceThread.debug");
afterRun();
+ if ("true".equals(debug)) {
+ throw new InternalError("THROWN AFTER DETACHING");
+ }
}
}

Running without the changes in this PR:
> env GraalServiceThread.debug=true java -jar dacapo.jar avrora
===== DaCapo 9.12 avrora starting =====
===== DaCapo 9.12 avrora PASSED in 4270 msec =====

Running with the changes in this PR:
> env GraalServiceThread.debug=true java -jar dacapo.jar avrora
===== DaCapo 9.12 avrora starting =====
Exception in thread "LibGraalHotSpotGraalManagement-init" java.lang.InternalError: THROWN AFTER DETACHING
at org.graalvm.compiler.core.GraalServiceThread.run(GraalServiceThread.java:52)
at com.oracle.svm.core.thread.JavaThreads.threadStartRoutine(JavaThreads.java:519)
at com.oracle.svm.core.posix.thread.PosixJavaThreads.pthreadStartRoutine(PosixJavaThreads.java:192)
===== DaCapo 9.12 avrora PASSED in 4688 msec =====

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

Commit messages:
- 8262011: [JVMCI] allow printing to tty from unattached libgraal thread

Changes: https://git.openjdk.java.net/jdk/pull/2640/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2640&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8262011
Stats: 322 lines in 8 files changed: 49 ins; 246 del; 27 mod
Patch: https://git.openjdk.java.net/jdk/pull/2640.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2640/head:pull/2640

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

Doug Simon

Re: RFR: 8262011: [JVMCI] allow printing to tty from unattached libgraal thread

On Fri, 19 Feb 2021 10:10:17 GMT, Doug Simon <[hidden email]> wrote:

> Currently, `HotSpotJVMCIRuntime.writeDebugOutput` does nothing if the current thread is not attached to HotSpot (i.e., `Thread::current_or_null() == NULL`). This means crucial debug info can be lost. For reference, an unattached libgraal thread is a thread started from within libgraal that has not yet attached itself to the VM (e.g., before [this line](https://github.com/oracle/graal/blob/e4b9ab931940e1946f96f2015b937ba100384573/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java#L42)) or has already detached itself (e.g., after [this line](https://github.com/oracle/graal/blob/e4b9ab931940e1946f96f2015b937ba100384573/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java#L46)).
>
> The reason for the current behavior is that `HotSpotJVMCIRuntime.writeDebugOutput` passes a Java byte array to C++ code and the C++ code calls back into Java to decode the byte array into a native buffer. These call backs require the current thread to be attached to the VM.
>
> This PR moves the Java-to-native-buffer decoding into Java and thus avoids the requirement for the current thread to be attached to the VM.
>
> Tested in libgraal by patching Graal as follows:
> diff --git a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java
> index 36064767c95..352395dd59b 100644
> --- a/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java
> +++ b/compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/GraalServiceThread.java
> @@ -43,7 +43,14 @@ public class GraalServiceThread extends Thread {
> try {
> runnable.run();
> } finally {
> + String debug = System.getenv("GraalServiceThread.debug");
> afterRun();
> + if ("true".equals(debug)) {
> + throw new InternalError("THROWN AFTER DETACHING");
> + }
> }
> }
>
> Running without the changes in this PR:
>> env GraalServiceThread.debug=true java -jar dacapo.jar avrora
> ===== DaCapo 9.12 avrora starting =====
> ===== DaCapo 9.12 avrora PASSED in 4270 msec =====
>
> Running with the changes in this PR:
>> env GraalServiceThread.debug=true java -jar dacapo.jar avrora
> ===== DaCapo 9.12 avrora starting =====
> Exception in thread "LibGraalHotSpotGraalManagement-init" java.lang.InternalError: THROWN AFTER DETACHING
> at org.graalvm.compiler.core.GraalServiceThread.run(GraalServiceThread.java:52)
> at com.oracle.svm.core.thread.JavaThreads.threadStartRoutine(JavaThreads.java:519)
> at com.oracle.svm.core.posix.thread.PosixJavaThreads.pthreadStartRoutine(PosixJavaThreads.java:192)
> ===== DaCapo 9.12 avrora PASSED in 4688 msec =====

src/hotspot/share/jvmci/jvmci.cpp line 220:

> 218: Thread* thread = Thread::current_or_null_safe();
> 219: if (thread != NULL) {
> 220: events->logv(thread, format, ap);

This fixes a bug found while testing this PR. The `StringEventLog::logv` method requires the current thread to not be NULL.

test/hotspot/jtreg/compiler/jvmci/compilerToVM/DebugOutputTest.java line 1:

> 1: /*

DebugOutputTest has been removed since it's redundant with TestHotSpotJVMCIRuntime.writeDebugOutputTest.

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

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

Vladimir Kozlov-2

Re: RFR: 8262011: [JVMCI] allow printing to tty from unattached libgraal thread

In reply to this post by Doug Simon

On Fri, 19 Feb 2021 10:10:17 GMT, Doug Simon <[hidden email]> wrote:

Seems fine.

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

Marked as reviewed by kvn (Reviewer).

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

Tom Rodriguez-3

Re: RFR: 8262011: [JVMCI] allow printing to tty from unattached libgraal thread

In reply to this post by Doug Simon

On Fri, 19 Feb 2021 10:10:17 GMT, Doug Simon <[hidden email]> wrote:

Looks good.

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

Marked as reviewed by never (Reviewer).

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

Doug Simon

Integrated: 8262011: [JVMCI] allow printing to tty from unattached libgraal thread

In reply to this post by Doug Simon

On Fri, 19 Feb 2021 10:10:17 GMT, Doug Simon <[hidden email]> wrote:

This pull request has now been integrated.

Changeset: d2b9c227
Author: Doug Simon <[hidden email]>
URL: https://git.openjdk.java.net/jdk/commit/d2b9c227
Stats: 322 lines in 8 files changed: 49 ins; 246 del; 27 mod

8262011: [JVMCI] allow printing to tty from unattached libgraal thread

Reviewed-by: kvn, never

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

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