Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

2019-10-07 Thread Per Liden
Hi David, On 10/8/19 3:58 AM, David Holmes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8231737 webrev: http://cr.openjdk.java.net/~dholmes/8231737/webrev/ Stylistic code cleanup of JvmtiRawMonitor code as previously promised: Very nice clean up! But when changing the names, may I sug

RFR(S) 8231986 [SA] Consolidate parts of the Linux and MacOSX versions of ps_core.c

2019-10-07 Thread Ioi Lam
https://bugs.openjdk.java.net/browse/JDK-8231986 http://cr.openjdk.java.net/~iklam/jdk14/8231986-consolidate-ps-core.v01/ One of my upcoming CDS changes (JDK-8231610) would affect the duplicated code in these 2 files. So instead of fixing the same thing twice, I have moved the duplicated parts

Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

2019-10-07 Thread David Holmes
On 8/10/2019 2:57 pm, serguei.spit...@oracle.com wrote: Hi David, Thank you for making the changes! Initially, I did not notice there are several spots with a wrong indent: Well spotted! I thought I had done a global re-indent - oops. Fixed now. Thanks, David 124 if (Atomic::replace_if_nul

Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

2019-10-07 Thread serguei.spit...@oracle.com
Hi David, Thank you for making the changes! Initially, I did not notice there are several spots with a wrong indent: 124 if (Atomic::replace_if_null(self, &_owner)) { 125return; 126 } ... 136 if (_owner == NULL && Atom

Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

2019-10-07 Thread David Holmes
Hi Serguei, On 8/10/2019 2:18 pm, serguei.spit...@oracle.com wrote: Hi David, Looks good in general. Thanks for looking at it. A couple of places still need a cleanup. http://cr.openjdk.java.net/~dholmes/8231737/webrev/src/hotspot/share/prims/jvmtiRawMonitor.hpp.frames.html QNode pointer

Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

2019-10-07 Thread serguei.spit...@oracle.com
Hi David, Looks good in general. A couple of places still need a cleanup. http://cr.openjdk.java.net/~dholmes/8231737/webrev/src/hotspot/share/prims/jvmtiRawMonitor.hpp.frames.html QNode pointer is not unified: 47 QNode * v

RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

2019-10-07 Thread David Holmes
Bug: https://bugs.openjdk.java.net/browse/JDK-8231737 webrev: http://cr.openjdk.java.net/~dholmes/8231737/webrev/ Stylistic code cleanup of JvmtiRawMonitor code as previously promised: - Self -> self - SimpleX -> simpleX - Contended -> contended - Node -> node - TState -> tState (variable name) -

Re: RFR (M): 8231595 [TEST] develop a test case for SuspendThreadList including current thread

2019-10-07 Thread serguei.spit...@oracle.com
On 10/7/19 17:14, serguei.spit...@oracle.com wrote: Hi Alex, Chris and David, The mach5 testing in 100 runs loop on all platform discovered a race in new test. It is between the native suspendTestedThreads() called on the suspender thr

Re: RFR (M): 8231595 [TEST] develop a test case for SuspendThreadList including current thread

2019-10-07 Thread serguei.spit...@oracle.com
Hi Alex, Chris and David, The mach5 testing in 100 runs loop on all platform discovered a race in new test. It is between the native suspendTestedThreads() called on the suspender thread and the checkSuspendedStatus() calling native checkTestedThreadsSuspende

Re: RFR: JDK-8199136: Dead code in src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java

2019-10-07 Thread serguei.spit...@oracle.com
Hi Evgeny, Looks good. Thanks, Serguei On 9/24/19 01:30, Evgeny Mandrikov wrote: On Tue, Sep 24, 2019 at 8:15 AM David Holmes

Re: RFR: JDK-8199136: Dead code in src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java

2019-10-07 Thread Daniil Titov
Hi Evgeny, I will sponsor this change. Thank you, Daniil From: serviceability-dev on behalf of Daniil Titov Date: Tuesday, September 24, 2019 at 9:49 PM To: Evgeny Mandrikov , David Holmes Cc: Subject: Re: JDK-8199136: Dead code in src/jdk.jcmd/share/classes/sun/tools/common/

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread serguei.spit...@oracle.com
On 10/7/19 04:23, David Holmes wrote: Hi Serguei, On 7/10/2019 6:42 pm, serguei.spit...@oracle.com wrote: Hi David, Suppose, T1 thread is blocked on a raw monitor hold by thread T3 in JVMTI MonitorContendedEntered callback on an object monitor hold by thread T2. In theory, if only raw monit

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread serguei.spit...@oracle.com
Hi David, Got it, thanks! Serguei On 10/7/19 04:33, David Holmes wrote: Hi Serguei, One follow up ... On 7/10/2019 5:45 pm, serguei.spit...@oracle.com wrote: Hi David, Thank you for replies! On 10/3/19 18:59, David Holmes wrote: Hi Serguei, Just coming back to your original review emai

Re: [RFR] 8196969: JTreg Failure: serviceability/sa/ClhsdbJstack.java causes NPE

2019-10-07 Thread Severin Gehwolf
On Mon, 2019-10-07 at 09:11 -0700, Tom Rodriguez wrote: > Looks good to me. Thanks for the review! Cheers, Severin > tom > > Severin Gehwolf wrote on 10/7/19 2:43 AM: > > Hi Tom, > > > > On Thu, 2019-10-03 at 11:14 -0700, Tom Rodriguez wrote: > > > I think your fix is a reasonable way to resol

Re: RFR: 8231666: ThreadIdTable::grow() invokes invalid thread transition

2019-10-07 Thread Daniil Titov
Hi Robbin, Yes, I ran my benchmark [1]. Please see below the output showing that the table was grown. ../jdk/build/linux-x64-release/images/jdk/bin/java -cp . -Xlog:thread+table=info ThreadStartupTest Starting the test [0.185s][info][thread,table] Grown to size:512 The test finished. Execution

Re: [RFR] 8196969: JTreg Failure: serviceability/sa/ClhsdbJstack.java causes NPE

2019-10-07 Thread Tom Rodriguez
Looks good to me. tom Severin Gehwolf wrote on 10/7/19 2:43 AM: Hi Tom, On Thu, 2019-10-03 at 11:14 -0700, Tom Rodriguez wrote: I think your fix is a reasonable way to resolve the NPE. We certainly shouldn't try to create a ScopeDesc from an invalid location. Maybe getPCDescNearDbg should t

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread David Holmes
Hi Serguei, One follow up ... On 7/10/2019 5:45 pm, serguei.spit...@oracle.com wrote: Hi David, Thank you for replies! On 10/3/19 18:59, David Holmes wrote: Hi Serguei, Just coming back to your original review emails to ensure everything covered. On 2/10/2019 6:57 pm, serguei.spit...@or

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread David Holmes
Hi Serguei, On 7/10/2019 6:42 pm, serguei.spit...@oracle.com wrote: Hi David, Suppose, T1 thread is blocked on a raw monitor hold by thread T3 in JVMTI MonitorContendedEntered callback on an object monitor hold by thread T2. In theory, if only raw monitor is used in further deadlock detection

Re: RFR (XS/T) 8231931: [TESTBUG] serviceability/sa/TestUniverse.java looks for wrong string with Shenandoah

2019-10-07 Thread Roman Kennke
Looks good to me. Thanks, Roman > Trivial test bug: > https://bugs.openjdk.java.net/browse/JDK-8231931 > > Fix: > > diff -r f9cd97ec8549 test/hotspot/jtreg/serviceability/sa/TestUniverse.java > --- a/test/hotspot/jtreg/serviceability/sa/TestUniverse.javaMon Oct 07 > 11:45:11 2019 +0200

RFR (XS/T) 8231931: [TESTBUG] serviceability/sa/TestUniverse.java looks for wrong string with Shenandoah

2019-10-07 Thread Aleksey Shipilev
Trivial test bug: https://bugs.openjdk.java.net/browse/JDK-8231931 Fix: diff -r f9cd97ec8549 test/hotspot/jtreg/serviceability/sa/TestUniverse.java --- a/test/hotspot/jtreg/serviceability/sa/TestUniverse.javaMon Oct 07 11:45:11 2019 +0200 +++ b/test/hotspot/jtreg/serviceability/sa/TestUniv

Re: [RFR] 8196969: JTreg Failure: serviceability/sa/ClhsdbJstack.java causes NPE

2019-10-07 Thread Severin Gehwolf
Hi Tom, On Thu, 2019-10-03 at 11:14 -0700, Tom Rodriguez wrote: > I think your fix is a reasonable way to resolve the NPE. We certainly > shouldn't try to create a ScopeDesc from an invalid location. Maybe > getPCDescNearDbg should try to do a better job finding a PcDesc with a > valid scope_

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread serguei.spit...@oracle.com
Hi David, Suppose, T1 thread is blocked on a raw monitor hold by thread T3 in JVMTI MonitorContendedEntered callback on an object monitor hold by thread T2. In theory, if only raw monitor is used in further deadlock detection analysis then a possible dependency loop with T1 an T2 involved won'

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread serguei.spit...@oracle.com
On 10/7/19 00:58, serguei.spit...@oracle.com wrote: Hi Dan, On 10/3/19 17:01, Daniel D. Daugherty wrote: On 10/3/19 7:35 PM, serguei.spit...@oracle.com wrote: On 10/3/19 3:33 PM, David Holmes wrote: On 4/10/2019 3:15 am, serguei.spit...@oracle.com wrote: On 10/3/19 03:13, David Holmes wro

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread serguei.spit...@oracle.com
Hi Dan, On 10/3/19 17:01, Daniel D. Daugherty wrote: On 10/3/19 7:35 PM, serguei.spit...@oracle.com wrote: On 10/3/19 3:33 PM, David Holmes wrote: On 4/10/2019 3:15 am, serguei.spit...@oracle.com wrote: On 10/3/19 03:13, David Holmes wrote: Hi Dan, On 3/10/2019 3:20 am, Daniel D. Daugher

Re: RFR: 8231289: Disentangle JvmtiRawMonitor from ObjectMonitor and clean it up

2019-10-07 Thread serguei.spit...@oracle.com
Hi David, Thank you for replies! On 10/3/19 18:59, David Holmes wrote: Hi Serguei, Just coming back to your original review emails to ensure everything covered. On 2/10/2019 6:57 pm, serguei.spit...@oracle.com wrote: I forgot to say, the fix looks pretty good to me. Also, it is quite educ

Re: RFR: 8231666: ThreadIdTable::grow() invokes invalid thread transition

2019-10-07 Thread Robbin Ehn
Hi Daniil, Yes, good, but: >> Testing:  Mach5 tier1, tier2, and tier3 tests successfully passed. And if you have not done so, you should test this with the benchmark you have as a stress test and see that this does what we think. Can you please test it with your benchmark, if