Re: RFR (S) 8210842: Handle JNIGlobalRefLocker.cpp

Hi Jc, On 18/09/2018 1:59 PM, JC Beyler wrote: Hi all, I've slowly started working on JDK-8191519 . However before starting to really refactor all the tests, I thought I'd get a few opinions. I am working on internalizing the error handling o

Re: RFR (M) 8210700: Clean up JNI_ENV_ARG and factorize the macros for vmTestbase/jvmti/unit tests

I'm fine with the code the way it is. Thanks, David On 18/09/2018 4:08 AM, Alex Menkov wrote: I raised the point because I remember I saw similar issue. Finally I found the issue it and it was about JNIEnv. So there is no problem here (as tests creates only a single jvmtiEnv). Anyway I think it

Re: RFR JDK-8210725: com/sun/jdi/RedefineClearBreakpoint.java fails with waitForPrompt timed out after 60 seconds

Hi Gary, updated webrev: http://cr.openjdk.java.net/~amenkov/sh2java/timeout/webrev.02/ see comments inline. On 09/17/2018 11:57, Gary Adams wrote: Should sleepTime also be adjusted? No. sleepTime is delay before we read jdb output (i.e. we don't read jdb output on every update, but check f

Re: RFR JDK-8210725: com/sun/jdi/RedefineClearBreakpoint.java fails with waitForPrompt timed out after 60 seconds

Should sleepTime also be adjusted? Should sleepTime and timeout be scoped to just waitForPrompt? On 9/17/18, 2:26 PM, Gary Adams wrote: Is the log decoration typical? 98 jdb.log("==="); Is the Utils.adjustTimeout() applied consistently? e.g. is

Re: RFR JDK-8210725: com/sun/jdi/RedefineClearBreakpoint.java fails with waitForPrompt timed out after 60 seconds

Is the log decoration typical? 98 jdb.log("==="); Is the Utils.adjustTimeout() applied consistently? e.g. is the timeout passed to waitFor() already adjusted? If you are promoting log() to be publicly visible, then it should be used for the other

Re: RFR (M) 8210700: Clean up JNI_ENV_ARG and factorize the macros for vmTestbase/jvmti/unit tests

I raised the point because I remember I saw similar issue. Finally I found the issue it and it was about JNIEnv. So there is no problem here (as tests creates only a single jvmtiEnv). Anyway I think it would be better to use jvmtiEnv passed to callbacks (then it remains correct even is other jvmt

RFR JDK-8210725: com/sun/jdi/RedefineClearBreakpoint.java fails with waitForPrompt timed out after 60 seconds

Hi all, please review small fix: http://cr.openjdk.java.net/~amenkov/sh2java/timeout/webrev.01/ It fixes https://bugs.openjdk.java.net/browse/JDK-8210725 - accordingly the logs of the failing tests, they work as expected, but sometimes (busy environment?) there is no reply from jdb for 60 secon

RE: RFR (S) 8210775: JVM TI Spec missing copyright

Hi, Mandy, David, and Serguei. Thank you for taking the time to Review. I've pushed the changeset. iris -Original Message- From: Serguei Spitsyn Sent: Monday, September 17, 2018 12:57 AM To: David Holmes ; Iris Clark ; serviceability-dev Subject: Re: RFR (S) 8210775: JVM TI Spec miss

Re: RFR (M) 8210700: Clean up JNI_ENV_ARG and factorize the macros for vmTestbase/jvmti/unit tests

Hi David, I think it is fine to leave the caching in the most tests I looked because they want to do JVMTI calls where there is jvmtiEnv* passed in. Would you rather I revert the rawmonitor changes to where it is still using the cached one instead of the one passed in by the call? Let me know, Jc

Re: RFR JDK-8210760: [TEST] rewrite com/sun/jdi shell tests to java version - step4

Hi Alex, It looks good modulo the Chris's comments. Thanks, Serguei On 9/14/18 12:59 PM, Alex Menkov wrote: Hi all, please review fix for https://bugs.openjdk.java.net/browse/JDK-8210760 webrev: http://cr.openjdk.java.net/~amenkov/sh2java/step4/webrev.01/ --alex

Re: RFR (S) 8210775: JVM TI Spec missing copyright

+1 Thanks, Serguei On 9/16/18 9:27 PM, David Holmes wrote: This looks fine to me Iris. Thanks, David On 15/09/2018 8:01 AM, Iris Clark wrote: Hi. Please review the following changes to add a copyright line to the end of the generated jvmti.html file: 8210775:  JVM TI Spec missing cop

Re: RFR (L) 8210481: Remove #ifdef cplusplus from vmTestbase

Hi Jc, It looks good to me. Thank you for taking care about it! Thanks, Serguei On 9/14/18 9:50 AM, JC Beyler wrote: Hi all, Could I get a review for the following webrev: Webrev: http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/

Re: RFR (XS) 8210726: Fix up a few minor nits forgotten by JDK-8210665

Hi Jc, +1 Thanks, Serguei On 9/14/18 10:54 AM, Chris Plummer wrote: One minor issue. There's an extra space in the following line:  157 fields[i].fid = env-> GetStaticFieldID(cls, fields[i].name, fields[i].sig); Chris On 9/14/18 9:49 AM, JC Beyler wrote: Hi all, When doing