Re: jmx-dev RFR 8147987: Remove sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java from problemList

2016-02-22 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 19 feb. 2016, at 03:57, Jaroslav Bachorik > wrote: > > Please, review the following change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8147987 > Webrev: http://cr.openjdk.java.net/~jbachorik/8147987/webrev.00 > > This change removes > sun/managemen

Re: jmx-dev [PING] Dropping support for the IIOP transport from the RMI connector

2015-05-26 Thread Staffan Larsen
Hi Steven, We started evaluating the inclusion of JMXMP in the JDK last year for the reasons you site. Unfortunately we came to the conclusion that the JMXMP code base in its current form was not suited for inclusion in the JDK. Improving the code base to modernize and remove technical debt was

Re: jmx-dev RFR 8057149: sun/management/jmxremote/startstop/JMXStartStopTest.java fails with "Starting agent on port ... should report port in use"

2014-09-24 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 24 sep 2014, at 11:35, Jaroslav Bachorik wrote: > Thanks, Staffan > > On 09/23/2014 08:20 PM, Staffan Larsen wrote: >> I think we should add some logging to say that we are retrying. >> >> nit: some weird indention on line 62

Re: jmx-dev RFR 8057149: sun/management/jmxremote/startstop/JMXStartStopTest.java fails with "Starting agent on port ... should report port in use"

2014-09-23 Thread Staffan Larsen
I think we should add some logging to say that we are retrying. nit: some weird indention on line 623. Otherwise looks good. /Staffan On 23 sep 2014, at 17:59, Jaroslav Bachorik wrote: > Please, review this test change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8057149 > Webrev: h

Re: jmx-dev RFR 8057134: sun/management/jmxremote/startstop/JMXStartStopTest.java failing intermittently

2014-09-03 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 3 sep 2014, at 16:02, Jaroslav Bachorik wrote: > Please, review this test change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8057134 > Webrev: http://cr.openjdk.java.net/~jbachorik/8057134/webrev.02 > > Currently the test expects one of the following e

Re: jmx-dev RFR 8057150: Add more diagnostics to JMXStartStopTest

2014-09-03 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 3 sep 2014, at 15:43, Jaroslav Bachorik wrote: > Please, review this trivial test change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8057150 > Webrev: http://cr.openjdk.java.net/~jbachorik/8057150/webrev.00 > > This change is to provide us with more in

Re: jmx-dev RFR 8040692: [TESTBUG] sun/management/jmxremote/bootstrap/JvmstatCountersTest.java requires -XX:+UsePerfData option to pass on embedded platforms

2014-08-21 Thread Staffan Larsen
Looks good, except I don’t think you wanted to add the test/sources.list file? /Staffan On 21 aug 2014, at 14:44, Jaroslav Bachorik wrote: > Please, review this simple fix. > > Issue : https://bugs.openjdk.java.net/browse/JDK-8040692 > Webrev: http://cr.openjdk.java.net/~jbachorik/8040692/web

Re: jmx-dev RFR 8052961: Test "com/sun/tools/attach/StartManagementAgent.java" failing intermittently

2014-08-12 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 29 jul 2014, at 15:47, Jaroslav Bachorik wrote: > Please, review this (hopefully last) change to StartManagementAgent test > > Issue : https://bugs.openjdk.java.net/browse/JDK-8052961 > Webrev: http://cr.openjdk.java.net/~jbachorik/8052961/webrev.00 > > The te

Re: jmx-dev RFR 8044427: [test] sun/management/jmxremote/startstop/JMXStartStopTest times out intermittently on Solaris/Sparcv9

2014-07-01 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 1 jul 2014, at 16:56, Jaroslav Bachorik wrote: > Please, review this simple test fix. > > Issue : https://bugs.openjdk.java.net/browse/JDK-8044427 > Webrev: http://cr.openjdk.java.net/~jbachorik/8044427/webrev.00 > > The test fails intermittently on Solaris 10

Re: jmx-dev RFR 8038794: java/lang/management/ThreadMXBean/SynchronizationStatistics.java fails intermittently

2014-07-01 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 1 jul 2014, at 10:48, Jaroslav Bachorik wrote: > On 07/01/2014 08:05 AM, Staffan Larsen wrote: >> Jaroslav, >> >> I wonder about this snippet: >> >> 167 ti1 = mbean.getThreadInfo(tid); >> 168 testBl

Re: jmx-dev RFR 8038794: java/lang/management/ThreadMXBean/SynchronizationStatistics.java fails intermittently

