Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 27, 2017, at 2:30 PM, Doug Simon wrote: > > >> On 27 Apr 2017, at 23:24, Mandy Chung wrote: >> >> >>> On Apr 27, 2017, at 7:47 AM, Doug Simon wrote: >>> >>> [1] http://cr.openjdk.java.net/~dnsimon/8177845.02 >>

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 27, 2017, at 7:47 AM, Doug Simon wrote: > > [1] http://cr.openjdk.java.net/~dnsimon/8177845.02 I reviewed the top repo and jdk repo change. For hotspot change, I reviewed jdk.vm.ci.services/* and jdk.internal.vm.ci module-info.java.

Re: RFR: 8177845: Need a mechanism to load Graal

> On 27 Apr 2017, at 21:15, Vladimir Kozlov wrote: > > make/CompileJavaModules.gmk Please, adjust comment for jdk.aot: > > ># Don't use Indy strings concatenation to have good JVMCI startup > >performance. > --- > <# Don't use Indy strings concatenation to have

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 27, 2017, at 11:51 AM, Doug Simon wrote: > >> >> On 27 Apr 2017, at 18:44, Christian Thalinger wrote: >> >> Thanks for doing this. I have a hard time following what’s happening :-) >> >> src/jdk.internal.vm.compiler/.mx.graal/suite.py

Re: RFR: 8177845: Need a mechanism to load Graal

