Re: RFR(XS): 8066814: Reduce accessibility in TraceEvent

2014-12-18 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 17 dec 2014, at 15:45, Markus Grönlund wrote: > > Greetings, > > Kindly asking for reviews for the following changeset: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8066814 > > Webrev: http://cr.openj

Re: RFR(S): backport 8039995 and 8061785 changes to SA testcase

2014-12-18 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 17 dec 2014, at 17:15, KEVIN WALLS wrote: > > Hi, > > I'd like a review of a backport of these test changes, into 8u. > > webrev: > http://cr.openjdk.java.net/~kevinw/8039995_8061785/webrev.00/ > > Changes simply hg imported from: > > 8039995 > http://hg.op

Re: RFR(L): 8049716: PPC64: Implement SA on Linux/PPC64

2014-12-18 Thread Staffan Larsen
Since both Volker and Dmitry have reviewed this, no further reviews are necessary. I took a look at the changes in shared code, anyway, and it looks good. Thanks, /Staffan > On 17 dec 2014, at 19:06, Dmitry Samersoff > wrote: > > Volker, > > The changes looks good for me and I'll sponsor th

Re: RFR(XXS): 8065226: sun/jvmstat/monitor/MonitoredVm/CR6672135.java should be quarantined

2015-01-08 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 8 jan 2015, at 09:17, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this very small fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8065226 > webrev: http://cr.openjdk.java.net/~ykantser/8065226/webrev.00 > > Thanks, > Ka

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-01-09 Thread Staffan Larsen
It’s getting difficult to get all the information into the same output: hierarchy, interfaces, class loaders and modules. I took a stab at it and it could look like this: java.lang.Object |--java.io.Serializable (java.base, 0x0007c00375f8, iface) |--java.util.RandomAccess (java.base, 0x0

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-01-09 Thread Staffan Larsen
> On 9 jan 2015, at 18:49, Karen Kinnear wrote: > > Staffan, > > On Jan 9, 2015, at 7:38 AM, Staffan Larsen wrote: > >> It’s getting difficult to get all the information into the same output: >> hierarchy, interfaces, class loaders and modules. I took a stab at

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-01-12 Thread Staffan Larsen
> On 9 jan 2015, at 21:08, Chris Plummer wrote: > > On 1/9/15 4:38 AM, Staffan Larsen wrote: >> It�s getting difficult to get all the information into the same output: >> hierarchy, interfaces, class loaders and modules. I took a stab at it and it &g

Re: RFR: 6588467: Add isDaemon() and getPriority() to ThreadInfo

2015-01-12 Thread Staffan Larsen
> On 13 jan 2015, at 06:28, David Holmes wrote: > > Hi Jeremy, > > On 13/01/2015 4:32 AM, Jeremy Manson wrote: >> Hi folks, >> >> This was long forgotten, seems to have been lost in the shuffle. I've >> done it, since we could use it, too: >> >> https://bugs.openjdk.java.net/browse/JDK-65884

Re: RFR 8067447: Factor out the shared implementation of the VM flags manipulation code

2015-01-12 Thread Staffan Larsen
> On 8 jan 2015, at 14:24, Jaroslav Bachorik > wrote: > > On 8.1.2015 12:12, David Holmes wrote: >> On 8/01/2015 7:22 PM, Jaroslav Bachorik wrote: >>> On 8.1.2015 03:45, David Holmes wrote: On 8/01/2015 1:59 AM, Jaroslav Bachorik wrote: > On 7.1.2015 02:31, David Holmes wrote: >> O

Re: RFR 8067447: Factor out the shared implementation of the VM flags manipulation code

2015-01-13 Thread Staffan Larsen
> On 13 jan 2015, at 13:21, David Holmes wrote: > > Hi Staffan, > > On 13/01/2015 5:26 PM, Staffan Larsen wrote: >> >>> On 8 jan 2015, at 14:24, Jaroslav Bachorik >>> wrote: >>> >>> On 8.1.2015 12:12, David Holmes wrote: >>>

Re: RFR 8067447: Factor out the shared implementation of the VM flags manipulation code

2015-01-14 Thread Staffan Larsen
> On 14 jan 2015, at 08:50, David Holmes wrote: > > Hi Staffan, > > On 13/01/2015 10:27 PM, Staffan Larsen wrote: >> >>> On 13 jan 2015, at 13:21, David Holmes >> <mailto:david.hol...@oracle.com>> wrote: >>> >>> Hi Staffan, >&

Re: RFR: JDK-8067479: verify-modules fails in bootcycle build

2015-01-19 Thread Staffan Larsen
SA changes look ok - the IA64 stuff isn’t needed as we don’t support it and will remove it. /Staffan > On 19 jan 2015, at 09:29, Erik Joelsson wrote: > > Hello, > > Any chance someone from serviceability could take a look at this? > > /Erik > > On 2015-01-12 03:45, David Holmes wrote: >> Hi

Re: RFR(XXS): 8069296: java/lang/management/MemoryMXBean/LowMemoryTest.java should be quarantined

2015-01-21 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 21 jan 2015, at 13:25, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this very small fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8069296 > webrev: http://cr.openjdk.java.net/~ykantser/8069296/webrev.00/ > > Thanks, >

Re: RFR: JDK-8067945: SVC jdk/test/* should be cleaned from JRE layout dependency (corrected per the review findings)

2015-01-22 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 22 jan 2015, at 11:59, Alexander Kulyakhtin > wrote: > > > Could someone from the reviewers group, please, review the changes, if > possible, so they can be pushed? > > bug: https://bugs.openjdk.java.net/browse/JDK-8067945 > webrev: http://cr.openjdk.java

Re: RFR(S): 8044419: TEST_BUG: com/sun/jdi/JdbReadTwiceTest.sh fails when run under root

2015-01-23 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 23 jan 2015, at 09:51, Yekaterina Kantserova > wrote: > > Hi, > > New webrev can be found here > http://cr.openjdk.java.net/~ykantser/8044419/webrev.03/ > The fix has been tested on all platforms except embedded. > > Thanks, > Katja > > > > On 01/21/2015

Re: RFR: JDK-8068589: GCCause should distinguish jcmd GC.run from System.gc()

2015-01-26 Thread Staffan Larsen
A bit of terminology here. ‘GC.run’ is called a ‘Diagnostic Command’. ‘jcmd’ is one way to invoke a Diagnostic Command. Another way is via a JMX MBean. With this in mind I think your references to ‘jcmd’ should be changed to ‘diagnostic command’ (or an abbreviation thereof). For example: _jcmd_g

Re: RFR(XS): 8068778: [TESTBUG] CompressedClassSpaceSizeInJmapHeap.java fails if SA not available

2015-01-26 Thread Staffan Larsen
Change looks good to me - I’ll sponsor it! /Staffan > On 26 jan 2015, at 09:20, Lindenmaier, Goetz > wrote: > > Hi, > > could please somebody from servicability sponsor this tiny change? > > (My first mail didn't go through, because I wasn't yet registered to > servicability-dev). > > Than

Re: RFR(XXS): 8071324: com/sun/jdi/ConnectedVMs.java should be quarantined

2015-01-26 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 26 jan 2015, at 08:50, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this very small fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8071324 > webrev: http://cr.openjdk.java.net/~ykantser/8071324/webrev.00/ > > Thanks, >

Re: RFR: JDK-8068589: GCCause should distinguish jcmd GC.run from System.gc()

2015-01-26 Thread Staffan Larsen
gt; } else { > output()->print_cr("%s", error); > } > } > } > > For example. For consistency, how would you suggest these be handled. > > Kind regards, > Kirk Pepperdine > > On Jan 26, 2015, at 9:12 AM, Staffan Larsen <mailto:staffan.l

Re: RFR: JDK-8068589: GCCause should distinguish jcmd GC.run from System.gc()

2015-01-27 Thread Staffan Larsen
gt; prefer to not have the “.” in the string. I would have suggested “GC.run Diagnostic Command”, or “Diagnostic Command: GC.run”. I don’t know what problems a ‘.’ causes. “System.gc()” already has one. /Staffan > > Regards, > Kirk > >> >> >> Could you review i

Re: RFR: JDK-8068589: GCCause should distinguish jcmd GC.run from System.gc()

2015-01-27 Thread Staffan Larsen
> On 27 jan 2015, at 20:57, Kirk Pepperdine wrote: > > > On Jan 27, 2015, at 8:37 PM, Staffan Larsen <mailto:staffan.lar...@oracle.com>> wrote: > >> >>> On 27 jan 2015, at 18:00, Kirk Pepperdine wrote: >>> >>> Hi, >>> >

Re: RFR(XXS): 8071545: Tests are still excluded while the appropriate bug has been fixed

2015-01-28 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 27 jan 2015, at 09:28, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this very small fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8071545 > webrev: http://cr.openjdk.java.net/~ykantser/8071545/webrev.00/ > > Thanks, >

Re: RFR: JDK-8068589: GCCause should distinguish jcmd GC.run from System.gc()

2015-01-28 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 28 jan 2015, at 05:48, Yasumasa Suenaga wrote: > > Hi Staffan, Kirk, > > I agree to set "Diagnostic Command" to GCCause. > So I applied it to new patch. > > http://cr.openjdk.java.net/~ysuenaga/JDK-8068589/webrev.02/ > > Could you review it again? > > > Th

Re: RFR(S): 8068613: Wrong number of objects pending finalization start

2015-01-28 Thread Staffan Larsen
Looks good! How much testing have you done? Thanks, /Staffan > On 28 jan 2015, at 11:31, Mattias Tobiasson > wrote: > > Hi, > Could I please have a review of this test bug fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8068613 > webrev: http://cr.openjdk.java.net/~ykantser/8068613/web

Re: RFR(S): 8068613: Wrong number of objects pending finalization start

2015-01-28 Thread Staffan Larsen
Sounds good to me. /Staffan > On 28 jan 2015, at 13:37, Mattias Tobiasson > wrote: > > I have tested 200 times on linux, and once on all other platforms (except > embedded). > > On 01/28/2015 11:38 AM, Staffan Larsen wrote: >> Looks good! How much testing h

Re: RFR: JDK-8071464: Clear up SVC jdk/test/* JRE layout dependencies other than those on tools.jar

2015-01-29 Thread Staffan Larsen
Looks good! nit: test/com/sun/jdi/ShellScaffold.sh:947, the alignment for ">&2” got broken. Thanks, /Staffan > On 28 jan 2015, at 17:24, Alexander Kulyakhtin > wrote: > > Hi, > > Could you, please, review the fix below. > > bug: https://bugs.openjdk.java.net/browse/JDK-8071464 > webrev: ht

Re: RFR 8068976: Remove JSDT implementation

2015-01-29 Thread Staffan Larsen
Looks good to me. Thanks, /Staffan > On 14 jan 2015, at 15:05, Jaroslav Bachorik > wrote: > > Please, review the following removal of functionality. > > Issue : https://bugs.openjdk.java.net/browse/JDK-8068976 > Webrev: http://cr.openjdk.java.net/~jbachorik/8068976/webrev.00 > > JDK-8062303

Re: RFR(XXS): 8071784: serviceability/attach/AttachWithStalePidFile.java should be quarantined

2015-01-29 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 29 jan 2015, at 14:23, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this very small fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8071784 > webrev: http://cr.openjdk.java.net/~ykantser/8071784/webrev.00/ > > Thanks, >

Re: RFR(XS): 8072401, 8072403, 8072405: Small fixes to newly added DCMD tests

2015-02-03 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 3 feb 2015, at 16:27, Mikael Auno wrote: > > Hi, could I please have some reviews for these very small fixes. > > Webrev: > http://cr.openjdk.java.net/~miauno/8072401_8072403_8072405/webrev.00/ > > Issues: > > Some of the newly added DCMD tests fail due to l

Re: RFR 8067447: Factor out the shared implementation of the VM flags manipulation code

2015-02-05 Thread Staffan Larsen
Jaroslav, This looks good, just a few small comments: writableFlags.hpp: L25 #ifndef WritableFlags_HPP L26 #define WritableFlags_HPP L96 #endif /* WritableFlags_HPP */ Should be SHARE_VM_SERVICES_WRITABLEFLAG_HPP L32: I don’t like inverted logic such as “NO_ERR”. I would prefer “SUCCESS” inste

Re: RFR 8062303: Remove com.sun.tracing API

2015-02-05 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 5 feb 2015, at 16:06, Jaroslav Bachorik > wrote: > > On 5.2.2015 10:48, David Holmes wrote: >> On 5/02/2015 7:39 PM, Jaroslav Bachorik wrote: >>> On 5.2.2015 10:26, David Holmes wrote: On 5/02/2015 7:16 PM, Jaroslav Bachorik wrote: > Please, review th

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-02-06 Thread Staffan Larsen
I think this looks good! Perhaps we should give a hint as to what the hex value is? I don’t know if it is best to print this as part of a “header” printed before the rest of the output or if we should include it as part of each line “(classloaderdata*=0x080609c8)”. /Staffan > On 6 feb 2015, at

Re: RFR 8067447: Factor out the shared implementation of the VM flags manipulation code

2015-02-06 Thread Staffan Larsen
> On 6 feb 2015, at 12:54, Jaroslav Bachorik > wrote: > > On 6.2.2015 06:16, David Holmes wrote: >> On 6/02/2015 1:17 AM, Jaroslav Bachorik wrote: >>> On 5.2.2015 14:55, Staffan Larsen wrote: >>>> Jaroslav, >>>> >>>> This look

Re: RFR 8067447: Factor out the shared implementation of the VM flags manipulation code

2015-02-09 Thread Staffan Larsen
> On 9 feb 2015, at 00:13, David Holmes wrote: > > Hi Jaroslav, > > On 6/02/2015 9:54 PM, Jaroslav Bachorik wrote: >> On 6.2.2015 06:16, David Holmes wrote: >>> On 6/02/2015 1:17 AM, Jaroslav Bachorik wrote: >>>> On 5.2.2015 14:55, Staffan Larsen wrot

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-02-09 Thread Staffan Larsen
= > > I can go with "null" for the null ClassLoader if you want. I used it's > ClassLoaderData* to be consistent with the other ClassLoaders. Just let me > know which you prefer. If we change the CLD* to the “Klass of the j.l.ClassLoader”, then it will become “null”. If

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-02-09 Thread Staffan Larsen
|-sun.misc.Launcher$AppClassLoader/0x0007c00375f8 (java.base, >> CLD=0x00087654320) >> |-myapp/0x00087654320 >> >> So now the classname and its classloader id (which is the CLD*) are grouped >> together to make it easy to strip out or to search for in

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-02-10 Thread Staffan Larsen
Chris, In general I think this looks very good. Simple and well-commented code to follow. I am missing a test, though. Please look at the hotspot/test/serviceability/dcmd set of tests. A couple of smaller comments: Are Unsafe.defineAnonymousClass classes included? Should they be? I think Cl

Re: RFR(S, testonly): JDK-8072835 sun/tools/jmap/heapconfig/JMapHeapConfigTest.java Key MaxHeapSize doesn't match

2015-02-10 Thread Staffan Larsen
I don’t think you need to respect -Xmx if it set from outside. We know that this test is not testing -Xmx, it is just using that flag to check that jmap works. You can override the value of -Xmx in two ways: 1) either set your value before or after the other value (I don’t know which value takes

Re: RFR(S, testonly): JDK-8072835 sun/tools/jmap/heapconfig/JMapHeapConfigTest.java Key MaxHeapSize doesn't match

2015-02-10 Thread Staffan Larsen
" + option); mx_found = true; break; } } /S > > If it is not the case - I'll change the code to use > Utils.getFilteredTestJavaOpts() > > -Dmitry > > On 2015-02-10 22:45, Staffan Larsen wrote: >> I don’t think you nee

Re: RFR(S, testonly): JDK-8072835 sun/tools/jmap/heapconfig/JMapHeapConfigTest.java Key MaxHeapSize doesn't match

2015-02-10 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 10 feb 2015, at 21:07, Dmitry Samersoff > wrote: > > Staffan, > > Changed in-place (press shift-reload) > > -Dmitry > > On 2015-02-10 22:59, Staffan Larsen wrote: >> >>> On 10 feb 2015, at 20:52, D

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-02-11 Thread Staffan Larsen
Most changes are discussed inline below. Here are a couple of additional > changes: > > - Changed "transitive interface" to "inherited interface" in the output. > - Went back to using the CLD* instead of the Klass*, except still use "null" > for the boots

Re: RFR(M): JDK-8072395: LingeredAppTest.java and JMapHeapConfigTest.java fail due to LingeredApp ERROR: java.io.IOException: Lock is too old. Aborting

2015-02-12 Thread Staffan Larsen
This looks good, except that I agree with Kevin that we should silently succeed if SA isn’t allowed to attach. Otherwise we will get lots of failures in the testing. Sorry I missed this in the last round of internal review. /Staffan > On 11 feb 2015, at 23:38, Kevin Walls wrote: > > > Hi Dmi

Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy

2015-02-12 Thread Staffan Larsen
Looks good! Thanks for providing incremental webrevs - very helpful! Thanks, /Staffan > On 12 feb 2015, at 09:31, Chris Plummer wrote: > > I suppose it would be nice if I included links to the webrevs: > > http://cr.openjdk.java.net/~cjplummer/8054888/webrev.02-03/ >

Re: RFR(M): JDK-8072395: LingeredAppTest.java and JMapHeapConfigTest.java fail due to LingeredApp ERROR: java.io.IOException: Lock is too old. Aborting

2015-02-12 Thread Staffan Larsen
Looks good. Thanks, /Staffan > On 12 feb 2015, at 10:17, Dmitry Samersoff > wrote: > > Staffan, > > Fixed in place (press shift-reload) > > http://cr.openjdk.java.net/~dsamersoff/JDK-8072395/webrev.04/ > > -Dmitry > > On 2015-02-12 12:03, Staffan Lars

Re: Review request: Backport of JDK-8046282 to 8u

2015-02-12 Thread Staffan Larsen
Hi Poonam, Did the changes apply cleanly? What tests have you run? /Staffan > On 29 jan 2015, at 20:42, Poonam Bajaj Parhar wrote: > > Hello, > > Review request for the backport of 8046282 to 8u: > > Bug: JDK-8046282 SA > update > Webrev:

Re: Review request: Backport of JDK-8046282 to 8u

2015-02-13 Thread Staffan Larsen
k not > working on core files > > which was a regression introduced in jdk9 with the changes of 8046282. > > Testing: JPRT, jstack with core and live process > > Thanks, > Poonam > > On 2/12/2015 11:51 PM, Staffan Larsen wrote: >> Hi Poonam, >> >> Did the

Re: gdb and OpenJDK

2015-02-15 Thread Staffan Larsen
Alexandre, I think what Erik suggested was if there was some way the JVM could expose data in a format that is easy to interpret by other tools (such as the python gdb plugin, but also plugins for other debuggers, or SA). Of course this would have to be data, not code, so that it would be avail

Re: RFR 8072908: 8072908: com/sun/management/OperatingSystemMXBean/TestTotalSwap.sh fails on OS X with exit code 2

2015-02-16 Thread Staffan Larsen
Looks good! (And verified on my mac) Thanks, /Staffan > On 13 feb 2015, at 10:40, Jaroslav Bachorik > wrote: > > On 13.2.2015 03:36, David Holmes wrote: >> Hi Jaroslav, >> >> Looks reasonable - but I can't vouch for MacOSX commands. > > I am not a lucky Mac owner neither. I confirmed the for

Re: RFR: 6588467: Add isDaemon() and getPriority() to ThreadInfo

2015-02-16 Thread Staffan Larsen
<http://cr.openjdk.java.net/~jmanson/6588467/webrev.01/> > > Can anyone drive the API process? It's such a minor change that I can't > imagine it wouldn't go through easily. Of course, I have no idea what's > involved. > > Jeremy > > On Mon,

Re: RFR 8071657:JDI ObjectReferenceImpl.invokeMethod() validation fails for virtual invocations of method with declaring type being an interface

2015-02-18 Thread Staffan Larsen
InterfaceMethodsTest.java:386 Is the ObjectReference.INVOKE_SINGLE_THREADED really needed or should you just pass 0 here? Thanks, /Staffan > On 6 feb 2015, at 15:54, Jaroslav Bachorik > wrote: > > Please, review the following change in JDI implementation > > Issue : https://bugs.openjdk.java

Re: RFR 8071657:JDI ObjectReferenceImpl.invokeMethod() validation fails for virtual invocations of method with declaring type being an interface

2015-02-18 Thread Staffan Larsen
> On 18 feb 2015, at 10:23, Jaroslav Bachorik > wrote: > > On 18.2.2015 10:09, Staffan Larsen wrote: >> InterfaceMethodsTest.java:386 Is the ObjectReference.INVOKE_SINGLE_THREADED >> really needed or should you just pass 0 here? > > The test is single-t

Re: RFR: 6588467: Add isDaemon() and getPriority() to ThreadInfo

2015-02-18 Thread Staffan Larsen
> On 18 feb 2015, at 00:58, Jeremy Manson wrote: > > Since this is not my code, I will happily defer to the people whose code it > is on these matters. I can very easily replace all of the instances, or > just the new ones, or none at all. Just let me know. I would be grateful if you took

Re: RFR: 6588467: Add isDaemon() and getPriority() to ThreadInfo

2015-02-20 Thread Staffan Larsen
; Done. > > http://cr.openjdk.java.net/~jmanson/6588467/webrev.02/ > <http://cr.openjdk.java.net/~jmanson/6588467/webrev.02/> > > Jeremy > > > On Wed, Feb 18, 2015 at 1:49 AM, Staffan Larsen <mailto:staffan.lar...@oracle.com>> wrote: > >> On 18 fe

Re: RFR: 6588467: Add isDaemon() and getPriority() to ThreadInfo

2015-02-23 Thread Staffan Larsen
ogle.com>> wrote: > Okay. Thanks for doing this, Staffan. I do have a "@see Thread#getPriority" > there already. Can I just add "This is an integer between {@linkplain > Thread#MIN_PRIORITY} and {@linkplain Thread#MAX_PRIORITY}, inclusive." to the > getPriority j

Re: RFR: 6588467: Add isDaemon() and getPriority() to ThreadInfo

2015-02-23 Thread Staffan Larsen
I can push this for you. Thanks! /Staffan > On 24 feb 2015, at 05:58, Jeremy Manson wrote: > > Thanks, Mandy. > > I guess it is time to submit. I don't have a committer bit. Any volunteers? > > Thanks to all for the review! > > Jeremy > > On Mon, Feb 23, 2015 at 5:29 PM, Mandy Chung

RFR: 8073713 javadoc warnings in serviceability code

2015-02-24 Thread Staffan Larsen
Please review these small fixes to javadoc in some of the serviceability code. Thanks, /Staffan diff --git a/src/java.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java b/src/java.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java --- a/src/java.man

Re: RFR: 8073688: Infinite loop reading types during jmap attach.

2015-03-04 Thread Staffan Larsen
> On 4 mar 2015, at 12:29, Kevin Walls wrote: > > Thanks Dmitry - > > I will explain the counter's presence where we introduce it: > > // Counter to ensure read loops terminate: > private static final int MAX_DUPLICATE_DEFINITIONS = 100; > private static int duplicateDefCount = 0; Should d

Re: RFR: 8073688: Infinite loop reading types during jmap attach.

2015-03-04 Thread Staffan Larsen
te check in readVMTypes(), we > have the protection we need as this is called first. It is highly unlikely > that would succeed and another of the methods would fail, the other checks > are just making the code consistent. > > Thanks > Kevin > > > > On 04/03/2

Re: RFR: 8073688: Infinite loop reading types during jmap attach.

2015-03-04 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 4 mar 2015, at 15:14, Kevin Walls wrote: > > > Sure, I updated the webrev: > > http://cr.openjdk.java.net/~kevinw/8073688/webrev.01/ > > Thanks > Kevin > > > On 04/03/2015 14:05, Staffan Larsen wrote: >>>

RFR: 8058470 [jconsole] VM Summary Tab is blank for JDK9's jconsole.

2015-03-04 Thread Staffan Larsen
The problem is that the makefiles do "cleanup" of the resource files, accidentally deleting half of some strings. In this case GC_INFO=Name = ''{0}'', Collections = {1,choice,-1#Unavailable|0#{1,number,integer}}, Total time spent = {2} becomes GC_INFO=Name \= ''{0}'', Collections \= {1,choi

Re: RFR (8u): 8073688: Infinite loop reading types during jmap attach.

2015-03-04 Thread Staffan Larsen
Ok with me. Thanks, /Staffan > On 4 mar 2015, at 18:41, Kevin Walls wrote: > > > Hi, > > While this is in memory: I'd like to request review for an 8u backport. > > The changeset imports cleanly into jdk8u/hs-dev/hotspot > > Thanks > Kevin > >

Re: RFR: 8058470 [jconsole] VM Summary Tab is blank for JDK9's jconsole.

2015-03-05 Thread Staffan Larsen
copy the properties files instead of cleaning them, especially if > there are no or few comments in them. > > /Erik > > On 2015-03-04 20:21, Staffan Larsen wrote: >> The problem is that the makefiles do "cleanup" of the resource files, >> accidentally deletin

Re: RFR 8049696: com/sun/jdi/RunToExit fails with "ConnectException: Connection refused"

2015-03-10 Thread Staffan Larsen
When catching the ConnectException, can you print it out as well? It may contain something that is interesting. With that change it looks good to me - no need for an updated review. Thanks, /Staffan > On 9 mar 2015, at 19:40, Jaroslav Bachorik > wrote: > > Please, review the following test c

RFR: 8074812 More specific error message when the .java_pid well-known file is not secure

2015-03-10 Thread Staffan Larsen
During attach, if the .java_pid file is not secure we currently say "well-known file is not secure". This can be enhanced to say _why_ the file is not considered secure. bug: https://bugs.openjdk.java.net/browse/JDK-8074812 webrev: http://cr.openjdk.java.net/~sla/8074812/webrev.00/ Note that it

Re: RFR: JDK-8072754: Getting read of unnecessary sun.misc.Version in the test

2015-03-10 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 10 mar 2015, at 13:38, Alexander Kulyakhtin > wrote: > > Hi, > > Could you, please, review a small fix below. > > bug: https://bugs.openjdk.java.net/browse/JDK-8072754 > webrev: http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8072754/webrev.00/ > > The te

Re: RFR: 8074812 More specific error message when the .java_pid well-known file is not secure

2015-03-10 Thread Staffan Larsen
quot;file's group is not the effective group"; > > Why do you use the word "effective" with the gid, but not the uid? No reason. How do you think I should word it? I’ve been struggling… /Staffan > > > On Tue, Mar 10, 2015 at 3:21 AM, Staffan Larsen <mailto:

Re: RFR(XXS): 8074905: Exclude aarch64 from Visual Studio projectcreator.make

2015-03-11 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 10 mar 2015, at 22:02, Markus Gronlund wrote: > > Greetings, > > > > Please review this small update to the Visual Studio projectcreator.make in > order to exclude aarch64 files. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8074905 > > Webrev:

Re: RFR: 8074812 More specific error message when the .java_pid well-known file is not secure

2015-03-11 Thread Staffan Larsen
Thanks for the feedback. Here is a new version that prints out more details for each of the errors messages. Let me know if you have suggestions for better wording. It also adds an #include for jvm.h that was missing from some of the files (it is needed for jio_snprintf). webrev: http://cr.open

Re: [ping] RFR 8074041: sun/management/jmxremote/startstop/JMXStartStopTest.java fails with InvocationTargetException

2015-03-11 Thread Staffan Larsen
Let try this! /Staffan > On 11 mar 2015, at 09:35, Jaroslav Bachorik > wrote: > > On 5.3.2015 11:37, Jaroslav Bachorik wrote: >> Please, review the following change >> >> Issue : https://bugs.openjdk.java.net/browse/JDK-8074041 >> Webrev: http://cr.openjdk.java.net/~jbachorik/8074041/webrev.0

RFR: 8074948 javadoc typo in DiagnosticCommandMBean.java: {code instead of {@code

2015-03-11 Thread Staffan Larsen
Please review this addition of a missing @. /Staffan diff --git a/src/java.management/share/classes/com/sun/management/DiagnosticCommandMBean.java b/src/java.management/share/classes/com/sun/management/DiagnosticCommandMBean.java --- a/src/java.management/share/classes/com/sun/management/Dia

Re: RFR: 8074812 More specific error message when the .java_pid well-known file is not secure

2015-03-12 Thread Staffan Larsen
or the octal representation 0777 instead which seemed fitting here. new webrev: http://cr.openjdk.java.net/~sla/8074812/webrev.02/ <http://cr.openjdk.java.net/~sla/8074812/webrev.02/> Thanks, /Staffan > > On Wed, Mar 11, 2015 at 2:30 AM, Staffan Larsen <mailto:staffan.l

RFR: Resolve disabled warnings for the JVMTI demos

2015-03-12 Thread Staffan Larsen
Please review these small fixes to remove a couple of warnings in the JVMTI demos. bug: https://bugs.openjdk.java.net/browse/JDK-8074841 bug: https://bugs.openjdk.java.net/browse/JDK-8074842 webrev: http://cr.openjdk.java.net/~sla/8074841/webrev.00/ Thanks, /Staffan

RFR: JDK-8075056 Remove Version.java.template from jconsole

2015-03-12 Thread Staffan Larsen
The build for jconsole currently takes a template file and inserts the version number of the build into the file. We can simplify this by removing the template file and reading the java.runtime.version system property at runtime. bug: https://bugs.openjdk.java.net/browse/JDK-8075056 webrev: http

Re: RFR: 8074812 More specific error message when the .java_pid well-known file is not secure

2015-03-13 Thread Staffan Larsen
Martin, Jaroslav: Thank you! > On 12 mar 2015, at 18:47, Martin Buchholz wrote: > > Looks good to me! > > > On Thu, Mar 12, 2015 at 12:18 AM, Staffan Larsen <mailto:staffan.lar...@oracle.com>> wrote: > >> On 11 mar 2015, at 20:37, Martin Buchholz &

Re: RFR(XS): 8071687: AIX port of "8039173: Propagate errors from Diagnostic Commands as exceptions in the attach framework"

2015-03-17 Thread Staffan Larsen
Looks good to me. Sorry I didn’t do those changes as well. /Staffan > On 17 mar 2015, at 18:40, Volker Simonis wrote: > > Hi, > > can I please have a review for this AIX-only change. > > It is an exact copy of the changes done in "8039173: Propagate errors > from Diagnostic Commands as except

Re: RFR: JDK-8075586: add @modules as needed to the open hotspot tests

2015-03-20 Thread Staffan Larsen
I haven’t looked at the changes in detail, but please change the requiredVersion in TEST.ROOT to 4.1 b11 as part of this change. Thanks, /Staffan > On 20 mar 2015, at 13:16, Alexander Kulyakhtin > wrote: > > Hi, > > Could you, please, review the fix below. > > CR: https://bugs.openjdk.java.

Re: RFR 8054890: Serviceability: New diagnostic commands 'VM.set_flag' and 'JVMTI.data_dump'

2015-03-23 Thread Staffan Larsen
diagnosticCommand.cpp: - Should SetVMFlagDCmd really be inside "#if INCLUDE_SERVICES” ? - L227-234: strange indentation /Staffan > On 19 mar 2015, at 10:59, Jaroslav Bachorik > wrote: > > Please, review the following change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8054890 > Webr

Re: RFR 8054890: Serviceability: New diagnostic commands 'VM.set_flag' and 'JVMTI.data_dump'

2015-03-23 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 23 mar 2015, at 11:55, Jaroslav Bachorik > wrote: > > On 23.3.2015 08:50, Staffan Larsen wrote: >> diagnosticCommand.cpp: >> - Should SetVMFlagDCmd really be inside "#if INCLUDE_SERVICES” ? > > Probably not. On th

Re: RFR 8024055: serviceability/attach/AttachWithStalePidFile.java createJavaPidFile() fails

2015-03-23 Thread Staffan Larsen
Looks good, but please print the exception at line 118 in AttachWithStalePidFile.java. Thanks, /Staffan > On 23 mar 2015, at 12:42, Jaroslav Bachorik > wrote: > > Please, review the following test change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8024055 > Webrev: http://cr.openjdk

Re: RFR 8024055: serviceability/attach/AttachWithStalePidFile.java createJavaPidFile() fails

2015-03-24 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 23 mar 2015, at 20:20, Jaroslav Bachorik > wrote: > > On 23.3.2015 13:44, Staffan Larsen wrote: >> Looks good, but please print the exception at line 118 in >> AttachWithStalePidFile.java. > > Hm, like this http://cr.openj

Re: RFR(XS): 8076154: com/sun/jdi/InstanceFilter.java failing due to missing MethodEntryRequest calls

2015-03-27 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 27 mar 2015, at 14:34, Fredrik Arvidsson > wrote: > > Please review this small test fix for JDK- 8076154. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8076154 > > Webrev: http://cr.openjdk.java.net/~far

Re: RFR 8023093: Add ManagementAgent.status diagnostic command

2015-03-30 Thread Staffan Larsen
Looks good! Thanks for splitting up the test file. This test has been known to be unstable - what platforms have you verified the changes on? Thanks, /Staffan > On 27 mar 2015, at 17:29, Jaroslav Bachorik > wrote: > > Please, review the following change > > Issue : https://bugs.openjdk.ja

Re: RFR 8076344: serviceability/dcmd/vm/SetVMFlagTest.java test fails with "java.lang.Error: 'MaxHeapSize' flag is not available or immutable"

2015-04-01 Thread Staffan Larsen
Looks good! nit: L133: “existeng” -> “existing" Thanks, /Staffan > On 1 apr 2015, at 10:56, Jaroslav Bachorik > wrote: > > Please, review the following test change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8076344 > Webrev: http://cr.openjdk.java.net/~jbachorik/8076344/webrev.00

Re: JEP 240: Remove the JVM TI hprof Agent

2015-04-01 Thread Staffan Larsen
Thanks for the feedback. I still think that the cost and drawbacks of the hprof agent outweighs the benefits and will move ahead with the JEP. It can be noted that since the hprof agent uses the standardized JVMTI interface, it should be perfectly possible to use the library shipped with a pre

Re: RFR(XS): 8027668: sun/tools/jstatd/TestJstatdPort.java: java.net.ConnectException: Connection refused: connect

2015-04-02 Thread Staffan Larsen
What does the name.toString() look like? Should the message be “jstatd listening on …”? > On 2 apr 2015, at 14:56, Yekaterina Kantserova > wrote: > > Jaroslav, > > thanks, will do! > > // Katja > > > > On 04/02/2015 02:44 PM, Jaroslav Bachorik wrote: >> Hi Katja, >> >> On 2.4.2015 14:16

Re: RFR(XS): 8027668: sun/tools/jstatd/TestJstatdPort.java: java.net.ConnectException: Connection refused: connect

2015-04-07 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 7 apr 2015, at 14:52, Yekaterina Kantserova > wrote: > > Hi, > > The updated webrev can be found here: > http://cr.openjdk.java.net/~ykantser/8027668/webrev.01/ > > Thanks, > Katja > > > > On 04/02/2015 02:44 PM, Jaroslav Bachorik wrote: >> Hi Katja, >>

Re: RFR

2015-04-07 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 7 apr 2015, at 17:15, Erik Joelsson wrote: > > Hello, > > When upgrading the toolchain to VS2013, management.dll stopped working on > certain Windows hosts. I've identified this to be related to the call to > GetProcessMemoryInfo. By adding -DPSAPI_VERSION=1

RFR(S): JDK-8077137 Port jdk.internal.instrumentation to jdk 9

2015-04-08 Thread Staffan Larsen
Please review these small changes to support an addition of closed code to the java.instrument module. webrev: http://cr.openjdk.java.net/~sla/8077137-open/webrev.01/ Thanks, /Staffan

RFR: 8075331 jdb eval java.util.Arrays.asList(array) shows inconsistent behaviour

2015-04-08 Thread Staffan Larsen
Hi, When calling varargs methods from JDI, we end up in MethodImpl.handleVarArgs() which has some logic to figure out how to call the varargs method with the parameters given. It depends on InvokeableType.isAssignableTo() to do this. This patch fixes a bug in isAssignableTo() where the implemen

Re: RFR(S): JDK-8044416 serviceability/sa/jmap-hashcode/Test8028623.java fails with AssertionFailure: can not get class data for java/lang/UNIXProcess$Platform$$Lambda

2015-04-09 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 9 apr 2015, at 19:01, Dmitry Samersoff wrote: > > Jaroslav, > > Thank you! > > Nits fixed in-place (press shift-reload) > > -Dmitry > > On 2015-04-09 19:12, Jaroslav Bachorik wrote: >> Hi Dmitry, >> >> Indentation should be 4 spaces. >> Copyright year will

Re: RFR 8076050: java/lang/management/ThreadMXBean/ThreadMXBeanStateTest.java fails intermittently

2015-04-09 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 9 apr 2015, at 20:33, Jaroslav Bachorik > wrote: > > Please, review the following test change > > Issue : https://bugs.openjdk.java.net/browse/JDK-8076050 > Webrev: http://cr.openjdk.java.net/~jbachorik/8076050/webrev.00 > > This is another intermittently fa

Re: RFR(XXS): 8077611: com/sun/jdi/ConnectedVMs.java should be unquarantined

2015-04-15 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 14 apr 2015, at 11:44, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this very small fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8077611 > webrev: http://cr.openjdk.java.net/~ykantser/8077611/webrev.00 > > Thanks, > K

Re: RFR(S): 8077832: SA's dumpreplaydata, dumpcfg and buildreplayjars are broken

2015-04-15 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 15 apr 2015, at 11:48, Roland Westrelin > wrote: > > http://cr.openjdk.java.net/~roland/8077832/webrev.00/ > > I found 3 locations where the SA code is out of sync with the hotspot code. > > Roland.

Re: RFR(XS): 8077423: jstatd is not terminated even though it cannot contact or bind to RMI Registry

2015-04-15 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 15 apr 2015, at 16:46, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8077423 > webrev: http://cr.openjdk.java.net/~ykantser/8077423/webrev.00/ > > A couple of comments

Re: RFR(S): JDK-8074146 jdb has succeded to read an unreadable file

2015-04-16 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 16 apr 2015, at 17:59, Dmitry Samersoff > wrote: > > Everybody, > > Please review: > > http://cr.openjdk.java.net/~dsamersoff/JDK-8074146/webrev.01/ > > With current testing infrastructure it's hard to make file unreadable > on all platforms. > > From the

Re: RFR: 8078144 many nightly tests failed due to NoSuchMethodError: sun.management.ManagementFactoryHelper.getDiagnosticMXBean

2015-04-21 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 21 apr 2015, at 09:01, Shanliang Jiang wrote: > > Hi, > > Please review this test fix: > > webrev: http://cr.openjdk.java.net/~sjiang/JDK-8078144/00/ > bug: https://bugs.openjdk.java.net/browse/JDK-8078144 > > The method sun.management.ManagementFactoryHelpe

Re: RFR(S): 8076524: Remove jhat tests and help library from JDK

2015-04-22 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 22 apr 2015, at 12:56, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8076524 > webrev: http://cr.openjdk.java.net/~ykantser/8076524/webrev.00 > > This fix is a part of

Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests

2015-04-22 Thread Staffan Larsen
Looks good! Thanks, /Staffan > On 22 apr 2015, at 11:17, Yekaterina Kantserova > wrote: > > Hi, > > Could I please have a review of this fix. > > bug: https://bugs.openjdk.java.net/browse/JDK-8059047 > webrev: http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/ > > This fix is a part of

Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests

2015-04-22 Thread Staffan Larsen
tser/8059047.jdk/webrev.00 - > BasicJMapTest.java will use HprofParser to verify hprof dumps created by the > test. > > // Katja > > > > On 04/22/2015 01:20 PM, Staffan Larsen wrote: >> Looks good! >> >> Thanks, >> /Staffan >> >>> On 22 apr 2015,

Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests

2015-04-22 Thread Staffan Larsen
I also found this test that uses jhat and needs an update: com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.sh /Staffan > On 22 apr 2015, at 15:05, Staffan Larsen wrote: > > I think you are missing a "@build jdk.test.lib.hprof..*”. > > /Staffan > >> On 22 a

  1   2   3   4   5   6   7   8   9   10   >