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

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Vladimir Kozlov
https://bugs.openjdk.java.net/browse/JDK-8182701
webrev:
http://cr.openjdk.java.net/~kvn/8182701/webrev.jdk/
http://cr.openjdk.java.net/~kvn/8182701/webrev.hs/

Contributed by Jaroslav Tulach.

JDK itself contains quite a lot of platform MBeans which get registered "on demand". Graal compiler (jdk.internal.vm.compiler) provides its own MBean(s) - however currently there is no way to register
it "on demand". JDK9 already contains support for collecting platform MBeans from various modules. We just need to expose Graal MBean through JVMCI.

Thanks,
Vladimir
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Alan Bateman
On 12/07/2017 22:20, Vladimir Kozlov wrote:

> https://bugs.openjdk.java.net/browse/JDK-8182701
> webrev:
> http://cr.openjdk.java.net/~kvn/8182701/webrev.jdk/
> http://cr.openjdk.java.net/~kvn/8182701/webrev.hs/
>
> Contributed by Jaroslav Tulach.
>
> JDK itself contains quite a lot of platform MBeans which get
> registered "on demand". Graal compiler (jdk.internal.vm.compiler)
> provides its own MBean(s) - however currently there is no way to
> register it "on demand". JDK9 already contains support for collecting
> platform MBeans from various modules. We just need to expose Graal
> MBean through JVMCI.
Just to say that using services, by having jdk.internal.vm.ci declare
that it `provides` an implementation of PlatformMBeanProvider is the
right way to do this (and the qualified export to get access to the
service type is correct too).

-Alan
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mandy Chung
In reply to this post by Vladimir Kozlov
Vladimir,

I believe you don’t want to add the dependency from JVMCI to java.management.  Otherwise, JVMCI can’t run with only java.base.  Is the MBean in jdk.internal.vm.compiler or in Lab’s Graal compiler?

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/src/jdk/vm/ci/services/internal/JVMCIMXBeans.java
- I suspect this file meant to be in src/jdk.internal.vm.ci/share/classes/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/internal/JVMCIMXBeans.java

Mandy

> On Jul 12, 2017, at 2:20 PM, Vladimir Kozlov <[hidden email]> wrote:
>
> https://bugs.openjdk.java.net/browse/JDK-8182701
> webrev:
> http://cr.openjdk.java.net/~kvn/8182701/webrev.jdk/
> http://cr.openjdk.java.net/~kvn/8182701/webrev.hs/
>
> Contributed by Jaroslav Tulach.
>
> JDK itself contains quite a lot of platform MBeans which get registered "on demand". Graal compiler (jdk.internal.vm.compiler) provides its own MBean(s) - however currently there is no way to register it "on demand". JDK9 already contains support for collecting platform MBeans from various modules. We just need to expose Graal MBean through JVMCI.
>
> Thanks,
> Vladimir

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Doug Simon @ Oracle

> On 25 Jul 2017, at 01:37, Mandy Chung <[hidden email]> wrote:
>
> Vladimir,
>
> I believe you don’t want to add the dependency from JVMCI to java.management.  Otherwise, JVMCI can’t run with only java.base.

The dependency is unfortunate. Can you suggest how JVMCI platform beans could participate in platform bean registration without the dependency?

>  Is the MBean in jdk.internal.vm.compiler or in Lab’s Graal compiler?

Anything in Lab’s Graal compiler is destined for JDK Graal so the distinction is only temporary at best.

>
> src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/src/jdk/vm/ci/services/internal/JVMCIMXBeans.java
> - I suspect this file meant to be in src/jdk.internal.vm.ci/share/classes/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/internal/JVMCIMXBeans.java

Not sure I follow - there is currently no such directory src/jdk.internal.vm.ci/share/classes/src

-Doug

>> On Jul 12, 2017, at 2:20 PM, Vladimir Kozlov <[hidden email]> wrote:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8182701
>> webrev:
>> http://cr.openjdk.java.net/~kvn/8182701/webrev.jdk/
>> http://cr.openjdk.java.net/~kvn/8182701/webrev.hs/
>>
>> Contributed by Jaroslav Tulach.
>>
>> JDK itself contains quite a lot of platform MBeans which get registered "on demand". Graal compiler (jdk.internal.vm.compiler) provides its own MBean(s) - however currently there is no way to register it "on demand". JDK9 already contains support for collecting platform MBeans from various modules. We just need to expose Graal MBean through JVMCI.
>>
>> Thanks,
>> Vladimir
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mandy Chung

> On Jul 25, 2017, at 1:33 AM, Doug Simon <[hidden email]> wrote:
>
>
>> On 25 Jul 2017, at 01:37, Mandy Chung <[hidden email]> wrote:
>>
>> Vladimir,
>>
>> I believe you don’t want to add the dependency from JVMCI to java.management.  Otherwise, JVMCI can’t run with only java.base.
>
> The dependency is unfortunate. Can you suggest how JVMCI platform beans could participate in platform bean registration without the dependency?
>

If it exposes a MBean, the dependency would be needed.

One consideration might be to separate the JVMCI MBean provider in its own module so that it’s registered only when such module is resolved in the module graph.  This way JVMCI can work even if java.management is not present.

>> Is the MBean in jdk.internal.vm.compiler or in Lab’s Graal compiler?
>
> Anything in Lab’s Graal compiler is destined for JDK Graal so the distinction is only temporary at best.
>

Good to know.

>>
>> src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/src/jdk/vm/ci/services/internal/JVMCIMXBeans.java
>> - I suspect this file meant to be in src/jdk.internal.vm.ci/share/classes/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/internal/JVMCIMXBeans.java
>
> Not sure I follow - there is currently no such directory src/jdk.internal.vm.ci/share/classes/src