> On 27 Apr 2017, at 18:44, Christian Thalinger wrote: > > Thanks for doing this. I have a hard time following what’s happening :-) > > src/jdk.internal.vm.compiler/.mx.graal/suite.py > > + "jdklibraries" : { > +"JVMCI_SERVICES" : { > + "path" :

Re: RFR: 8177845: Need a mechanism to load Graal

Thanks for doing this. I have a hard time following what’s happening :-) src/jdk.internal.vm.compiler/.mx.graal/suite.py + "jdklibraries" : { +"JVMCI_SERVICES" : { + "path" : "lib/jvmci-services.jar", + "sourcePath" : "lib/jvmci-services.src.zip", + "optional" : False, +

Re: RFR: 8177845: Need a mechanism to load Graal

> On 27 Apr 2017, at 17:40, Alan Bateman wrote: > > On 27/04/2017 15:47, Doug Simon wrote: > >> : >> >> - The jaotc launcher now needs to explicitly export JVMCI and >> jdk.internal.vm.compiler to jdk.aot[4]. >> Unfortunately there needs to be one --add-exports

Re: RFR: 8177845: Need a mechanism to load Graal

On 27/04/2017 15:47, Doug Simon wrote: : - The jaotc launcher now needs to explicitly export JVMCI and jdk.internal.vm.compiler to jdk.aot[4]. Unfortunately there needs to be one --add-exports option per qualified export target as combining them with a comma (e.g.,

Re: RFR: 8177845: Need a mechanism to load Graal

> On 21 Apr 2017, at 13:46, Doug Simon wrote: > > 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

Re: RFR: 8177845: Need a mechanism to load Graal

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

Re: RFR: 8177845: Need a mechanism to load Graal

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

Re: RFR: 8177845: Need a mechanism to load Graal

> On 19 Apr 2017, at 22:29, Vladimir Kozlov wrote: > > ReflectionAccessJDK ? Based on your comment in the file. > > Vladimir > > On 4/19/17 1:02 PM, Doug Simon wrote: >> Sure - how about good old Util? ;-) I'm open to other suggestions. >> >> Sent from my iPhone

Re: RFR: 8177845: Need a mechanism to load Graal

> On 20 Apr 2017, at 00:31, Vladimir Kozlov wrote: > > Doug, > > Can you point (link) particular code which needs to be reviewed? And what > security issues could be? I believe Chris had possible concerns with Services.initializeJVMCI() (at

Re: RFR: 8177845: Need a mechanism to load Graal

Doug, Can you point (link) particular code which needs to be reviewed? And what security issues could be? Thanks, Vladimir On 4/19/17 2:12 PM, Doug Simon wrote: 3. Services.initializeJVMCI() 3 is harmless from a security perspective in my opinion. Would be good if one of Oracle’s

Re: RFR: 8177845: Need a mechanism to load Graal

Hotspot changes looks good to me. Thanks, Vladimir On 4/19/17 2:26 PM, Doug Simon wrote: I've updated http://cr.openjdk.java.net/~dnsimon/8177845/hotspot/ with these changes: 1. JVMCIServiceLocator.getProvider(Class) is now protected 2. JVMCIServiceLocator.getProviders(Class) now checks

Re: RFR: 8177845: Need a mechanism to load Graal

I've updated http://cr.openjdk.java.net/~dnsimon/8177845/hotspot/ with these changes: 1. JVMCIServiceLocator.getProvider(Class) is now protected 2. JVMCIServiceLocator.getProviders(Class) now checks JVMCIPermission 3. Rename: jdk.vm.ci.services.internal.JDK9 ->

Re: RFR: 8177845: Need a mechanism to load Graal

> On 19 Apr 2017, at 21:40, Christian Thalinger wrote: > >> >> On Apr 19, 2017, at 9:27 AM, Doug Simon wrote: >> >>> >>> On 19 Apr 2017, at 21:04, Mandy Chung wrote: >>> >>> On Apr 19, 2017, at 11:55 AM,

Re: RFR: 8177845: Need a mechanism to load Graal

ReflectionAccessJDK ? Based on your comment in the file. Vladimir On 4/19/17 1:02 PM, Doug Simon wrote: Sure - how about good old Util? ;-) I'm open to other suggestions. Sent from my iPhone On Apr 19, 2017, at 9:46 PM, Vladimir Kozlov wrote: Hi Doug, Can you

Re: RFR: 8177845: Need a mechanism to load Graal

Sure - how about good old Util? ;-) I'm open to other suggestions. Sent from my iPhone > On Apr 19, 2017, at 9:46 PM, Vladimir Kozlov > wrote: > > Hi Doug, > > Can you consider using other name and not JDK9 for new JVMCI class? It will > be used in JDK 10 too: >

Re: RFR: 8177845: Need a mechanism to load Graal

Hi Doug, Can you consider using other name and not JDK9 for new JVMCI class? It will be used in JDK 10 too: jdk.vm.ci.services.internal.JDK9; Thanks, Vladimir On 4/18/17 3:13 PM, Doug Simon wrote: Please review these changes that make jdk.internal.vm.compiler an upgradable compiler. The

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 19, 2017, at 9:27 AM, Doug Simon wrote: > >> >> On 19 Apr 2017, at 21:04, Mandy Chung wrote: >> >> >>> On Apr 19, 2017, at 11:55 AM, Christian Thalinger >>> wrote: >>> >>> On Apr 19, 2017, at 8:38 AM,

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 19, 2017, at 12:07 PM, Doug Simon wrote: > > >> On 19 Apr 2017, at 20:01, Mandy Chung wrote: >> >> We should catch this issue and detect if VM.getSavedProperty is called when >> init level < 1 during early startup. > > What are you

Re: RFR: 8177845: Need a mechanism to load Graal

> On 19 Apr 2017, at 20:01, Mandy Chung wrote: > >> >> On Apr 19, 2017, at 8:03 AM, Peter Levart wrote: >> >> Hi Alan, >> >> On 04/19/2017 03:41 PM, Alan Bateman wrote: >>> On 19/04/2017 08:37, Peter Levart wrote: >>> : Note

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 19, 2017, at 11:55 AM, Christian Thalinger > wrote: > > >> On Apr 19, 2017, at 8:38 AM, Mandy Chung wrote: >> >> Since jdk.internal.vm.compiler becomes an upgradeable module, it is not >> hashed with java.base to allow it to be

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 19, 2017, at 8:38 AM, Mandy Chung 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

Re: RFR: 8177845: Need a mechanism to load Graal

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

Re: RFR: 8177845: Need a mechanism to load Graal

I lost track a bit but why are we exporting jdk.vm.ci.services to the world now? module jdk.internal.vm.ci { -exports jdk.vm.ci.services to jdk.internal.vm.compiler; +exports jdk.vm.ci.services; > On Apr 18, 2017, at 9:16 PM, Doug Simon wrote: > > (adding

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 19, 2017, at 8:03 AM, Peter Levart wrote: > > Hi Alan, > > On 04/19/2017 03:41 PM, Alan Bateman wrote: >> On 19/04/2017 08:37, Peter Levart wrote: >> >>> : >>> >>> Note that Properties class is thread-safe, while HashMap isn't. But I think >>> this should

Re: RFR: 8177845: Need a mechanism to load Graal

> On 19 Apr 2017, at 16:04, Alan Bateman wrote: > > On 18/04/2017 23:13, Doug Simon wrote: > >> : >> >> https://bugs.openjdk.java.net/browse/JDK-8177845 >> http://cr.openjdk.java.net/~dnsimon/8177845/ >> > I'm happy to see jdk.internal.vm.compiler changing to be an

Re: RFR: 8177845: Need a mechanism to load Graal

On 18/04/2017 23:13, Doug Simon wrote: : https://bugs.openjdk.java.net/browse/JDK-8177845 http://cr.openjdk.java.net/~dnsimon/8177845/ I'm happy to see jdk.internal.vm.compiler changing to be an upgradable module and the patching foo going away. If I read the changes correctly then I can

Re: RFR: 8177845: Need a mechanism to load Graal

On 19/04/2017 08:37, Peter Levart wrote: : Note that Properties class is thread-safe, while HashMap isn't. But I think this should not be a problem here, as during initPhase1(), as far as I know, no threads apart from main thread are started yet (is this true?), so anyone accessing

Re: RFR: 8177845: Need a mechanism to load Graal

On 04/19/2017 11:49 AM, Peter Levart wrote: I was not aware of the new Map.ofEntries method. Nice to see more space efficient map implementations being added to the JDK. Admittedly, I used this method in a way not envisioned by the author. Maybe there's a reason there is no Map.copyOf(Map)

Re: RFR: 8177845: Need a mechanism to load Graal

Hi Simon, On 04/19/2017 11:25 AM, Doug Simon wrote: Hi Peter, All of your suggestions look good. I've updated http://cr.openjdk.java.net/~dnsimon/8177845/jdk/src/java.base/share/classes/jdk/internal/misc/VM.java.udiff.html to include them (please check I didn't make any copy errors in the

Re: RFR: 8177845: Need a mechanism to load Graal

Hi Peter, All of your suggestions look good. I've updated http://cr.openjdk.java.net/~dnsimon/8177845/jdk/src/java.base/share/classes/jdk/internal/misc/VM.java.udiff.html to include them (please check I didn't make any copy errors in the process). I was not aware of the new Map.ofEntries

Re: RFR: 8177845: Need a mechanism to load Graal

On 04/19/2017 09:42 AM, Peter Levart wrote: On 04/19/2017 09:37 AM, Peter Levart wrote: http://cr.openjdk.java.net/~plevart/jdk9-dev/8177845_VM.getSavedProperties/webrev.01/ Also, while we are at it, the following javadocs in the getSavedProperty() do not apply any more: 138

Re: RFR: 8177845: Need a mechanism to load Graal

On 04/19/2017 09:37 AM, Peter Levart wrote: http://cr.openjdk.java.net/~plevart/jdk9-dev/8177845_VM.getSavedProperties/webrev.01/ Also, while we are at it, the following javadocs in the getSavedProperty() do not apply any more: 138 * It accesses a private copy of the system

Re: RFR: 8177845: Need a mechanism to load Graal

(adding hotspot-compiler-dev) > On 19 Apr 2017, at 02:02, Mandy Chung wrote: > > >> On Apr 18, 2017, at 3:13 PM, Doug Simon wrote: >> >> Please review these changes that make jdk.internal.vm.compiler an upgradable >> compiler. >> : >>

RFR: 8177845: Need a mechanism to load Graal

(adding hotspot-compiler-dev) Please review these changes that make jdk.internal.vm.compiler an upgradable compiler. The primary requirement for this is to remove all usage of JDK internals from Graal. While this most involves changes in Graal, there remain a few capabilities that must be

Re: RFR: 8177845: Need a mechanism to load Graal

> On 19 Apr 2017, at 02:02, Mandy Chung wrote: > > >> On Apr 18, 2017, at 3:13 PM, Doug Simon wrote: >> >> Please review these changes that make jdk.internal.vm.compiler an upgradable >> compiler. >> : >>

Re: RFR: 8177845: Need a mechanism to load Graal

> On Apr 18, 2017, at 3:13 PM, Doug Simon wrote: > > Please review these changes that make jdk.internal.vm.compiler an upgradable > compiler. > : > http://cr.openjdk.java.net/~dnsimon/8177845/ Thanks for making this change. This would simplify the way to replace JDK’s

RFR: 8177845: Need a mechanism to load Graal

Please review these changes that make jdk.internal.vm.compiler an upgradable compiler. The primary requirement for this is to remove all usage of JDK internals from Graal. While this most involves changes in Graal, there remain a few capabilities that must be exposed via JVMCI. Namely: 1.