> On 19 Apr 2017, at 02:02, Mandy Chung <mandy.ch...@oracle.com> wrote:
>
>
>> On Apr 18, 2017, at 3:13 PM, Doug Simon <doug.si...@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
Sorry, the webrev was out of date. I've updated it and also fixed the comment
for VM::getSavedProperty.
> 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.
JVMCI is broken into fine grained "projects" and the jdk.vm.ci.runtime[1]
project depends on the jdk.vm.ci.services project[2] so I cannot make a direct
reference without introducing a circular dependency. I don't expect the
reflection cost to be significant in practice.
> 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.
Yes, I think you're right. I've updated the webrev with that change.
Thanks for the review.
-Doug
[1]
http://hg.openjdk.java.net/jdk9/dev/hotspot/file/560d7aa083a2/.mx.jvmci/suite.py#l94
[2]
http://hg.openjdk.java.net/jdk9/dev/hotspot/file/560d7aa083a2/.mx.jvmci/suite.py#l45