2014-06-30 Thread Staffan Larsen
Jaroslav, I wonder about this snippet: 167 ti1 = mbean.getThreadInfo(tid); 168 testBlocked(ti, () -> mbean.getThreadInfo(tid), lockName, lock1); 169 ti = ti1; When ti is used later (line 177) it may not have the values of the ThreadInfo that was actually ok:ed by test

Re: jmx-dev RFR 8038826: sun/management/jmxremote/startstop/JMXStartStopTest.java fails with "should report port in use"

2014-06-24 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 24 jun 2014, at 20:08, Jaroslav Bachorik wrote: > Please, review the following test fix. > > Issue : https://bugs.openjdk.java.net/browse/JDK-8038826 > Webrev: http://cr.openjdk.java.net/~jbachorik/8038826/webrev.00 > > The test used to use a final array to pa

Re: jmx-dev Codereview request: JDK-8044865 Fix raw and unchecked lint warnings in management-related code

2014-06-11 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 12 jun 2014, at 08:21, shanliang wrote: > Hi, > > Please review the following fix: > > webrev: http://cr.openjdk.java.net/~sjiang/JDK-8044865/00/ > > bug: https://bugs.openjdk.java.net/browse/JDK-8044865

Re: jmx-dev RFR 8035395: sun/management/jmxremote/startstop/JMXStartStopTest.java fails intermittently: Port already in use

2014-02-20 Thread Staffan Larsen
Looks good. You have a commented out call to debugPortUsage(pa); which you can remove before pushing. Thanks, /Staffan On 20 feb 2014, at 15:21, Jaroslav Bachorik wrote: > Please, review this test fix. > > Issue : https://bugs.openjdk.java.net/browse/JDK-8035395 > Webrev: http://cr.openjdk.

Re: jmx-dev RFR 8031559: javax/management/monitor/StartStopTest.java fails intermittently

2014-01-21 Thread Staffan Larsen
On 21 jan 2014, at 08:52, Jaroslav Bachorik wrote: > On 21.1.2014 08:48, Staffan Larsen wrote: >> You added: >> >> 30 * @library /lib/testlibrary >> 31 * @run build jdk.testlibrary.Utils >> ... >> 57 import jdk.testlibrary.Utils; >>

Re: jmx-dev [ping] Re: RFR 8022221: Intermittent test failures in sun/management/jmxremote/startstop/JMXStartStopTest.sh

2014-01-21 Thread Staffan Larsen
Looks good! And very sorry for letting this slip. Thanks, /Staffan On 14 jan 2014, at 14:13, Jaroslav Bachorik wrote: > Thanks, Staffan! > > On 14.1.2014 13:13, Staffan Larsen wrote: >> JMXStartStopTest.java:162 >> I see no path that calls testConnect with port == -1

Re: jmx-dev RFR 8031559: javax/management/monitor/StartStopTest.java fails intermittently

2014-01-20 Thread Staffan Larsen
You added: 30 * @library /lib/testlibrary 31 * @run build jdk.testlibrary.Utils ... 57 import jdk.testlibrary.Utils; But I don’t see any actual usage of Utils. Otherwise it looks good. /Staffan On 20 jan 2014, at 16:41, Jaroslav Bachorik wrote: > Please, review the following test

Re: jmx-dev [ping] Re: RFR 8022221: Intermittent test failures in sun/management/jmxremote/startstop/JMXStartStopTest.sh

2014-01-14 Thread Staffan Larsen
JMXStartStopTest.java:162 I see no path that calls testConnect with port == -1, so can we we remove the setting of port to 4567? I don’t like that hardcoded port, and I don’t see it being used. Other than that I think it looks good. /Staffan On 14 jan 2014, at 12:27, Jaroslav Bachorik wrote:

Re: jmx-dev RFR 8031420: sun/management/jmxremote/bootstrap/CustomLauncherTest.java fails on some platforms: Unable to locate 'libjvm.so'

2014-01-09 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 9 jan 2014, at 15:05, Jaroslav Bachorik wrote: > Please, review this small test change. > > Issue : https://bugs.openjdk.java.net/browse/JDK-8031420 > Webrev: http://cr.openjdk.java.net/~jbachorik/8031420/webrev.00 > > The test needs to check for the supported

Re: jmx-dev [ping] Re: RFR 8027163: sun/management/jmxremote/bootstrap/CustomLauncherTest.java should be updated for jdk8 removal of solaris-32bit support

2013-11-18 Thread Staffan Larsen
Reviewed. /Staffan On 18 Nov 2013, at 14:41, Jaroslav Bachorik wrote: > Ok, just to be sure - http://cr.openjdk.java.net/~jbachorik/8027163/webrev.01 > > -JB- > > On 18.11.2013 13:51, Staffan Larsen wrote: >> Ah! Yes. >> >> /S >> >> On 18 Nov

