Re: RFR (L) 8211782: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/[I-S]*

2018-10-05 Thread serguei . spitsyn
Hi Jc, It looks good to me. Thanks, Serguei On 10/5/18 3:00 PM, JC Beyler wrote: Hi all, I continued the NSK_CPP_STUB removal with this webrev: Webrev: http://cr.openjdk.java.net/~jcbeyler/8211782/webrev.00/ Bug: https://bugs.open

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

2018-10-05 Thread serguei . spitsyn
Hi Alex, It looks good to me. Thank you for the update. Thanks, Serguei On 10/5/18 4:53 PM, Alex Menkov wrote: ok, this is updated webrev: http://cr.openjdk.java.net/~amenkov/sh2java/DeferredStep_final/webrev.03/ --alex On 10/05/2018 12:37, serguei.spit...@oracle.com wrote: In general, like

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

2018-10-05 Thread Alex Menkov
ok, this is updated webrev: http://cr.openjdk.java.net/~amenkov/sh2java/DeferredStep_final/webrev.03/ --alex On 10/05/2018 12:37, serguei.spit...@oracle.com wrote: In general, like the suggestion from Jc with the correction for lastLine to be a local. But leave it up to Alex to decide what is b

Re: RFR (M) 8211261: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/[A-G]*

2018-10-05 Thread Chris Plummer
Thanks! On 10/5/18 4:02 PM, JC Beyler wrote: Just for completeness:  @Chris, I created https://bugs.openjdk.java.net/browse/JDK-8211802 for the first case you were talking about, which is slightly

Re: RFR: 8208686: [AOT] JVMTI ResourceExhausted event repeated for same allocation

2018-10-05 Thread serguei . spitsyn
+1 Thanks, Serguei On 10/5/18 9:05 AM, Vladimir Kozlov wrote: Looks good. Thanks, Vladimir On 10/5/18 8:22 AM, Doug Simon wrote: Hi, Testing found a bug in the original webrev. Namely, when clearing out a pending exception and returning null in the JVMCI ..._or_null stubs, the JavaThread:

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

2018-10-05 Thread serguei . spitsyn
In general, like the suggestion from Jc with the correction for lastLine to be a local. But leave it up to Alex to decide what is better as changes would require another round of testing. Thanks, Serguei On 10/5/18 12:10 PM, JC Beyler wrote: You're right for the single threaded part; I misread

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

2018-10-05 Thread JC Beyler
You're right for the single threaded part; I misread that part and thought it would be multi-threaded as well. And fair enough for the keeping it then as a do..while(false); it just took me a while to figure out what was being done. You could put the data.lastLine in a local variable and update it

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

2018-10-05 Thread Alex Menkov
Hi Jc, On 10/05/2018 10:34, JC Beyler wrote: Hi Alex, One question and a comment on this: - I thought HashMap was not thread safe so I think you need to synchronize the access to the map threadData The map is accessed from a single thread (main test thread which sends jdb commands and hand

Re: RFR: 8208686: [AOT] JVMTI ResourceExhausted event repeated for same allocation

2018-10-05 Thread Doug Simon
Thanks for the review. -Doug > On 5 Oct 2018, at 19:27, Tom Rodriguez wrote: > > Looks good. > > tom > > Doug Simon wrote on 10/5/18 8:22 AM: >> Hi, >> Testing found a bug in the original webrev. Namely, when clearing out a >> pending exception and returning null in the JVMCI ..._or_null stu

Re: RFR: 8208686: [AOT] JVMTI ResourceExhausted event repeated for same allocation

2018-10-05 Thread Doug Simon
Thanks for re-reviewing. -Doug > On 5 Oct 2018, at 18:05, Vladimir Kozlov wrote: > > Looks good. > > Thanks, > Vladimir > > On 10/5/18 8:22 AM, Doug Simon wrote: >> Hi, >> Testing found a bug in the original webrev. Namely, when clearing out a >> pending exception and returning null in the J

Re: RFR JDK-8211292: [TEST] convert com/sun/jdi/DeferredStepTest.sh test

2018-10-05 Thread JC Beyler
Hi Alex, One question and a comment on this: - I thought HashMap was not thread safe so I think you need to synchronize the access to the map threadData - I think your test code could be simplified if you moved it into a helper method (not tested but just for example): +private void next() {

Re: RFR: 8208686: [AOT] JVMTI ResourceExhausted event repeated for same allocation

2018-10-05 Thread Tom Rodriguez
Looks good. tom Doug Simon wrote on 10/5/18 8:22 AM: Hi, Testing found a bug in the original webrev. Namely, when clearing out a pending exception and returning null in the JVMCI ..._or_null stubs, the JavaThread::_vm_result field was not being set to NULL. I've addressed this in v2 of the

Re: RFR: 8208686: [AOT] JVMTI ResourceExhausted event repeated for same allocation

2018-10-05 Thread Vladimir Kozlov
Looks good. Thanks, Vladimir On 10/5/18 8:22 AM, Doug Simon wrote: Hi, Testing found a bug in the original webrev. Namely, when clearing out a pending exception and returning null in the JVMCI ..._or_null stubs, the JavaThread::_vm_result field was not being set to NULL. I've addressed this

Re: RFR 8193879: Java debugger hangs on method invocation

2018-10-05 Thread gary.ad...@oracle.com
Looks OK to me. On 10/4/18 5:19 PM, Daniil Titov wrote: Hi Gary and Chris, Please review an updated version of the patch that has newly added test for the case when suspend policy is set to SUSPEND_EVENT_THREAD reimplemented using JDI API. Thus, the changes in src/jdk.jdi/share/classes/com/s

Re: RFR: 8208686: [AOT] JVMTI ResourceExhausted event repeated for same allocation

2018-10-05 Thread Doug Simon
Hi, Testing found a bug in the original webrev. Namely, when clearing out a pending exception and returning null in the JVMCI ..._or_null stubs, the JavaThread::_vm_result field was not being set to NULL. I've addressed this in v2 of the webrev: Relative diff for bug fix: