Re: [8u] RFR: 8195088: [TEST_BUG] StartManagementAgent got unexpected exception

Hi Severin, It looks good and applies cleanly. So, I'm not sure you really need a review for this. Thanks, Serguei On 10/1/19 06:01, Severin Gehwolf wrote: Hi, Please review this OpenJDK 8u vs. Oracle JDK 8 parity patch. I wasn't sure whether I need review for this one as the bug in question

Re: RFR(S): 8231288: "jhsdb jmap" test needed to reproduce issues that used to be reproduced by JShellHeapDumpTest

thanks! On 10/2/19 4:32 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks good. Thanks, Serguei On 10/2/19 08:31, Chris Plummer wrote: Ping! I updated the webrev to actually exclude any 8196969 reference in ProblemList.txt. https://bugs.openjdk.java.net/browse/JDK-8231288 http://

Re: RFR(S): 8231288: "jhsdb jmap" test needed to reproduce issues that used to be reproduced by JShellHeapDumpTest

Hi Chris, It looks good. Thanks, Serguei On 10/2/19 08:31, Chris Plummer wrote: Ping! I updated the webrev to actually exclude any 8196969 reference in ProblemList.txt. https://bugs.openjdk.java.net/browse/JDK-8231288 http://cr.openjdk.java.net/~cjplummer/8231288/webrev.01/index.html tha

Re: RFR: 8170299: Debugger does not stop inside the low memory notifications code

Hi Daniil, +1 I also prefer (agree with) a new VM option to opt-out from the new behavior. Sorry for some latency in the review and discussion process. Thanks, Serguei On 10/1/19 20:20, David Holmes wrote: Hi Daniil, Thanks again for your perseverance with this one. This looks fine to me.

Re: RFR(XXS): 8231768: Remove duplicate if/else in vmTestbase/nsk/share/jdi/Binder.java

Hi Leonid, It looks good. Thank you for taking care about it! Thanks, Serguei On 10/2/19 11:11, Leonid Mesnik wrote: Hi Could you please review following tiny and trivial fix which just remove duplicated if/else branch in nsk/share/jdi/Binder class. Verified that all tests still pass loca

Re: RFA: CSR: 8231593 : Add a command line option to control notification mechanism.

Hi Daniil, I reviewed the CSR 3 days ago. Only one review is needed for the CSR process. We don't need to do an email RFA for a CSR request. Cheers, David On 3/10/2019 3:08 am, Daniil Titov wrote: Please review/approve a CSR request. The proposed CSR [1] is for adding a new VM option UseN

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

Hi Daniil, On 3/10/2019 2:21 am, Daniil Titov wrote: Hi David and Robbin, Could we consider making the ServiceThread responsible for the ThreadIdTable resizing in the similar way how it works for StringTable and ResolvedMethodTable, rather than having ThreadIdTable::add() method calling Th

Re: RFR(XXS): 8231768: Remove duplicate if/else in vmTestbase/nsk/share/jdi/Binder.java

Thank you, I updated copyright. Leonid > On Oct 2, 2019, at 12:48 PM, Chris Plummer wrote: > > Looks good. Please update the copyright. > > thanks, > > Chris > > On 10/2/19 11:11 AM, Leonid Mesnik wrote: >> Hi >> >> Could you please review following tiny and trivial fix which just remove >

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

On 10/2/19 8:53 AM, Severin Gehwolf wrote: On Wed, 2019-10-02 at 08:35 -0700, Chris Plummer wrote: On 10/2/19 1:57 AM, Severin Gehwolf wrote: On Wed, 2019-10-02 at 10:39 +0200, Severin Gehwolf wrote: On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote: Hi Severin, Sorry, this is not an ar

Re: RFR(XXS): 8231768: Remove duplicate if/else in vmTestbase/nsk/share/jdi/Binder.java

Looks good. Please update the copyright. thanks, Chris On 10/2/19 11:11 AM, Leonid Mesnik wrote: Hi Could you please review following tiny and trivial fix which just remove duplicated if/else branch in nsk/share/jdi/Binder class. Verified that all tests still pass locally. webrev: http://

