Re: 8218401: WRONG_PHASE: vmTestbase/nsk/jvmti test crash

2019-03-18 Thread Daniil Titov
Hi Serguei and JC, Please review a new version of the fix that locks a monitor across the callbacks, as Serguei suggested. Webrev: http://cr.openjdk.java.net/~dtitov/8218401/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8218401 Thanks! --Daniil On 3/18/19, 9:47 AM, "serguei.spit

Re: 8218401: WRONG_PHASE: vmTestbase/nsk/jvmti test crash

2019-03-18 Thread Daniil Titov
Hi JC, You are right, the synchronization is required across callbacks,  otherwise JVMTI phase could change in the middle of a callback. I will send a new version of the patch soon. Thanks! --Daniil From: Jean Christophe Beyler Date: Friday, March 15, 2019 at 4:45 PM To: Daniil Tito

Re: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread serguei.spit...@oracle.com
Hi Matthias, +1 Thanks, Serguei On 3/18/19 09:47, Alan Bateman wrote: On 18/03/2019 14:34, Baesken, Matthias wrote: Updated webrev :   http://cr.openjdk.java.net/~mb

Re: RFR: 8218401: WRONG_PHASE: vmTestbase/nsk/jvmti test crash

2019-03-18 Thread serguei.spit...@oracle.com
Hi Daniil, The JVMTI phase can change in the middle of callback work after the check you added. I'd suggest to lock a raw monitor across the callbacks to make them atomic. Thank you for taking care about this issue! Thanks, Serguei On 3/15/19 16:08, Daniil Titov wrote: Please review the c

Re: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread Alan Bateman
On 18/03/2019 14:34, Baesken, Matthias wrote: Updated webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.2/ Alan + JC , may I add you as reviewers ? Yes, looks good to me. -Alan

Re: RFR (S) 8220512: Deoptimize redefinition functions that have dirty ICs

2019-03-18 Thread serguei.spit...@oracle.com
Hi Coleen, Renaming looks good. Thanks, Serguei On 3/15/19 07:58, coleen.phillim...@oracle.com wrote: From some offline feedback, I changed the name of has_evol_ics => has_evol_metadata and a couple of other small things.  I reran this though builds and tier1 tests. Incremental: http://

Re: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread Jean Christophe Beyler
Hi Matthias, You can add me (LGTM btw), I'm not an official reviewer (as per http://openjdk.java.net/census ) but Alan is :) Jc On Mon, Mar 18, 2019 at 7:34 AM Baesken, Matthias wrote: > Updated webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/

RE: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread Baesken, Matthias
Updated webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.2/ Alan + JC , may I add you as reviewers ? Thanks, Matthias From: Baesken, Matthias Sent: Montag, 18. März 2019 11:01 To: 'Alan Bateman' ; Jean Christophe Beyler Cc: serviceability-dev@openjdk.java.net Subject: RE: RFR:

Re: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread Roger Riggs
Hi, InvocationAdapter.c: 581, remove the space after "(".  As long as that line is being changed. $.02, Roger On 03/18/2019 06:00 AM, Baesken, Matthias wrote: Hi Alan, thanks for the review . >I think this looks okay except I think "emergency" should be dropped from the message (and the

Re: [PING] RFR: 8220579: [Containers] SubSystem.java out of sync with osContainer_linux.cpp

2019-03-18 Thread Andrew Dinn
On 18/03/2019 10:06, Severin Gehwolf wrote: > Could I get a review from an OpenJDK Reviewer for this, please? Bob is > already OK with it. Yes, this is fine. Reviewed. regards, Andrew Dinn ---

Re: [PING] RFR: 8220579: [Containers] SubSystem.java out of sync with osContainer_linux.cpp

2019-03-18 Thread Severin Gehwolf
Hi, Could I get a review from an OpenJDK Reviewer for this, please? Bob is already OK with it. Thanks, Severin On Thu, 2019-03-14 at 13:58 -0400, Bob Vandette wrote: > The change looks good. Thanks for fixing this. > > I’d send this to core-libs (cc’d). > > Bob. > > > > On Mar 14, 2019, at

Re: RFR(XS): 8220707: [TESTBUG] serviceability/sa/TestHeapDumpForLargeArray.java fails with jtreg -vmoption:-Xmx < 8g

2019-03-18 Thread Nick Gasson
Hi Sharath, On 15/03/2019 23:32, Sharath Ballal wrote: Fix looks good. How have you tested it ? I ran the test using the "make test" target with and without JTREG="MAX_MEM=512m" on AArch64 and x86. Previously it would fail with that argument. Thanks, Nick

RE: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread Baesken, Matthias
Hi Alan, thanks for the review . > I think this looks okay except I think "emergency" should be dropped from the > message (and the existing comment). Are you referring to this “emergency” : * First make our emergency fallback InternalError throwable. */ result = initialize

Re: RFR(XS): 8220707: [TESTBUG] serviceability/sa/TestHeapDumpForLargeArray.java fails with jtreg -vmoption:-Xmx < 8g

2019-03-18 Thread Nick Gasson
Hi Jean Christophe, On 15/03/2019 22:47, Jean Christophe Beyler wrote: Not a reviewer but looks good to me :) I would perhaps put a comment above the argument creation to note that the order is important, ie something like: "// Need to add the default arguments first to have explicit -Xmx8g

Re: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread Alan Bateman
On 18/03/2019 08:54, Baesken, Matthias wrote: Hi JC, thanks for your comments . Second webrev , may I have a second review please ? http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.1/ I think this looks okay except I think "emergency" should be dropped from the message (and the existi

RE: RFR: 8220355: Improve assertion texts and exception messages in eventHandlerVMInit

2019-03-18 Thread Baesken, Matthias
Hi JC, thanks for your comments . Second webrev , may I have a second review please ? http://cr.openjdk.java.net/~mbaesken/webrevs/8220355.1/ One question that came to my mind : do we still have to worry about the old-style C variable declaration order in eventHandlerVMInit after changing