Typo - it’s an existing directory:

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.services/src/jdk/vm/ci/services/internal

Mandy

>
> -Doug
>
>>> On Jul 12, 2017, at 2:20 PM, Vladimir Kozlov <[hidden email]> wrote:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8182701
>>> webrev:
>>> http://cr.openjdk.java.net/~kvn/8182701/webrev.jdk/
>>> http://cr.openjdk.java.net/~kvn/8182701/webrev.hs/
>>>
>>> Contributed by Jaroslav Tulach.
>>>
>>> JDK itself contains quite a lot of platform MBeans which get registered "on demand". Graal compiler (jdk.internal.vm.compiler) provides its own MBean(s) - however currently there is no way to register it "on demand". JDK9 already contains support for collecting platform MBeans from various modules. We just need to expose Graal MBean through JVMCI.
>>>
>>> Thanks,
>>> Vladimir
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Alan Bateman
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).


>
>> If it exposes a MBean, the dependency would be needed.
> Isn't there a way to require an optional dependency? That would be sufficient -
> as the classes in question are only loaded once java.management searches for
> them. E.g. only when java.management is installed.
There is `requires static` but it would be a lot cleaner if the
management support could be refactored into its own service provider
module, meaning JVMCIMXBeans would be in its own module.

-Alan
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mandy Chung
In reply to this post by Mandy Chung

> On Jul 27, 2017, at 2:07 AM, Jaroslav Tulach <[hidden email]> wrote:
>
>> One consideration might be to separate the JVMCI MBean provider in its own
>> module so that it’s registered only when such module is resolved in the
>> module graph.  This way JVMCI can work even if java.management is not
>> present.
>
> Yes, I can create something like jdk.internal.vm.ci.management and move the
> JVMCIMXBeans.java (to be renamed to JVMCIMBeans.java per Vladimir's
> suggestion, if I remember correctly) there. This module would have dependency
> on jdk.internal.vm.ci and java.management and bridge them together.
>
> How do I make sure this module is enabled when possible? Or is that automatic?
>

Service binding will resolve a service provider module during resolution.  In other words, if java.management is in the resolved graph, any modules providing the services it uses will be added to the graph.

>>>> Is the MBean in jdk.internal.vm.compiler or in Lab’s Graal compiler?
>>>
>>> Anything in Lab’s Graal compiler is destined for JDK Graal so the
>>> distinction is only temporary at best.
>> Good to know.
>
> The bean is in there and implements DynamicMBean interface. E.g. this part of
> Graal compiler module has to have dependency on java.management - that means
> to make that dependency optional or split the module in two, I assume.

This is Graal-specific MBean.  It doesn’t seem that it must be registered as “platform mbean” which has to implement PlatformManagedObject.

Graal can register the MBean at runtime when java.management is present by calling ManagementFactory.getPlatformMBeanServer().registerMBean method.  That seems to be a better alternative.  Separating the MBean in a different module would still be applicable.

Mandy
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mandy Chung

> On Aug 2, 2017, at 7:08 AM, Jaroslav Tulach <[hidden email]> wrote:
>
>> This is Graal-specific MBean.  It doesn’t seem that it must be registered as
>> “platform mbean” which has to implement PlatformManagedObject.
>>
>> Graal can register the MBean at runtime when java.management is present by
>> calling ManagementFactory.getPlatformMBeanServer().registerMBean method.
>> That seems to be a better alternative.  Separating the MBean in a different
>> module would still be applicable.
>
> This is not possible to do cleanly. When shall Graal register the bean? On
> first compilation? Then it may enable the whole ManagementFactory
> infrastructure too early and influence results of regular Java programs.
>
> We have seen a benchmark failure due to registering the Graal bean too early.
> The bean registration triggered the java.util.logging infrastructure on and as
> a result the benchmarking test failed.
>
> The test tried to set "java.util.logging.config.file" property and then it
> assumed it will be used on subsequent call to Logger.getLogger(...). But the
> property was ignored as the Logger was already initialized.
>
> I believe that exactly for this reason there is the PlatformManagedObject &
> co. infrastructure. To not trigger the management infrastructure on
> prematurely. All the HotSpot beans are registered "lazily". As part of
> internal JDK infrastructure we believe Graal should have a way to be part of
> such "lazy initialization" too.
>
> I hope the need for the requested functionality is now clearer.

As described in the previous mail, the current way to register a platform
MBean is to have a provider module that provides
sun.management.spi.PlatformMBeanProvider.  The provider module will get
resolved during service binding.  jdk.management is one example.

Mandy

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Vladimir Kozlov
In reply to this post by Alan Bateman
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/ca9071941a1be7f1a3725529ecc231ff621d5ed0
> 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
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Vladimir Kozlov
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/ca9071941a1be7f1a3725529ecc231ff621d5ed0
>> 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
>>
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Doug Simon @ Oracle
Hi Jaroslav,

In general, thanks for pushing through with this change.

I don't think JVMCIMXBeans should be in the hotspot-agnostic jdk.vm.ci.services.internal package since it has a direct reference to HotSpotJVMCIRuntime. I would suggest moving it to jdk.vm.ci.hotspot.

In HotSpotJVMCRuntime.java:

 558     public String mbeanName() {
 568     public Object mbean() {

Any reason these methods don't follow the conventions of other getter methods in this class (i.e. getMBeanName() and getMBean())?

  95     /** Name of the {@link MBeanInfo MBean} representing the internals

This should be:

/**
 * Gets the name of the ...

Same comment for mbean() method. Also {@code null} is used in JVMCI instead of <code>null</code>.

-Doug

> On 18 Aug 2017, at 20:49, Vladimir Kozlov <[hidden email]> 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/ca9071941a1be7f1a3725529ecc231ff621d5ed0
>>> 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
>>>
>>>

Loading...