[9] RFR(XL) 8166417: Integrate Graal-core into JDK for AOT compiler

[Vladimir Kozlov]

Thank you, Mandy

On 12/8/16 7:46 PM, Mandy Chung wrote:
>
>> On Dec 7, 2016, at 2:10 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8166417
>>
>> It is part of JEP 295: Ahead-of-Time Compilation
>> https://bugs.openjdk.java.net/browse/JDK-8166089
>>
>> http://cr.openjdk.java.net/~kvn/8166417/top.webrev/
>> http://cr.openjdk.java.net/~kvn/8166417/jdk.webrev/
>
> I reviewed module-info.java.*.
>
> src/java.base/share/classes/module-info.java.extra
>
> Now I realized jdk.vm.compiler may be excluded from JDK time at
> build/configure time.  That explains why you can’t add these
> qualified exports to module-info.java.
>
> As we discussed offline, you can move this file to unix/classes
> and if jdk.vm.compiler is not included in the JDK build, the build
> tool that generates gensrc/java.base/module-info.java would filter
> out these qualified exports.

Done. And it works.

>
> Related to the use of jdk.internal.misc.VM, you mentioned separately
> that Graal uses jdk.internal.misc.VM.savedProps to find properties with
> GRAAL_OPTION_PROPERTY_PREFIX.  Why can’t you use System.getProperties?
> savedProps stores the properties that are removed from system properties.
> Are Graal’s properties not in system properties?

I asked Graal guys. May be Doug can answer.

>
>
>>
>> http://cr.openjdk.java.net/~kvn/8166417/hotspot.webrev/
>
> src/jdk.vm.compiler/share/classes/module-info.java
>
> jdk.management requires transitive java.management and so line 28 is
> not strictly necessary but no harm to have it:
>   28     requires java.management;

Then I will keep it.

>
> make/gensrc/Gensrc-jdk.vm.compiler.gmk
>
>  131         for i in $$($(FIND) $(GENSRC_DIR) -name '*_OptionDescriptors.java'); do \
>  132             c=$$($(ECHO) $$i | $(SED) 's:.*/jdk\.vm\.compiler/\(.*\)\.java:\1:' | $(TR) '/' '.'); \
>  133             $(ECHO) "provides org.graalvm.compiler.options.OptionDescriptors with $$c;" >> $@; \
>  134         done
>
> The new syntax for provides is to have one `provides` clause of the same SPI
> with comma-separated implementation classes (see [1] for reference)

Previous loop in this makefile also produces multiple `provides` of the 
same SPI. I fixed it too. The output is next for example:

provides org.graalvm.compiler.hotspot.CompilerConfigurationFactory with
     org.graalvm.compiler.hotspot.CoreCompilerConfigurationFactory,
     org.graalvm.compiler.hotspot.EconomyCompilerConfigurationFactory,
     ;

Looks like build tools excepts ; after ,
If you are fine with it I will make this change.

Thanks,
Vladimir

>
> The build tool temporarily allows multiple `provides` of the same SPI just
> for transition.  Is it easy to change the above to generate one single `provides`
> clauses?  I’m okay if you want to file a JBS issue to follow up this separately
> after the integration.
>
> Mandy
> [1] http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/tip/src/java.desktop/share/classes/module-info.java
>