Re: [UNVERIFIED SENDER] RFR(XXS): 8231768: Remove duplicate if/else in vmTestbase/nsk/share/jdi/Binder.java

Thank you for review. Leonid > On Oct 2, 2019, at 11:14 AM, Hohensee, Paul wrote: > > Looks good. > > Paul > > From: serviceability-dev > on behalf of Leonid > Mesnik mailto:leonid.mes...@oracle.com>> > Date: Wednesday, October 2, 2019 a

RFR(XXS): 8231768: Remove duplicate if/else in vmTestbase/nsk/share/jdi/Binder.java

Hi Could you please review following tiny and trivial fix which just remove duplicated if/else branch in nsk/share/jdi/Binder class. Verified that all tests still pass locally. webrev: http://cr.openjdk.java.net/~lmesnik/8231768/webrev.00/

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

+1 --alex On 10/01/2019 19:27, David Holmes wrote: Looks good. Thanks, David On 2/10/2019 9:57 am, serguei.spit...@oracle.com wrote: Hi David, Yes, this is another place to fix the same typo, thanks. It has to be results[i] instead of err. I'll update the webrev in place. Thanks, Serguei

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

Sorry for the delay in reviewing this one... I've been playing whack-a-mole with Robbin's MoCrazy test and my AsyncMonitorDeflation bits... On 9/24/19 1:09 AM, David Holmes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8231289 webrev: http://cr.openjdk.java.net/~dholmes/8231289/webrev/

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

Hi Daniil, On 2019-10-02 18:21, Daniil Titov wrote: Hi David and Robbin, Could we consider making the ServiceThread responsible for the ThreadIdTable resizing in the similar way how it works for StringTable and ResolvedMethodTable, rather than having ThreadIdTable::add() method calling Thr

RFA: CSR: 8231593 : Add a command line option to control notification mechanism.

Please review/approve a CSR request. The proposed CSR [1] is for adding a new VM option UseNotificationThread (default true) to opt-out from the new behavior introduced by the suggested fix [3] for the issue [2] that is on review now in the separate email thread [4]. [1] CSR: https://bugs

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

Hi David, On 2019-10-02 15:25, David Holmes wrote: Hi Robbin, On 2/10/2019 7:58 pm, Robbin Ehn wrote: Hi David, What if the table is full and must be grown? The table uses chaining, it just means load factor tip over what is considered a good backing array size. Coleen raised a good que

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

Hi David and Robbin, Could we consider making the ServiceThread responsible for the ThreadIdTable resizing in the similar way how it works for StringTable and ResolvedMethodTable, rather than having ThreadIdTable::add() method calling ThreadIdTable::grow()? As I understand It should solve t

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

On Wed, 2019-10-02 at 08:35 -0700, Chris Plummer wrote: > On 10/2/19 1:57 AM, Severin Gehwolf wrote: > > On Wed, 2019-10-02 at 10:39 +0200, Severin Gehwolf wrote: > > > On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote: > > > > Hi Severin, > > > > > > > > Sorry, this is not an area that I hav

Re: RFR(S): 8231288: "jhsdb jmap" test needed to reproduce issues that used to be reproduced by JShellHeapDumpTest

Hi Chris, On Wed, 2019-10-02 at 08:31 -0700, Chris Plummer wrote: > Ping! > > I updated the webrev to actually exclude any 8196969 reference in > ProblemList.txt. > > https://bugs.openjdk.java.net/browse/JDK-8231288 > http://cr.openjdk.java.net/~cjplummer/8231288/webrev.01/index.html Looks goo

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

On 10/2/19 1:57 AM, Severin Gehwolf wrote: On Wed, 2019-10-02 at 10:39 +0200, Severin Gehwolf wrote: On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote: Hi Severin, Sorry, this is not an area that I have any expertise in. However, I did confirm that it fixes the NPE I was seeing with JShel

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

On 10/2/19 1:39 AM, Severin Gehwolf wrote: Hi Chris, On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote: Hi Severin, Sorry, this is not an area that I have any expertise in. However, I did confirm that it fixes the NPE I was seeing with JShellHeapDumpTest.java, which brings up a question.

