RFR: 8177845: Need a mechanism to load Graal

[Peter Levart]

Hi Doug, Mandy,

Just a minor comment on the VM class changes...

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 getSavedProperty(ies) will either be the 
main thread itself or a thread started afterwards, so there is a 
happens-before relationship.

If this is true, then we could wrap the copy of saved properties with an 
unmodifiable map view before setting the savedProps field so that 
getSavedProperties() could always return the same instance. Like for 
example in the following alternative change:

http://cr.openjdk.java.net/~plevart/jdk9-dev/8177845_VM.getSavedProperties/webrev.01/

What do you think?

Regards, Peter

On 04/19/2017 02:02 AM, Mandy Chung wrote:
>> On Apr 18, 2017, at 3:13 PM, Doug Simon <doug.simon at oracle.com> 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 graal with the lab graal.  A couple of comments:
>
> jdk/internal/misc/VM.java
>
>   161      * Note that the saved system properties do not include
>   162      * the ones set by sun.misc.Version.init().
>
> sun.misc.Version is no longer present in JDK 9.  Renamed to java.lang.VersionProps
>
> jdk/vm/ci/services/Services.java
>    67             Class.forName("jdk.vm.ci.runtime.JVMCI”);
>
> JVMCI class is local in jdk.internal.vm.ci module.  An alternative may be
> to provide a static initialize method rather than Class::forName.
>
> jdk/vm/ci/hotspot/HotSpotJVMCIRuntime.java
>   211         for (HotSpotJVMCIBackendFactory factory : ServiceLoader.load(HotSpotJVMCIBackendFactory.class)) {
>
> This uses the thread context class loader to load providers.  Services::load uses
> the system class loader to load providers.  I suspect you want this to use the
> system class loader here too.
>
> jdk/vm/ci/services/JVMCIServiceLocator.java
>    78         for (JVMCIServiceLocator access : ServiceLoader.load(JVMCIServiceLocator.class)) {
>
> Same comment as above. I think you want to use system class loader to load providers.
>
> Mandy