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

[mandy chung]

On 9/12/17 10:44 AM, Jaroslav Tulach wrote:
> Dear reviewers,
> after several reconsiderations I have webrev #4 ready for your review. Can you
> please take a look at
>
> http://cr.openjdk.java.net/~jtulach/8182701/webrev.04/
>
> and let me know if it is in a reasonable shape? Thanks a lot.
> -jt

Yes defining a new provider module for the platform mbean registration 
is a reasonable solution.  Generally the patch looks right.  I have a 
question on the build and a comment on the new mbean method.
./make/common/Modules.gmk
     Nit: can you move jdk.internal.vm.compiler.management to keep the 
list in alphabetical order

  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.

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.

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.

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.  As we 
discussed offline, we agree that HotSpotRuntimeMBean should belong to 
this new module but it requires some refactoring which may take some 
amount of work.  Such clean up will be followed up in a separate JBS issue.

Mandy