eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 722 forks source link

Need JVM support for nestmates #2269

Closed keithc-ca closed 6 years ago

keithc-ca commented 6 years ago

The latest merge from jdk-11+19 expects three new JVM functions; builds will fail with link errors until these are defined:

JNIEXPORT jboolean JNICALL
JVM_AreNestMates(JNIEnv *env, jclass current, jclass member);

JNIEXPORT jclass JNICALL
JVM_GetNestHost(JNIEnv *env, jclass current);

JNIEXPORT jobjectArray JNICALL
JVM_GetNestMembers(JNIEnv *env, jclass current);
theresa-m commented 6 years ago

Working on stubs to unblock builds.

JasonFengJ9 commented 6 years ago

I have a change to add these three methods however adding them alone still won't make raw build -version work due to a j.l.IllegalAccessError.

tajila commented 6 years ago

@JasonFengJ9 we need to turn on the spec as well, https://github.com/eclipse/openj9/pull/2270

We also need to do some refactoring for the JVM_* methods so we don't duplicate the code, but that can be done after the stubs are added.

JasonFengJ9 commented 6 years ago

Enabled the nestmates spec as per #2270, now get following error:

Exception in thread "main" java/lang/IncompatibleClassChangeError: java/lang/ThreadLocal$ThreadLocalMap.getEntry(Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;
    at java/lang/ThreadLocal.get (java.base@9/ThreadLocal.java:165)
    at java/lang/StringCoding.decodeASCII (java.base@9/StringCoding.java:520)
    at java/lang/StringCoding.decode (java.base@9/StringCoding.java:235)
    at java/lang/String.<init> (java.base@9/String.java:586)
    at java/lang/String.<init> (java.base@9/String.java:627)
    at java/lang/System.initProperties (java.base@9/NativeMethod:4294967295)
    at java/lang/System.ensureProperties (java.base@9/System.java:284)
    at java/lang/System.afterClinitInitialization (java.base@9/System.java:128)
    at java/lang/Thread.initialize (java.base@9/Thread.java:371)
    at java/lang/Thread.<init> (java.base@9/Thread.java:153)
JasonFengJ9 commented 6 years ago

Discussed with Tobi, this IncompatibleClassChangeError doesn't happen in a CCM so it is raw build specific only.

JasonFengJ9 commented 6 years ago

Raw build with nestmates enabled without JVM_ stub methods shows symbol JVM_GetNestHost, version SUNWprivate_1.1 not defined in file libjvm.so It appears JVM_ stub methods are required for raw builds. If CCM doesn't require these JVM_ methods, we might have to re-think how to make raw builds.

tajila commented 6 years ago

@JasonFengJ9 turns out we will need the stubs.

They are used in Java_jdk_internal_reflectReflection*

keithc-ca commented 6 years ago

I suggest that 'raw builds' are no longer useful: there's no point expending effort to update them.

JasonFengJ9 commented 6 years ago

When the automated build starts merging latest OpenJDK tip into extension repo, it will be straightforward to discover VM/JCL incompatible issue with CCM than raw builds.

pshipton commented 6 years ago

When the automated build starts merging latest OpenJDK tip into extension repo

This is already occurring, and is how we got the jdk-11+19 update.

tajila commented 6 years ago

Update: The version I tested with was b19 (394c091d02f906cbed0a783c06f464cf7dba1dc8) but slightly older than the one Jason was using. When I updated I ran into the same ICCE issue that Jason encountered.

The nestmates feature allows private access between nestmates. Because of this javac no longer has to generate access bridges for inner classes, it simply puts the inner classes in the same nest has the outer class. The outer class can then access private fields/methods in the inner classes directly.

Here is an example:

With last weeks javac:

 public T get();
    Code:
       0: invokestatic  #10                 // Method java/lang/Thread.currentThread:()Ljava/lang/Thread;
       3: astore_1
       4: aload_0
       5: aload_1
       6: invokevirtual #11                 // Method getMap:(Ljava/lang/Thread;)Ljava/lang/ThreadLocal$ThreadLocalMap;
       9: astore_2
      10: aload_2
      11: ifnull        33
      14: aload_2
      15: aload_0
      16: invokestatic  #12                 // Method java/lang/ThreadLocal$ThreadLocalMap.access$000:(Ljava/lang/ThreadLocal$ThreadLocalMap;Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;
      19: astore_3
      20: aload_3
      21: ifnull        33
      24: aload_3
      25: getfield      #13                 // Field java/lang/ThreadLocal$ThreadLocalMap$Entry.value:Ljava/lang/Object;
      28: astore        4
      30: aload         4
      32: areturn
      33: aload_0
      34: invokespecial #14                 // Method setInitialValue:()Ljava/lang/Object;
      37: areturn

With this weeks javac however:

  public T get();
    descriptor: ()Ljava/lang/Object;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=2, locals=5, args_size=1
         0: invokestatic  #10                 // Method java/lang/Thread.currentThread:()Ljava/lang/Thread;
         3: astore_1
         4: aload_0
         5: aload_1
         6: invokevirtual #11                 // Method getMap:(Ljava/lang/Thread;)Ljava/lang/ThreadLocal$ThreadLocalMap;
         9: astore_2
        10: aload_2
        11: ifnull        33
        14: aload_2
        15: aload_0
        16: invokevirtual #12                 // Method java/lang/ThreadLocal$ThreadLocalMap.getEntry:(Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;
        19: astore_3
        20: aload_3
        21: ifnull        33
        24: aload_3
        25: getfield      #13                 // Field java/lang/ThreadLocal$ThreadLocalMap$Entry.value:Ljava/lang/Object;
        28: astore        4
        30: aload         4
        32: areturn

The big change is : 16: invokestatic #12 // Method java/lang/ThreadLocal$ThreadLocalMap.access$000:(Ljava/lang/ThreadLocal$ThreadLocalMap;Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;

to:

16: invokevirtual #12 // Method java/lang/ThreadLocal$ThreadLocalMap.getEntry:(Ljava/lang/ThreadLocal;)Ljava/lang/ThreadLocal$ThreadLocalMap$Entry;

Our invokevirtual resolution doesn't currently deal with private methods which is causing the ICCE.

tajila commented 6 years ago

If we can turn off the javac nestmates changes for the CCM builds we should be fine, until we fix the rest of the issues on the interpreter side

JasonFengJ9 commented 6 years ago

As per Tobi suggested above, changing -source 11 -target 11 to -source 10 -target 10 within https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/openj9-jdk/make/common/SetupJavaCompilers.gmk#L75 (&L85) fixed the ICCE. Ideally the change need to be made within https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/openj9-jdk/closed/make/common/SetupJavaCompilers.gmk instead.

pshipton commented 6 years ago

I suppose we can do that temporarily, just to ensure we can continue to build/run this hybrid Java 11 while work on Nestmates changes are in progress. Anything using Java 11, such as compiling the tests, is going to use the default (-target 11) and not be able to run. Not sure we want to change everything only to have to revert it once Nestmates are working.

pshipton commented 6 years ago

fyi https://github.com/eclipse/openj9/issues/2897 which reverts to target 11. I don't believe we need this issue open any more, the remaining work is covered by other issues and PRs.