Re: jmx-dev [ping] Re: RFR 8027163: sun/management/jmxremote/bootstrap/CustomLauncherTest.java should be updated for jdk8 removal of solaris-32bit support

2013-11-18 Thread Staffan Larsen
Ah! Yes. /S On 18 Nov 2013, at 13:15, Jaroslav Bachorik wrote: > On 18.11.2013 13:06, Staffan Larsen wrote: >> We don’t have solaris 32bit support any more, so please remove that file. > > In that case I will remove also solaris-i586 > > -JB- > >> >> Ot

Re: jmx-dev [ping] Re: RFR 8027163: sun/management/jmxremote/bootstrap/CustomLauncherTest.java should be updated for jdk8 removal of solaris-32bit support

2013-11-18 Thread Staffan Larsen
We don’t have solaris 32bit support any more, so please remove that file. Otherwise: looks good! /Staffan On 18 Nov 2013, at 13:00, Jaroslav Bachorik wrote: > On 18.11.2013 12:50, Staffan Larsen wrote: >> Shouldn’t test/sun/management/jmxremote/bootstrap/solaris-sparc/launcher be &

Re: jmx-dev [ping] Re: RFR 8027163: sun/management/jmxremote/bootstrap/CustomLauncherTest.java should be updated for jdk8 removal of solaris-32bit support

2013-11-18 Thread Staffan Larsen
Shouldn’t test/sun/management/jmxremote/bootstrap/solaris-sparc/launcher be removed as part of this change? /Staffan On 18 Nov 2013, at 11:09, Jaroslav Bachorik wrote: > Could I get this reviewed, please? > > -JB- > > On 4.11.2013 14:07, Jaroslav Bachorik wrote: >> Please, review the follow

Re: jmx-dev RFR Doclint cleanup of javax.management

2013-11-12 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 12 Nov 2013, at 15:52, roger riggs wrote: > Hi Jaroslav, > > Thanks for the review and comments. > > I derived the changes from the warning messages from -Xdoclint. > > For both and the closing tag is optional in HTML 4.01 Transitional > generated by java

Re: jmx-dev RFR: 6543856: MonitorVmStartTerminate.sh fails intermittently

2013-11-05 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 5 nov 2013, at 11:25, Erik Gahlin wrote: > Could I please have a review of this intermittently failing test. > > Removed Thread.sleep and instead used two count down latches. Also did some > cleaning up; removed an unused import, added generics etc. > > Thanks

Re: jmx-dev RFR: 6959636: testcase failing on windows javax/management/loading/LibraryLoader/LibraryLoaderTest.java,

2013-11-05 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 5 nov 2013, at 10:28, Erik Gahlin wrote: > Could I please have a review, fix is to run test in separate vm. > > I also removed warnings and allowed the exception to propagate. > > Thanks > Erik > > Webrev: > http://cr.openjdk.java.net/~egahlin/6959636_1/ > >

Re: jmx-dev RFR: 8027209: javax/management/monitor/ThreadPoolAccTest.java fails intermittently

2013-11-05 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 5 nov 2013, at 12:15, Erik Gahlin wrote: > Thanks for the review, here is an updated webrev. > > http://cr.openjdk.java.net/~egahlin/8027209_4/ > > Erik > > Jaroslav Bachorik skrev 2013-11-05 17:30: >> L49-51 The test declaration should go to the original bloc

Re: jmx-dev RFR 7144200: java/lang/management/ClassLoadingMXBean/LoadCounts.java failed with JFR enabled

2013-10-31 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 31 okt 2013, at 11:27, Jaroslav Bachorik wrote: > On 7.10.2013 16:35, Staffan Larsen wrote: >> This will make it less likely for the test to fail, but does not guarantee >> it since there is nothing that says classloading will be done

Re: jmx-dev RFR 8020467: Inconsistency between usage.getUsed() and isUsageThresholdExceeded() with CMS Old Gen pool

2013-10-30 Thread Staffan Larsen
Quoting Bengt from earlier in this conversation: "As for just doing a System.gc() to force a GC I think you can rely on that System.gc() does a full GC in Hotspot unless someone sets -XX:+DisableExplicitGC on the command line. Considering that you are relying on Hotspot specifc names for pools

Re: jmx-dev RFR 8020467: Inconsistency between usage.getUsed() and isUsageThresholdExceeded() with CMS Old Gen pool

2013-10-23 Thread Staffan Larsen
On 23 okt 2013, at 10:12, Jaroslav Bachorik wrote: > On 23.10.2013 10:08, Staffan Larsen wrote: >> I think you can simplify the logic for forcing a GC to just a simple call to >> "System.gc();". AFAIK System.gc() will cause a full collection to happen for >> a

