Thanks for your comments, Mandy.
On pondělí 21. srpna 2017 12:42:09 CEST mandy chung wrote:
> cc'ing serviceability-dev which is the right mailing list for platform
> management discussion.
>
> JVMCI is currently named as `jdk.internal.vm.ci` (a JDK internal
> module). I suppose this new module is intended to be kept as an
> internal module?
The module doesn't export any API - e.g. it is internal. The current name is
jdk.vm.ci.management - shall I change that to something like
jdk.internal.vm.ci.management? Or some other (shorter) suggestion?
> 30 * @since 9
>
> should be @since 10.
I see. I'll fix that.
> JVMCIMBeans.java
>
> 55 @Override
> 56 public Set<Class<?>> mbeanInterfaces() {
> 57 return Collections.singleton(mbean.getClass());
> 58 }
>
> This mbeanInterfaces method should return the MBean interface class but
> not the class of the mbean implementation. This allows the platform
> mbean to be looked up from ManagementFactory.getPlatformMXBean. If this
> is a dynamic mbean, then this method simply returns an empty list (see
> DiagnosticCommandMBean).
I am almost 100% sure that the bean(s) exposed from Truffle is going to be
DynamicMBean - we compose the attributes dynamically, that wouldn't work with
reflection.
I'll change the code to return an empty list if the bean is DynamicMBean.
> To support standard mbean,
> HotSpotJVMCICompilerFactory::mbeans method would need to include the
> mbean interface type in addition to the name and the mbean
> implementation object, i.e. may need to define a specific type for it.
As we don't have any use for this yet I suggest to keep things simple. I
believe following contract could work:
public Set<Class<?>> mbeanInterfaces() {
if (mbean instanceof DynamicMBean) {
return Collections.emptySet();
} else {
Class<?>[] interfaces = mbean.getClass().getInterfaces();
return new HashSet<>(Arrays.asList(interfaces));
}
}
if this is acceptable, I would change documentation of the mbean() method to
mention how the set of mbeanInterfaces() is computed. OK?
Thanks for the review.
-jt
> On 8/18/17 11:49 AM, Vladimir Kozlov wrote:
> > Updated changes in all repos:
> > http://cr.openjdk.java.net/~kvn/8182701/webrev.01/
> >
> > On 8/18/17 7:12 AM, Jaroslav Tulach wrote:
> >
> > Thanks for pushing me forward, Vladimir. Yes, the changes are still
> > needed if
> > we want the Graal compiler to expose its MBean in a lazy way. I am
> > offering
> > new webrev for review. It contains following changes:
> >
> > Per Mandy's suggestion I created new module jdk.vm.ci.management to
> > bridge between
> > JVMCI and jdk.management. Adding new module was a bit tricky, but with
> > great help of Jan
> > Lahoda I even managed to register it as a boot module.
> >
> > I renamed the JVMCIMXBean class and dropped X per Vladimir's advice. I
> > fixed
> > the non-standard location of JVMCIMXBean class.
> >
> > I changed the interface to use Map, so the compiler is able to expose
> > more
> > than a single bean.
> >
> > That's it. I am looking forward to your review comments.
> > -jt
> >
> > Here are original changes for reference:
> >
> > http://cr.openjdk.java.net/~kvn/8182701/webrev.jdk/
> > http://cr.openjdk.java.net/~kvn/8182701/webrev.hs/
> >
> > On 8/17/17 11:54 AM, Vladimir Kozlov wrote:
> >> Hi Jaroslav,
> >>
> >> What we should do with 8182701? Do you still need JVMCI changes?
> >>
> >> Note, your changes to Graal [GR-5435] were integrated recently into
> >> JDK (jdk10/hs):
> >> https://bugs.openjdk.java.net/browse/JDK-8186158
> >>
> >> Thanks,
> >> Vladimir
> >>
> >> On 8/14/17 10:06 AM, Jaroslav Tulach wrote:
> >>> On čtvrtek 3. srpna 2017 17:03:39 CEST Jaroslav Tulach wrote:
> >>>> On čtvrtek 27. července 2017 15:01:17 CEST Alan Bateman wrote:
> >>>>> On 27/07/2017 10:07, Jaroslav Tulach wrote:
> >>>>>> Yes, it seems like a desirable goal to let Graal compiler work
> >>>>>> with just
> >>>>>> java.base. Is there a description how to build JDK9/10 with just
> >>>>>> java.base
> >>>>>> that I could follow and test against?
> >>>>>
> >>>>> You can use jlink to create a run-time image that only contains
> >>>>> java.base (jlink --module-path $JAVA_HOME/jmods --add-modules
> >>>>> java.base
> >>>>> --output smalljre).
> >>>>
> >>>> Status update: I've just tried to run Graal compiler against JDK9
> >>>> with only
> >>>> java.base and jdk.internal.vm.ci modules, and there are some
> >>>> problems. I
> >>>> need to resolve them first before I provide updated version of my
> >>>> patch.
> >>>
> >>> FYI: As of
> >>> https://github.com/graalvm/graal/commit/ca9071941a1be7f1a3725529ecc231ff
> >>> 621d5ed0
> >>>
> >>> the Graal compiler can run with java.base, jdk.unsupported and
> >>> jdk.vm.ci only
> >>> modules. But it wasn't easy, especially the
> >>> http://wiki.apidesign.org/wiki/PropertyChangeListener required a bit
> >>> of work.
> >>>
> >>> -jt
