[10] RFR(M) 8182701: Modify JVMCI to allow Graal Compiler to expose platform MBean

[Doug Simon]

> On 15 Sep 2017, at 14:32, Jaroslav Tulach <jaroslav.tulach at oracle.com> wrote:
> 
> Thanks for the review Mandy, Daniel. Now, that the consolidated JDK10 
> repository is available, I have updated my webrev to its structure. In 
> addition to that I addressed your comments:
> 
> On úterý 12. září 2017 11:19:45 CEST mandy chung wrote:
>> ./make/common/Modules.gmk
>>     Nit: can you move jdk.internal.vm.compiler.management to keep the
>> list in alphabetical order
> 
> Inserted at appropriate place.
> 
>>  199 # Filter out Graal specific modules if Graal build is disabled
>>  200
>>  201 ifeq ($(INCLUDE_GRAAL), false)
>>  202   MODULES_FILTER += jdk.internal.vm.compiler
>>  203 endif
>> 
>> When will INCLUDE_GRAAL be set to false?  I think
>> jdk.internal.vm.compiler.management should also be filtered if
>> jdk.internal.vm.compiler is disabled.
> 
> That is probably true. Fixed.
> 
>> 
>> Is jdk.internal.vm.compiler and jdk.internal.vm.compiler.management
>> built for all platforms in JDK 10? If not,
>>    jdk/src/java.management/share/classes/module-info.java may fail to
>> compile when jdk.internal.vm.compiler.management is not present.   We
>> can consult with the build team when you find out what configuration
>> that jdk.internal.vm.compiler is not built.
> 
> I haven't found configuration where jdk.internal.vm.compiler wouldn't be built. 
> However I wasn't looking very extensively...
> 
>> hotspot/src/jdk.internal.vm.compiler/share/classes/module-info.java 29
>> requires transitive jdk.internal.vm.ci;
>> do you get any error without this requires transitive?
>> jdk.internal.vm.compiler.management already requires
>> jdk.internal.vm.ci.  I would think this requires transitive is not
>> necessary.
> 
> Looks like this change isn't necessary. I am not sure what was the problem 
> before, when I introduced it.
> 
>> Is HotSpotGraalCompiler::mbean method necessary?  In GraalMBeans.java
>> 
>>   53     public static Object findGraalRuntimeBean() {
>>   54         JVMCIRuntime r = JVMCI.getRuntime();
>>   55         JVMCICompiler c = r.getCompiler();
>>   56         if (c instanceof HotSpotGraalCompiler) {
>>   57             return ((HotSpotGraalCompiler) c).mbean();
>>   58         }
>>   59         return null;
>>   60     }
>> 
>> It seems that you can call HotspotGraalRuntime::mbean directly. 
> 
> I don't think I can. There is no way to get to HotspotGraalRuntime except 
> asking the HotSpotGraalCompiler. The HotspotGraalRuntime isn't JVMCIRuntime... 
> At least I think so

That is correct - you have to obtain a HotspotGraalRuntime from a HotSpotGraalCompiler. There can be other HotspotGraalRuntime instances (e.g., Truffle has it's own HotSpotGraalCompiler/HotSpotGraalRuntime).

-Doug