Re: jmx-dev RFR 8020467: Inconsistency between usage.getUsed() and isUsageThresholdExceeded() with CMS Old Gen pool

2013-10-23 Thread Staffan Larsen
I think you can simplify the logic for forcing a GC to just a simple call to "System.gc();". AFAIK System.gc() will cause a full collection to happen for all collectors. /Staffan On 23 okt 2013, at 10:02, Jaroslav Bachorik wrote: > On 22.10.2013 22:04, Mandy Chung wrote: >> Hi Jaroslav, >>

Re: jmx-dev RFR 7197919: java/lang/management/ThreadMXBean/ThreadBlockedCount.java has concurency issues

2013-10-18 Thread Staffan Larsen
Looks good! Nit: for(int i=0;i<100;i++) should have more spaces: for (int i = 0; i < 100; i++) Thanks, /Staffan On 16 okt 2013, at 16:18, Jaroslav Bachorik wrote: > Please, review this simple test change. > > The test tries to get the number of times a certain thread was blocked during >

Re: jmx-dev [PING] RFR: 8024613 javax/management/remote/mandatory/connection/RMIConnector_NPETest.java failing intermittently

2013-10-18 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 16 okt 2013, at 16:44, Jaroslav Bachorik wrote: > On 2.10.2013 12:55, Jaroslav Bachorik wrote: >> On 20.9.2013 14:54, shanliang wrote: >>> Jaroslav, >>> >>> It is a good idea to use the RMI Testlibrary. >>> >>> Better to call: >>>agent.close(); >>> >>

Re: jmx-dev RFR 6523160: RuntimeMXBean.getUptime() returns negative values

2013-10-18 Thread Staffan Larsen
er clock_getres says >>>>>> elapsed_time -> counter/frequency -> ??? >>>>>> >>>>>> So elapsed_time not, in general, going to give the elapsed time in >>>>>> seconds. And elapsed_time is not dependent on the "frequency"

Re: jmx-dev RFR 6523160: RuntimeMXBean.getUptime() returns negative values

2013-10-10 Thread Staffan Larsen
On 10 okt 2013, at 13:02, Jaroslav Bachorik wrote: > On 10.10.2013 05:44, David Holmes wrote: >> On 10/10/2013 4:12 AM, Staffan Larsen wrote: >>> >>> On 9 okt 2013, at 16:19, Jaroslav Bachorik >>> wrote: >>> >>>> On 9.10.2013 16:10,

Re: jmx-dev RFR 6523160: RuntimeMXBean.getUptime() returns negative values

2013-10-09 Thread Staffan Larsen
On 9 okt 2013, at 16:19, Jaroslav Bachorik wrote: > On 9.10.2013 16:10, Staffan Larsen wrote: >> There is now an awful amount of different timestamps in the Management >> class. Can they be consolidated somehow? At least _begin_vm_creation_time >> and the new

Re: jmx-dev RFR 6523160: RuntimeMXBean.getUptime() returns negative values

2013-10-09 Thread Staffan Larsen
There is now an awful amount of different timestamps in the Management class. Can they be consolidated somehow? At least _begin_vm_creation_time and the new _begin_vm_creation_ns. This discussion also implies that the "elapsed time" we print in the hs_err file is also wrong. It uses os::elapsed

Re: jmx-dev RFR 7144200: java/lang/management/ClassLoadingMXBean/LoadCounts.java failed with JFR enabled

2013-10-07 Thread Staffan Larsen
This will make it less likely for the test to fail, but does not guarantee it since there is nothing that says classloading will be done in 300 ms. Any failures will unfortunately be harder to reproduce. (And the test is now 300 ms slower to run.) A different solution is to only allow the numbe

Re: jmx-dev RFR 6523160: RuntimeMXBean.getUptime() returns negative values

2013-10-02 Thread Staffan Larsen
Looks good! Thanks, /Staffan On 2 okt 2013, at 10:47, Jaroslav Bachorik wrote: > Hello, > > currently the JVM uptime reported by the RuntimeMXBean is based on > System.currentTimeMillis() which makes it susceptible to changes of the OS > time (eg. changing timezone, NTP synchronization etc.)

Re: jmx-dev RFR: 8002307 javax.management.modelmbean.ModelMBeanInfoSupport may expose internal representation by storing an externally mutable object

2013-05-28 Thread Staffan Larsen
Looks good. /Staffan On 28 maj 2013, at 15:04, Jaroslav Bachorik wrote: > Please, review the fix for JDK-8002307. > > The fix assures the immutability by cloning the provided arrays in the > constructor and then cloning them again in the getters. > > The constructors are fixed in the javax/m