Re: RFR(S): 8231288: "jhsdb jmap" test needed to reproduce issues that used to be reproduced by JShellHeapDumpTest

Ping! I updated the webrev to actually exclude any 8196969 reference in ProblemList.txt. https://bugs.openjdk.java.net/browse/JDK-8231288 http://cr.openjdk.java.net/~cjplummer/8231288/webrev.01/index.html thanks, Chris On 10/1/19 9:52 AM, Chris Plummer wrote: On 10/1/19 2:59 AM, Severin Ge

Re: RFR (S) 8229900: RedefineDeleteJmethod.java fails with -Xcheck:jni

On 10/2/19 12:53 AM, David Holmes wrote: Hi Coleen, Sorry rather long-winded reply ... On 1/10/2019 11:52 pm, coleen.phillim...@oracle.com wrote: Summary: Remove RedefineClasses adjustment and test, but improve checking for method/class matching. Tested with tier1 with -Xcheck:jni locally

Re: RFR (S) 8229900: RedefineDeleteJmethod.java fails with -Xcheck:jni

On 10/2/19 12:53 AM, David Holmes wrote: Hi Coleen, Sorry rather long-winded reply ... On 1/10/2019 11:52 pm, coleen.phillim...@oracle.com wrote: Summary: Remove RedefineClasses adjustment and test, but improve checking for method/class matching. Tested with tier1 with -Xcheck:jni locally

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

Hi Robbin, On 2/10/2019 7:58 pm, Robbin Ehn wrote: Hi David, What if the table is full and must be grown? The table uses chaining, it just means load factor tip over what is considered a good backing array size. Coleen raised a good question in a separate discussion, which made me realiz

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

Hi David, What if the table is full and must be grown? The table uses chaining, it just means load factor tip over what is considered a good backing array size. That aside, I'd like to know how expensive it is to grow this table. What are we talking about here? We use global counter whic

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

On 2/10/2019 7:15 pm, Robbin Ehn wrote: Hi, since holding the Threads_lock while growing can block out safepoints for a longer period, I would suggest just skip growing when holding Threads_lock. E.g. return before creating the GrowTask. What if the table is full and must be grown? That aside

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

Hi, since holding the Threads_lock while growing can block out safepoints for a longer period, I would suggest just skip growing when holding Threads_lock. E.g. return before creating the GrowTask. /Robbin On 2019-10-02 08:46, David Holmes wrote: Hi Daniil, On 2/10/2019 4:13 pm, Daniil Titov w

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

On Wed, 2019-10-02 at 10:39 +0200, Severin Gehwolf wrote: > On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote: > > Hi Severin, > > > > Sorry, this is not an area that I have any expertise in. However, I did > > confirm that it fixes the NPE I was seeing with JShellHeapDumpTest.java, > > whi

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

I forgot to say, the fix looks pretty good to me. Also, it is quite educational. :) On 10/2/19 01:51, serguei.spit...@oracle.com wrote: Hi David, http://cr.openjdk.java.net/~dholmes/8231289/webrev/src/hotspot/share/s

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

Thank you for review, David! Serguei On 10/1/19 19:27, David Holmes wrote: Looks good. Thanks, David On 2/10/2019 9:57 am, serguei.spit...@oracle.com wrote: Hi David, Yes, this is another place to fix the same typo, thanks. It has to be results[i] instead of err. I'll update the webrev in pl

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

Hi David, http://cr.openjdk.java.net/~dholmes/8231289/webrev/src/hotspot/share/services/threadService.cpp.frames.html Minor comment: 397 waitingToLockMonitor = jt->current_pending_monitor(); 398 if (waitingToLockMonitor == NULL) { 399 // we can

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

Hi Chris, On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote: > Hi Severin, > > Sorry, this is not an area that I have any expertise in. However, I did > confirm that it fixes the NPE I was seeing with JShellHeapDumpTest.java, > which brings up a question. You said this happens with -Xcomp,