JEP 371 Hidden Classes

[pshipton]

https://openjdk.java.net/jeps/371

This is delivered to OpenJDK now, affecting OpenJ9 builds of jdk15.

[pshipton]

@DanHeidinga fyi. I'm attempting to stub enough to build in https://github.com/eclipse/openj9/pull/9292

[pshipton]

@DanHeidinga stubbing isn't sufficient, hidden classes are used at runtime.

Exception in thread "main" java/lang/InternalError: java.lang.NullPointerException
    at java/lang/invoke/InnerClassLambdaMetafactory.spinInnerClass (java.base@9/InnerClassLambdaMetafactory.java:357)
    at java/lang/invoke/InnerClassLambdaMetafactory.buildCallSite (java.base@9/InnerClassLambdaMetafactory.java:209)
    at java/lang/invoke/LambdaMetafactory.metafactory (java.base@9/LambdaMetafactory.java:328)
    at java/lang/invoke/MethodHandle.invokeBsm (java.base@9/MethodHandle.java:1076)
    at java/lang/invoke/MethodHandle.resolveInvokeDynamic (java.base@9/MethodHandle.java:1200)
    at jdk/internal/module/SystemModuleFinders$1.find (java.base@9/SystemModuleFinders.java:215)
    at jdk/internal/module/ModuleBootstrap.boot (java.base@9/ModuleBootstrap.java:230)
    at java/lang/ClassLoader.initializeClassLoaders (java.base@9/ClassLoader.java:211)
    at java/lang/Thread.initialize (java.base@9/Thread.java:430)
    at java/lang/Thread.<init> (java.base@9/Thread.java:155)

[hangshao0]

I can pick this up.

[DanHeidinga]

Thanks @hangshao0. As mentioned on slack, I think the first step is to try and stub these apis with Unsafe.defineAnonyousClass() so we can build 15 again. Then look at a proper implementation

[hangshao0]

I plan to take the change in #9292 and start from there. But it seems that I cannot build with the latest code in branch open-staging from https://github.com/ibmruntimes/openj9-openjdk-jdk.

https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDKnext_x86-64_linux_Personal/425/console

error: warnings found and -Werror specified
...
....
16:32:06  CompileJavaModules.gmk:607: recipe for target '/home/jenkins/workspace/Build_JDKnext_x86-64_linux_Personal/build/linux-x86_64-server-release/jdk/modules/java.base/_the.java.base_batch' failed
16:32:06  make/Main.gmk:189: recipe for target 'java.base-java' failed
16:32:06  === End of repeated output ===

The message error: warnings found and -Werror specified is not there in the passing builds launched by other people, which I believe are on open branch.

