Re: RFR 8211736: jdb doesn't print prompt when breakpoint is hit and suspend policy is STOP_EVENT_THREAD

2018-10-11 Thread Chris Plummer
Hi Daniil, Can you send output from an example jdb session? Do you think maybe we should also call printCurrentLocation() when the suspend policy is NONE? There's a typo in the following line. The space is on the wrong side of the comma.

RE: [8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation

2018-10-11 Thread Fairoz Matte
Thanks Serguei for the review... > -Original Message- > From: Serguei Spitsyn > Sent: Thursday, October 11, 2018 10:41 PM > To: Fairoz Matte ; serviceability- > d...@openjdk.java.net > Subject: Re: [8u-backport] RFR: JDK-8193879: Java debugger hangs on > method invocation > > Hi Fairoz, >

RE: [8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation

2018-10-11 Thread Fairoz Matte
Hi JC, Thanks for looking into it. I have moved two methods (getTestSourcePath and parseBreakpoints) into Utils (existing helper class) class as part of testlibrary framework. Now it makes sense to have both the methods as public. Updated webrev - http://cr.openjdk.java.net/~fmatte/8193879/webr

Re: RFR 8211736: jdb doesn't print prompt when breakpoint is hit and suspend policy is STOP_EVENT_THREAD

2018-10-11 Thread Daniil Titov
Thank you,  JC! Please review an updated version of the patch that fixes newly added test for Windows platform  (now it uses system dependent line separator string). Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.02/ Issue: https://bugs.openjdk.java.net/browse/JDK-8211736 B

RFR 8211736: jdb doesn't print prompt when breakpoint is hit and suspend policy is STOP_EVENT_THREAD

2018-10-11 Thread Daniil Titov
Please review the change that fixes the issue with missing prompt in jdb when a breakpoint is hit and the suspend policy is set to stop the thread only. Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.01 Issue: https://bugs.openjdk.java.net/browse/JDK-8211736 Thanks! --Daniil

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread Hohensee, Paul
Looks good to me. Paul From: JC Beyler Date: Thursday, October 11, 2018 at 6:38 PM To: David Holmes Cc: "Hohensee, Paul" , "serviceability-dev@openjdk.java.net" Subject: Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods Hi all, @David: I did your two requests for

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread David Holmes
Thanks those updates look fine. David On 12/10/2018 8:37 AM, JC Beyler wrote: Hi all, @David: I did your two requests for volatile and removing the lock The latest webrev is now: Webrev: http://cr.openjdk.java.net/~jcbeyler/8211980/webrev.02

Re: RFR: JDK-8212028: Use run-test makefile framework for testing in Oracle's Mach5

2018-10-11 Thread David Holmes
Hi Erik, On 12/10/2018 8:29 AM, Erik Joelsson wrote: Hello, (adding serviceability-dev and hotspot-dev for test changes) Bug: https://bugs.openjdk.java.net/browse/JDK-8212028 Webrev: http://cr.openjdk.java.net/~erikj/8212028/webrev.01/index.html (From ihse-runtestprebuilt-branch in jdk-sandb

RFR: JDK-8212028: Use run-test makefile framework for testing in Oracle's Mach5

2018-10-11 Thread Erik Joelsson
Hello, (adding serviceability-dev and hotspot-dev for test changes) Bug: https://bugs.openjdk.java.net/browse/JDK-8212028 Webrev: http://cr.openjdk.java.net/~erikj/8212028/webrev.01/index.html (From ihse-runtestprebuilt-branch in jdk-sandbox) In order to fully adopt the new run-test framewor

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread David Holmes
On 12/10/2018 8:20 AM, JC Beyler wrote: I'm 100% in agreement with David :) Inlined are my comments: On Thu, Oct 11, 2018 at 3:13 PM David Holmes > wrote: On 12/10/2018 3:44 AM, Hohensee, Paul wrote: > So, given that the lock was only used to protect l

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread JC Beyler
I'm 100% in agreement with David :) Inlined are my comments: On Thu, Oct 11, 2018 at 3:13 PM David Holmes wrote: > On 12/10/2018 3:44 AM, Hohensee, Paul wrote: > > So, given that the lock was only used to protect log_table > initialization, and that the patch moves that into the C++ "class > in

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread David Holmes
On 12/10/2018 3:44 AM, Hohensee, Paul wrote: So, given that the lock was only used to protect log_table initialization, and that the patch moves that into the C++ "class initializer" which is run when the first Thread object is constructed, we don't need any locking/memory ordering anymore, rig

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread Hohensee, Paul
I don’t think you’re missing anything, see my response to David. I don’t think you need _log_table_initialized, but I’ll defer to David. Paul From: JC Beyler Date: Wednesday, October 10, 2018 at 4:20 PM To: "Hohensee, Paul" Cc: "serviceability-dev@openjdk.java.net" Subject: Re: RFR (S) 821198

Re: RFR (L) 8211899: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[E-M]

2018-10-11 Thread Alex Menkov
got it. LGTM. --alex On 10/10/2018 19:03, JC Beyler wrote: Hi Alex, Thanks for the review! Yes I had seen that too before sending this review out and forked that fix into this: https://bugs.openjdk.java.net/browse/JDK-8211905 Which now is merged and I've updated my webrev to reflect this o

Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-11 Thread Alex Menkov
Hi Jc, updated webrev: http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.02/ -> Why not make it javadoc like the other methods of the same file (so @return instead of r

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread Hohensee, Paul
So, given that the lock was only used to protect log_table initialization, and that the patch moves that into the C++ "class initializer" which is run when the first Thread object is constructed, we don't need any locking/memory ordering anymore, right? Paul On 10/11/18, 4:11 AM, "David Holme

Re: [8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation

2018-10-11 Thread serguei . spitsyn
Hi Fairoz, It looks good to me. Thanks, Serguei On 10/11/18 6:22 AM, Fairoz Matte wrote: Hi, Kindly review the backport of "JDK-8193879: Java debugger hangs on method invocation" to 8u Code is almost cleanly applied, test case has been modified to fit into the JDK8 test framework. Webrev

Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-11 Thread Alex Menkov
Hi Serguei, I run the test itself and jdk/com/sun/jdi multiple times. --alex On 10/10/2018 17:17, serguei.spit...@oracle.com wrote: Hi Alex, It looks good to me. How did you test it? Thanks, Serguei On 10/10/18 16:25, Alex Menkov wrote: Hi all, please review a fix for https://bugs.openjd

[8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation

2018-10-11 Thread Fairoz Matte
Hi, Kindly review the backport of "JDK-8193879: Java debugger hangs on method invocation" to 8u Code is almost cleanly applied, test case has been modified to fit into the JDK8 test framework. Webrev - http://cr.openjdk.java.net/~fmatte/8193879/webrev.00/ JBS bug - https://bugs.openjdk.java.n

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-11 Thread David Holmes
On 11/10/2018 3:10 AM, Hohensee, Paul wrote: There used to be a rule against using “namespace”. Don’t know if that’s still true or not: I believe it is. Hence the “static const ”. You only need OrderAccess if there could be a race on accesses to whatever you’re guarding. Looks like the old cod