There has been some discussion about whether we want to update Graal in the JDK at this late stage. The main (only?) risk is a regression in the AOT tool.
If we don't update Graal from upstream, then the qualified exports from JVMCI to jdk.internal.vm.compiler cannot be removed in JDK 9. Note that in addition to updating Graal to remove the qualified exports, there would also need to be changes in the relevant make files to add --add-exports options when compiling Graal and jaotc as they use the dynamically exported JVMCI packages. I have an updated hotspot patch that adapts Graal to the JVMCI API changes: http://cr.openjdk.java.net/~dnsimon/8177845/hotspot.02/ Note that this patch does not include the changes removing use of JDK internal API from Graal. Cherry picking those upstream Graal changes would be more work than simply doing a complete update from upstream Graal. As I see it, there are 2 options here: 1. Go with the current webrev (including hotspot.02 patch) and live with the qualified exports. 2. Go with the current webrev (including hotspot.02 patch) and create a follow up bug to update Graal from upstream, perform the relevant make file changes and remove the qualified exports. I've tested the current webrev (including hotspot.02 patch) against both upstream Graal and with jprt. -Doug > On 20 Apr 2017, at 20:50, Doug Simon <doug.si...@oracle.com> wrote: > > I've had to update the webrev again to support selection of a "null" compiler > (i.e. one that raises an > exception upon a compilation request) and added -Djvmci.Compiler=null to a > large number of JVMCI jtreg > tests to prevent Graal being selected and initialized by the JVMCI compiler > auto-selection mechanism. > Initializing Graal will currently fail with errors (see stack trace below) > until Graal is updated to > the version compatible with the JVMCI API changes. > > In addition to resolving the compatibility issue, explicitly selecting the > "null" compiler for these > tests better isolates them from parts of the runtime they are not aiming to > test. > > org.graalvm.compiler.debug.GraalError: java.lang.ClassCastException: > java.base/java.util.ImmutableCollections$MapN cannot be cast to > java.base/java.util.Properties > at > jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.getSavedProperties(HotSpotGraalCompilerFactory.java:217) > at > jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.initializeOptions(HotSpotGraalCompilerFactory.java:138) > at > jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.onSelection(HotSpotGraalCompilerFactory.java:95) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCICompilerConfig.getCompilerFactory(HotSpotJVMCICompilerConfig.java:104) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.<init>(HotSpotJVMCIRuntime.java:290) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.<init>(HotSpotJVMCIRuntime.java:65) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime$DelayedInit.<clinit>(HotSpotJVMCIRuntime.java:73) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime(HotSpotJVMCIRuntime.java:83) > at jdk.internal.vm.ci/jdk.vm.ci.runtime.JVMCI.initializeRuntime(Native > Method) > at jdk.internal.vm.ci/jdk.vm.ci.runtime.JVMCI.<clinit>(JVMCI.java:58) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime(HotSpotJVMCIRuntime.java:82) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotVMConfig.config(HotSpotVMConfig.java:41) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotResolvedJavaMethodImpl.getHolder(HotSpotResolvedJavaMethodImpl.java:92) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotResolvedJavaMethodImpl.fromMetaspace(HotSpotResolvedJavaMethodImpl.java:110) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.CompilerToVM.asResolvedJavaMethod(Native > Method) > at > jdk.internal.vm.ci/jdk.vm.ci.hotspot.CompilerToVMHelper.asResolvedJavaMethod(CompilerToVMHelper.java:185) > at > compiler.jvmci.common.CTVMUtilities.getResolvedMethod(CTVMUtilities.java:59) > at > compiler.jvmci.common.CTVMUtilities.getResolvedMethod(CTVMUtilities.java:64) > at > compiler.jvmci.compilerToVM.AllocateCompileIdTest.runSanityCorrectTest(AllocateCompileIdTest.java:125) > at java.base/java.util.ArrayList.forEach(ArrayList.java:1378) > at > compiler.jvmci.compilerToVM.AllocateCompileIdTest.main(AllocateCompileIdTest.java:71) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:563) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:115) > at java.base/java.lang.Thread.run(Thread.java:844) > Caused by: java.lang.ClassCastException: > java.base/java.util.ImmutableCollections$MapN cannot be cast to > java.base/java.util.Properties > at > jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.getSavedProperties(HotSpotGraalCompilerFactory.java:215) > ... 26 more > > -Doug > >> On 19 Apr 2017, at 23:26, Doug Simon <doug.si...@oracle.com> wrote: >> >> I've updated http://cr.openjdk.java.net/~dnsimon/8177845/hotspot/ with these >> changes: >> >> 1. JVMCIServiceLocator.getProvider(Class<S>) is now protected >> 2. JVMCIServiceLocator.getProviders(Class<S>) now checks JVMCIPermission >> 3. Rename: jdk.vm.ci.services.internal.JDK9 -> >> jdk.vm.ci.services.internal.ReflectionAccessJDK >> >> -Doug >> >>> On 19 Apr 2017, at 23:12, Doug Simon <doug.si...@oracle.com> wrote: >>> >>>> >>>> On 19 Apr 2017, at 21:40, Christian Thalinger <cthalin...@twitter.com> >>>> wrote: >>>> >>>>> >>>>> On Apr 19, 2017, at 9:27 AM, Doug Simon <doug.si...@oracle.com> wrote: >>>>> >>>>>> >>>>>> On 19 Apr 2017, at 21:04, Mandy Chung <mandy.ch...@oracle.com> wrote: >>>>>> >>>>>> >>>>>>> On Apr 19, 2017, at 11:55 AM, Christian Thalinger >>>>>>> <cthalin...@twitter.com> wrote: >>>>>>> >>>>>>> >>>>>>>> On Apr 19, 2017, at 8:38 AM, Mandy Chung <mandy.ch...@oracle.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Since jdk.internal.vm.compiler becomes an upgradeable module, it is >>>>>>>> not hashed with java.base to allow it to be upgraded and there is no >>>>>>>> integrity check. Such qualified export will be granted to any module >>>>>>>> named jdk.internal.vm.compiler at runtime. The goal is for >>>>>>>> upgradeable modules not to use any internal APIs and eliminate the >>>>>>>> qualified exports. >>>>>>>> >>>>>>>> The main thing is that jdk.vm.ci.services API would need to be guarded >>>>>>>> if it’s used by non-Graal modules. >>>>>>> >>>>>>> This all makes sense but where is the restriction that only >>>>>>> jdk.internal.vm.compiler can use jdk.vm.ci.services? >>>>>> >>>>>> It's unqualified and no restriction in this change. >>>>> >>>>> The public methods currently in jdk.vm.ci.services are: >>>>> >>>>> 1. JVMCIServiceLocator.getProvider(Class<S>) >>>>> 2. JVMCIServiceLocator.getProviders(Class<S>) >>>>> 3. Services.initializeJVMCI() >>>>> 4. Services.getSavedProperties() >>>>> 5. Services.exportJVMCITo(Class<?>) >>>>> 6. Services.load(Class<S>) >>>>> 7. Services.loadSingle(Class<S>, boolean) >>>>> >>>>> 1 should be made protected. I'll update the webrev with this change. >>>> >>>> Good. >>>> >>>>> >>>>> 2 should check for JVMCIPermission. I'll update the webrev with this >>>>> change. >>>> >>>> Good. >>>> >>>>> >>>>> 3 is harmless from a security perspective in my opinion. >>>> >>>> Would be good if one of Oracle’s security engineers could take a quick >>>> look just to be sure. >>> >>> Vladimir, can you please bring this to the attention of the relevant >>> engineer. >>> >>>>> >>>>> 4 checks for JVMCIPermission. >>>> >>>> Ok. >>>> >>>>> >>>>> 5, 6 and 7 will be removed in a follow bug that updates Graal from >>>>> upstream (and removes its usage of these methods). >>>> >>>> About this, will this Graal update happen for JDK 9? >>> >>> Yes. >>> >>>> It’s awfully late in the cycle... >>> >>> These are jigsaw related changes and I've been told jigsaw has an FC >>> exception (although I don't exactly know what that is). >>> >>> -Doug >> >
