> On 29 Oct 2019, at 10:12, David Holmes <david.hol...@oracle.com> wrote:
>
> Hi Doug,
>
> Thanks for taking a look so quickly! :)
>
> On 29/10/2019 6:55 pm, Doug Simon wrote:
>> Hi David,
>> Since Graal needs to work against multiple JVMCI versions (and VM
>> versions!), the Graal changes should only disable the intrinsic when the
>> relevant VM config is missing:
>
> So to be clear I should revert all the Graal file changes I made and just
> apply the patch below?
Yes please.
-Doug
>
>> diff --git
>> a/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java
>>
>> b/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java
>> index 0752a621215..baa2136a6ba 100644
>> ---
>> a/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java
>> +++
>> b/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java
>> @@ -315,7 +315,7 @@ public class GraalHotSpotVMConfig extends
>> GraalHotSpotVMConfigBase {
>> public final int javaThreadAnchorOffset =
>> getFieldOffset("JavaThread::_anchor", Integer.class, "JavaFrameAnchor");
>> public final int javaThreadShouldPostOnExceptionsFlagOffset =
>> getFieldOffset("JavaThread::_should_post_on_exceptions_flag", Integer.class,
>> "int", Integer.MIN_VALUE);
>> public final int threadObjectOffset =
>> getFieldOffset("JavaThread::_threadObj", Integer.class, "oop");
>> - public final int osThreadOffset =
>> getFieldOffset("JavaThread::_osthread", Integer.class, "OSThread*");
>> + public final int osThreadOffset =
>> getFieldOffset("JavaThread::_osthread", Integer.class, "OSThread*",
>> Integer.MAX_VALUE);
>> public final int threadIsMethodHandleReturnOffset =
>> getFieldOffset("JavaThread::_is_method_handle_return", Integer.class, "int");
>> public final int threadObjectResultOffset =
>> getFieldOffset("JavaThread::_vm_result", Integer.class, "oop");
>> public final int jvmciCountersThreadOffset =
>> getFieldOffset("JavaThread::_jvmci_counters", Integer.class, "jlong*");
>> diff --git
>> a/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java
>>
>> b/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java
>> index 6b37fff083d..ffc8032d2b0 100644
>> ---
>> a/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java
>> +++
>> b/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java
>> @@ -441,7 +441,9 @@ public class HotSpotGraphBuilderPlugins {
>> }
>> });
>> - r.registerMethodSubstitution(ThreadSubstitutions.class,
>> "isInterrupted", Receiver.class, boolean.class);
>> + if (config.osThreadOffset != Integer.MAX_VALUE) {
>> + r.registerMethodSubstitution(ThreadSubstitutions.class,
>> "isInterrupted", Receiver.class, boolean.class);
>> + }
>> }
>> public static final String reflectionClass;
>> diff --git
>> a/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java
>>
>> b/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java
>> index 1d694fae2a4..8500c4de115 100644
>> ---
>> a/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java
>> +++
>> b/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java
>> @@ -308,6 +308,7 @@ public class HotSpotReplacementsUtil {
>> @Fold
>> public static int osThreadOffset(@InjectedParameter
>> GraalHotSpotVMConfig config) {
>> + assert config.instanceKlassInitThreadOffset != Integer.MAX_VALUE;
>> return config.osThreadOffset;
>> }
>> All the other JVMCI changes look good to me.
>> -Doug
>>> On 29 Oct 2019, at 08:42, David Holmes <david.hol...@oracle.com> wrote:
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8229516
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8232676 (already approved)
>>> webrev: http://cr.openjdk.java.net/~dholmes/8229516/webrev/
>>>
>>> This cross-cuts core-libs, hotspot-runtime and the JIT compilers, but only
>>> in small pieces each. There is also a small touch to serviceability code.
>>>
>>> This change is "simply" moving the "interrupted" field out of the osThread
>>> and into the java.lang.Thread so that it can be set independently of
>>> whether the thread is alive (and to make things easier for loom in the near
>>> future). It is very straightforward:
>>>
>>> - old scheme
>>> - interrupted field is in osThread
>>> - VM can read/write directly
>>> - Java code calls into VM to read/write
>>>
>>> - new scheme
>>> - interrupted field is in java.lang.Thread
>>> - VM has to use javaCalls to read/write "directly"
>>> - Java code can read/write directly
>>>
>>> No changes to any of the semantics regarding the actual interrupt
>>> mechanism. Special thanks to Patricio for tracking down a bug I had
>>> introduced in that regard!
>>>
>>> Special Note (Hi Roger!): on windows we still need to set/clear the
>>> _interrupt_event used by the Process.waitFor logic. To facilitate clearing
>>> via Thread.interrupted() I had to introduce a native method that is a no-op
>>> except on Windows. This seemed the cheapest and least intrusive means to
>>> achieve this.
>>>
>>> Other changes revolve around the fact we used to have an intrinsic for
>>> Thread.isInterrupted and that is not needed any more. So we strip some code
>>> out of C1/C2.
>>>
>>> The changes in the JVMCI/Graal code are a bit more far reaching as entire
>>> classes disappear. I've cc'd Doug and Tom at Vladimir's request so that
>>> they can comment on the JVMCI changes and whether I have gone too far or
>>> not far enough. There are a bunch of tests for interruption in JVMCI that
>>> could potentially be deleted if they are only intended to test the JVMCI
>>> handling of interrupt:
>>>
>>> ./jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.jtt/src/org/graalvm/compiler/jtt/threads/Thread_isInterrupted*.java
>>>
>>> Testing:
>>> - Tiers 1-3 on all Oracle platforms
>>> - Focused testing on Linux x64:
>>> - Stress runs of JSR166TestCase
>>> - Anything that seems to use interrupt():
>>> - JDK
>>> - java/lang/Thread
>>> - java/util/concurrent
>>> - jdk/internal/loader/InterruptedClassLoad.java
>>> - javax/management
>>> - java/nio/file/Files
>>> - java/nio/channels
>>> - java/net/Socket/Timeouts.java
>>> - java/lang/Runtime/shutdown/
>>> - java/lang/ProcessBuilder/Basic.java
>>> - com/sun/jdi/
>>> - Hotspot
>>> - vmTestbase/nsk/monitoring/
>>> - vmTestbase/nsk/jdwp
>>> - vmTestbase/nsk/jdb/
>>> - vmTestbase/nsk/jdi/
>>> - vmTestbase/nsk/jvmti/
>>> - runtime/Thread
>>> - serviceability/jvmti/
>>> - serviceability/jdwp
>>> - serviceability/sa
>>>
>>> Thanks,
>>> David
>>> -----
