Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-02-03 Thread Mandy Chung
On 2/3/15 5:16 AM, shanliang wrote: Hi, Hope this is last time :) http://cr.openjdk.java.net/~sjiang/JDK-8065213/02/ Looks good. Some lines are quite long that are better to reformat (no need to send a new webrev). Mandy

Re: jmx-dev RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-02-03 Thread Daniel Fuchs
Looks good to me Shanliang. -- daniel On 03/02/15 14:16, shanliang wrote: Hi, Hope this is last time :) http://cr.openjdk.java.net/~sjiang/JDK-8065213/02/ 1) Mandy Chung wrote: You may want to consider using limited doPrivileged (that can be done in the second phase). Done as in Managem

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-02-03 Thread shanliang
Hi, Hope this is last time :) http://cr.openjdk.java.net/~sjiang/JDK-8065213/02/ 1) Mandy Chung wrote: You may want to consider using limited doPrivileged (that can be done in the second phase). Done as in ManagementFactory: 878 // get all providers 879 List provider

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-30 Thread Mandy Chung
On 1/30/2015 9:38 AM, shanliang wrote: Thanks for all your comments, here is the new version: http://cr.openjdk.java.net/~sjiang/JDK-8065213/01/ Thank you for the update. Looks fine to me. Minor comment: PlatformMBeanProviderImpl.java line 43: better to rename it as "list" or other name.

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-30 Thread Jaroslav Bachorik
On 30.1.2015 20:41, Daniel Fuchs wrote: On 30/01/15 20:05, Jaroslav Bachorik wrote: On 30.1.2015 19:52, Daniel Fuchs wrote: Hi Jaroslav, On 30/01/15 19:40, Jaroslav Bachorik wrote: Hi, On 30.1.2015 18:38, shanliang wrote: Hi, Thanks for all your comments, here is the new version: http:

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-30 Thread Daniel Fuchs
On 30/01/15 20:05, Jaroslav Bachorik wrote: On 30.1.2015 19:52, Daniel Fuchs wrote: Hi Jaroslav, On 30/01/15 19:40, Jaroslav Bachorik wrote: Hi, On 30.1.2015 18:38, shanliang wrote: Hi, Thanks for all your comments, here is the new version: http://cr.openjdk.java.net/~sjiang/JDK-8065213

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-30 Thread Jaroslav Bachorik
On 30.1.2015 19:52, Daniel Fuchs wrote: Hi Jaroslav, On 30/01/15 19:40, Jaroslav Bachorik wrote: Hi, On 30.1.2015 18:38, shanliang wrote: Hi, Thanks for all your comments, here is the new version: http://cr.openjdk.java.net/~sjiang/JDK-8065213/01/ * java/lang/management/ManagementFacto

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-30 Thread Daniel Fuchs
Hi Jaroslav, On 30/01/15 19:40, Jaroslav Bachorik wrote: Hi, On 30.1.2015 18:38, shanliang wrote: Hi, Thanks for all your comments, here is the new version: http://cr.openjdk.java.net/~sjiang/JDK-8065213/01/ * java/lang/management/ManagementFactory.java L775-788 Can't you use 'return ge

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-30 Thread Jaroslav Bachorik
Hi, On 30.1.2015 18:38, shanliang wrote: Hi, Thanks for all your comments, here is the new version: http://cr.openjdk.java.net/~sjiang/JDK-8065213/01/ * java/lang/management/ManagementFactory.java L775-788 Can't you use 'return getPlatformComponents().stream().flatMap(p->getProxyNames(p,

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-30 Thread shanliang
Hi, Thanks for all your comments, here is the new version: http://cr.openjdk.java.net/~sjiang/JDK-8065213/01/ Mandy Chung wrote: ManagementFactory.java line 760: would be helpful to add some comments describing that the implementation of this method. You can use MemoryManagerMXBea

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-29 Thread Mandy Chung
On 1/29/15 3:07 AM, Daniel Fuchs wrote: On 28/01/15 07:46, Mandy Chung wrote: com/sun/management/internal/PlatformMBeanProviderImpl.java line 43: does this mxbeanList have to be created lazily? Would it be better to make it a final field and create it at the constructor? Hi Mandy, I w

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-29 Thread Daniel Fuchs
On 28/01/15 07:46, Mandy Chung wrote: com/sun/management/internal/PlatformMBeanProviderImpl.java line 43: does this mxbeanList have to be created lazily? Would it be better to make it a final field and create it at the constructor? Hi Mandy, I was the one to suggest the lazy initializa

Re: jmx-dev RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-28 Thread Daniel Fuchs
Hi Shanliang, This looks good. ManagementFactory.java: line 871: there's a stray debug trace that you probably forgot to remove: 871 System.out.println("\n\n= jsl adding: "+provider); lines 877-885: I believe it would improved readability if the content of the forEachOrd

Re: RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-27 Thread Mandy Chung
On 1/27/2015 9:43 AM, shanliang wrote: Hi, Please review: bug: https://bugs.openjdk.java.net/browse/JDK-8065213 webrev: http://cr.openjdk.java.net/~sjiang/JDK-8065213/00/ The class PlatformMBeanProvider.java is added to allow different modules to inform ManagementFactory of their Plaform M

RFR: 8065213 Specify and implement PlatformMBeanProvider for looking for all platform MBeans

2015-01-27 Thread shanliang
Hi, Please review: bug: https://bugs.openjdk.java.net/browse/JDK-8065213 webrev: http://cr.openjdk.java.net/~sjiang/JDK-8065213/00/ The class PlatformMBeanProvider.java is added to allow different modules to inform ManagementFactory of their Plaform MBeans. In this version we use the Servic