I tried manually with --disable-warnings-as-errors, and it does not help. I noticed there are large number of make file changes this week into openj9-staging (e.g. https://github.com/ibmruntimes/openj9-openjdk-jdk/commit/109f6a571897eda3cf1309aaa876aa6a67320543, https://github.com/ibmruntimes/openj9-openjdk-jdk/commit/6b2547ecaab087d617f6205648966bb1bba3ace3), might be related to the build issue.

[pshipton]

There are 100+ warnings like the following, which aren't there in the last working build. I assume the compiler has changed. To fix it you can move final class ThunkTable, abstract class Comparator, final class ThunkTuple, and anything that shows up, out from MethodHandle.java to their own source files.

[2020-05-01T20:22:05.583Z] /home/jenkins/workspace/Build_JDKnext_x86-64_linux_Personal/build/linux-x86_64-server-release/support/j9jcl_sources/java.base/share/classes/java/lang/invoke/InvokeExactHandle.java:68: warning: auxiliary class ThunkTable in /home/jenkins/workspace/Build_JDKnext_x86-64_linux_Personal/build/linux-x86_64-server-release/support/j9jcl_sources/java.base/share/classes/java/lang/invoke/MethodHandle.java should not be accessed from outside its own source file
[2020-05-01T20:22:05.583Z]      private static final ThunkTable _thunkTable = new ThunkTable();

Otherwise to get started you can build from sha fed4f3e. It's the level I was using that was working up to the Optimizing the exploded image step, and then gets the exception in https://github.com/eclipse/openj9/issues/9328#issuecomment-618109380

[pshipton]

Ok, it's not that simple. I moved those classes into their own files, but there are many other warnings, and error: warnings found and -Werror specified still occurs. I suspect we removed -Werror previously but this change is lost and we need to restore it.

[pshipton]

This is the new code

https://github.com/ibmruntimes/openj9-openjdk-jdk/blob/dd30cb1200bd9d5a11c8ea9efdfdf63756a3d4fc/make/common/JavaCompilation.gmk#L242-#L243

While we figure out the best way to override that, export JAVA_WARNINGS_ARE_ERRORS= gets past that problem. Then the next problem is

Running ddrgen to generate j9ddr.dat and superset.dat
Blob written to file: ../j9ddr.dat
Superset written to file: ../superset.dat
make[4]: Leaving directory '/home/pete/openj9-openjdk-jdk/build/linux-x86_64-server-release/vm/ddr'
Compiling 89 properties into resource bundles for java.desktop
Creating support/modules_libs/java.base/libverify.so from 1 file(s)
Creating support/modules_libs/java.base/libjava.so from 66 file(s)
Creating support/native/java.base/libfdlibm.a from 57 file(s)
Creating support/modules_libs/java.base/libzip.so from 5 file(s)
Creating support/modules_libs/java.base/libjimage.so from 6 file(s)
Creating support/modules_libs/java.base/libjli.so from 8 file(s)
Creating support/modules_libs/java.base/libnet.so from 21 file(s)
Creating support/modules_libs/java.base/libnio.so from 20 file(s)
Updating support/src.zip
Compiling 2781 files for java.desktop
/usr/bin/ld: cannot find -ljvm
collect2: error: ld returned 1 exit status

[pshipton]

Here is a fix for the -Werror problem https://github.com/ibmruntimes/openj9-openjdk-jdk/pull/201

[andrew-m-leonard]

jdkhead merge job test build failing due to this, various errors:

03:39:39  /home/jenkins/workspace/Build_JDKnext_x86-64_linux_Personal/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java:45: error: static import only from classes and interfaces
03:39:39  import static java.lang.invoke.MethodHandles.Lookup.ClassOption.NESTMATE;
03:39:39  ^
03:39:39  /home/jenkins/workspace/Build_JDKnext_x86-64_linux_Personal/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java:46: error: cannot find symbol
03:39:39  import static java.lang.invoke.MethodHandles.Lookup.ClassOption.STRONG;
03:39:39                                                     ^
03:39:39    symbol:   class ClassOption
03:39:39    location: class Lookup

[keithc-ca]

@hangshao0 I cherry-picked the commit at the head of your JEP371_Test branch and merged it with #9292 and #9471 and was able to build the openj9-staging branch of jdknext:

$ ./images/jdk/bin/java -version

openjdk version "15-internal" 2020-09-15
OpenJDK Runtime Environment (build 15-internal+0-adhoc.keithc.jdk15)
Eclipse OpenJ9 VM (build h371b-6e8c206915a, JRE 15 Linux amd64-64-Bit Compressed References 20200507_000000 (JIT enabled, AOT enabled)
OpenJ9   - 6e8c206915a
OMR      - b22bd644869
JCL      - f33985b9f78 based on jdk-15+21)

Do you anticipate more work is required, or is it time for you to open a pull request?

[hangshao0]

Yes, I am about the open the PR. It seems that your machine finished building faster than mine.

[pshipton]

A number of openjdk tests are failing related to hidden classes, which makes sense considering the current impl is a hack until they can be properly implemented.

java/lang/invoke/defineHiddenClass/BasicTest.java.BasicTest java/lang/invoke/defineHiddenClass/HiddenNestmateTest.java.HiddenNestmateTest java/lang/invoke/defineHiddenClass/LambdaNestedInnerTest.java.LambdaNestedInnerTest java/lang/invoke/defineHiddenClass/SelfReferenceDescriptor.java.SelfReferenceDescriptor java/lang/invoke/defineHiddenClass/TypeDescriptorTest.java.TypeDescriptorTest java/lang/invoke/defineHiddenClass/UnloadingTest.java.UnloadingTest java/lang/invoke/defineHiddenClass/UnreflectTest.java.UnreflectTest java/lang/reflect/AccessibleObject/HiddenClassTest.java.HiddenClassTest

@JasonFengJ9 fyi

[hangshao0]

Was distracted by some SCC work recently, finally get back to this issue. Still in the process of writing new code, currently resolving some jave level exceptions.

[hangshao0]

Replaced the Unsafe.defineAnonymousClass() hack with a native implementation of a new define class API in ClassLoader.java, now Java -version is working.

[hangshao0]

I am running one hidden class test suite. Spent some time digging into the failures. I can get 4 more tests passing with my local change (put a bit details of the tests in work item 143941).

I will launch some builds to make sure my change does not break anything. If everything is fine, I will create a Pull Request for my current change.

[hangshao0]

From the JEP:

A hidden class can be unloaded when it is no longer reachable, or it can share the lifetime of a class loader so that it is unloaded only when the class loader is garbage collected.

the STRONG option may be passed to Lookup::defineHiddenClass. This arranges for a hidden class to have the same strong relationship with its notional defining loader as a normal class has with its defining loader, which is to say, the hidden class will only be unloaded if its notional defining loader can be reclaimed.

So if the hidden class is defined with ClassOption.STRONG, then it will be unloaded when its loader is GC'ed. If not defined with ClassOption.STRONG, it can be unloaded earlier (once it is not reachable). I guess there are GC work required for this. @dmitripivkine

in #10207 J9AccClassHiddenClass will be set in romClass->extraModifiers for a hidden class. J9AccHiddenClassOptionStrong will be set in romClass->extraModifiers if ClassOption.STRONG is used to define the hidden class.

[dmitripivkine]

So if the hidden class is defined with ClassOption.STRONG, then it will be unloaded when its loader is GC'ed. If not defined with ClassOption.STRONG, it can be unloaded earlier (once it is not reachable). I guess there are GC work required for this. @dmitripivkine

Class unloading is done on Classloader level (with exceptional handling for Anonymous classes unloaded individually but kept by artificial Anonymous Classloader can not be unloaded at all). If there is no mandatory early unloading for hidden classes I would prefer to keep its unloading on Classloader level. Otherwise change of unloading process would be fundamental and GC processing is just small fraction of it.

[dmitripivkine]

Is an implementation of hidden classes based on Anonymous (any hidden class is Anonymous from GC point of view)?

[hangshao0]

Is an implementation of hidden classes based on Anonymous (any hidden class is Anonymous from GC point of view)?

Yes, it is mostly based on anonymous classes. Hidden classes are introduced by this JEP to replace anonymous classes. But hidden class will have some extra flags set in romClass->extraModifiers and its defining class loader is a real class loader, not anonymous Classloader.

[hangshao0]

If there is no mandatory early unloading for hidden classes I would prefer to keep its unloading on Classloader level.

There are hidden class test that checks after System.gc(), hidden classes without ClassOption.STRONG are gone.

[dmitripivkine]

Yes, it is mostly based on anonymous classes. But hidden class will have some extra flags set in romClass->extraModifiers and its defining class loader is a real class loader, not anonymous Classloader.

I am asking from GC/unloading point of view. An Anonymous classes management is constructed special way to allow individual unloading:

• An artificial Anonymous classloader is used to manage Anonymous classes (even it's parent classloader is different)
• there is only one Anonymous class can be allocated per classloader RAM segment
• there is only one correspondent ROM class can be allocated per classloader ROM segment

Would you please explain what "mostly based" means in this context?

If hidden class is a kind of Anonymous class there is no extra GC work required and class can be unloaded individually. If hidden class is an extension of "regular" class there is no extra GC work required and class can be unloaded with classloader only. If there is a case that non of two statements above are true so what are special requirements for hidden class unloading and what is developed to support these requirements?

[hangshao0]

If hidden class is a kind of Anonymous class there is no extra GC work required and class can be unloaded individually. If hidden class is an extension of "regular" class there is no extra GC work required and class can be unloaded with classloader only.

OK, I see your point. The VM can decide whether to construct the hidden class the same way as anonymous (or regular) classes to control if it can be unloaded individually. Will try and see, probably we don't need any GC change.

[adamfarley]

The tests in sanity.OpenJDK that seemed to fail without this JEP have been excluded.

https://github.com/eclipse/openj9/issues/10345

When this JEP is complete, please re-integrate the tests, or notify the #testing channel in the Adopt Slack so someone else can (e.g. me).

[DanHeidinga]

@hangshao0 Now that the fix to prevent gcing hidden classes is released, is there anything else that needs to be completed for this JEP?

[hangshao0]

This JEP is functionally working now. But not all hidden class tests are passing. There are some negative test cases that assert certain type of exception/error be thrown, but sometimes we are throwing a different type of exception, making them to fail. I guess that might be due to openj9 has independent/different implementation (Methodhandles, verifier, etc). I am going through those test cases one by one now.

[hangshao0]

Made a bit update in work item 143941. Now I am looking into the failures mentioned in https://github.com/eclipse/openj9/issues/9328#issuecomment-671960644

[DanHeidinga]

@hangshao0 Do you have an update on this? We're fast approaching the M2 build and should be aiming to have fixed as many of the hidden classes issues as possible

[hangshao0]

I am launching some builds to verify my local changes. Will open a pull request soon.