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

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 On 10/1/19 4:00 PM, David Holmes wrote: Hi Serguei, Shouldn't this:  80   for (int i = 0; i < threadsCount; i++) {   81 LOG("  threa

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

On 10/3/19 3:33 PM, David Holmes wrote: On 4/10/2019 3:15 am, [email protected] wrote: On 10/3/19 03:13, David Holmes wrote: Hi Dan, On 3/10/2019 3:20 am, Daniel D. Daugherty wrote: Sorry for the delay in reviewing this one... I've been playing whack-a-mole with Robbin's MoCrazy

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

Ping... Thanks, Serguei On 10/7/19 5:25 PM, [email protected] wrote: On 10/7/19 17:14, [email protected] 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

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

Hi Chris, On 10/8/19 12:56 PM, Chris Plummer wrote: Hi Serguei, The just seems like an odd coding pattern to me:   // Block until the suspender thread competes the tested threads suspension   agent_lock(jni);   agent_unlock(jni); I do not consider it as odd but normal. We have similar codi

Re: RFR: JDK-8224159: JDWP IPv6 scope support

Hi Alex, I have a couple of minor suggestions according to our recent conversation. http://cr.openjdk.java.net/~amenkov/jdk14/jdwp_scopes/webrev/src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c.frames.html 272 * Parses scope id. 273 * Scope id is ulong on Windows, uint32 on unix,

Re: RFR: JDK-8224159: JDWP IPv6 scope support

Hi Alex, Thank you for the update! It looks good. The final modifier is still missed for the IsWindows field. No need for new webrev if you fix it. You are right, camel case naming style is used all over the file. So, I'm Okay with keeping it. Replacement 'hostLen' => 'hostnameLen' is a good on

Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

Hi David, The update looks good. Thanks, Serguei On 10/29/19 9:28 PM, David Holmes wrote: Hi Doug, Your proposed patch wasn't quite right. I made some adjustments but I'm still having issues with the test, HotSpotMethodSubstitutionTest.testThreadSubstitutions, as I don't see how to make th

Re: RFR: 8233549: Thread interrupted state must only be accessed when not in a safepoint-safe state

Hi David, Thank you for the update! It looks good to me. You are right about my first suggestion. The lines need to stay where they are, or additional curly brackets are needed to force the ThreadBlockInVM destructor earlier. Thanks, Serguei On 11/14/19 2:21 PM, David Holmes wrote: Hi Sergue

Re: RFR (S) 8173361: various crashes in JvmtiExport::post_compiled_method_load

Hi Coleen, I have some questions. Both the compiler method load and unload are posted as deferred events. Both events keep the nmethod alive until the ServiceThread processes the event. The implementation is: JvmtiDeferredEvent JvmtiDeferredEvent::compiled_method_load_event(nmethod* nm) {

Re: RFR (S) 8173361: various crashes in JvmtiExport::post_compiled_method_load

Some additional details on this... The side effect of a nmethodLocker::lock_nmethod(nm) call is the nmethod::is_locked_by_vm() returns true. It is checked in the nmethod::flush(), nmethod::can_convert_to_zombie() and some other places. If it is still not safe to look at metadata when the nmethod i

Re: RFR (S) 8173361: various crashes in JvmtiExport::post_compiled_method_load

Hi Coleen, On 11/15/19 2:12 PM, [email protected] wrote: Hi, I've been working on answers to these questions, so I'll start with this one. The nmethodLocker keeps the nmethod from being reclaimed (made_zombie or memory released) by the sweeper, but the nmethod could be unloaded.

Re: RFR(S): 8169467: GetLocalInstance returns JVMTI_ERROR_TYPE_MISMATCH (rather than JVMTI_ERROR_INVALID_SLOT) on static method

Thank you, Alex! Serguei On 11/20/19 3:32 PM, Alex Menkov wrote: Looks good. --alex On 11/19/2019 15:36, [email protected] wrote: Please, review a fix for:    https://bugs.openjdk.java.net/browse/JDK-8169467 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8169467-jvmti-loc

Re: 8233197(S): Invert JvmtiExport::post_vm_initialized() and Jfr:on_vm_start() start-up order for correct option parsing

Serguei, but these capabilities are not yet needed. Here is the updated webrev: http://cr.openjdk.java.net/~mgronlun/8233197/webrev02/ Thanks again Markus *From:*Serguei Spitsyn *Sent:* den 20 november 2019 04:10 *To:* Markus Gronlund ; hotspot-jfr-dev ; [email protected]

Re: RFR (S) 8173361: various crashes in JvmtiExport::post_compiled_method_load

Hi Coleen, Looks good in general. Nice approach, thank you for working on this! It is great to get rid of the nmethodLocker's in JvmtiDeferredEvent class. I have some questions/comments. http://cr.openjdk.java.net/~coleenp/2019/8173361.03/webrev/src/hotspot/share/runtime/serviceThread.cpp.fram

Re: RFR(S): 8221372: Test vmTestbase/nsk/jvmti/GetThreadState/thrstat001/TestDescription.java times out

Hi Alex, Thank you for review and the comment! I saw this bug forgot to fix. Will fix now. Thanks, Serguei On 11/25/19 2:12 PM, Alex Menkov wrote: +1 The only nit: 87 jvmtiError err = jvmti->SetEventNotificationMode(mode, event_type, event_thread);   88 if (err != JVMTI_ERROR_NONE)

Re: RFR(S): 8221372: Test vmTestbase/nsk/jvmti/GetThreadState/thrstat001/TestDescription.java times out

Hi Chris, Thank you for looking at it! May I count you as a reviewer? My plan is to submit another mach5 100-times run before the push. Thanks, Serguei On 11/25/19 12:41 PM, Chris Plummer wrote: Hi Serguei, It looks like before your fix, runs were normally just a few seconds, but there occas

Re: RFR(S): 8221372: Test vmTestbase/nsk/jvmti/GetThreadState/thrstat001/TestDescription.java times out

Thanks, Chris! Serguei On 11/25/19 4:47 PM, Chris Plummer wrote: Yes On 11/25/19 3:47 PM, [email protected] wrote: Hi Chris, Thank you for looking at it! May I count you as a reviewer? My plan is to submit another mach5 100-times run before the push. Thanks, Serguei On 11/25/19 12

RFR (XXS): 8235280: UnProblemList vmTestbase/nsk/jvmti/GetThreadState/thrstat001/TestDescription.java

Please, review a trivial fix for sub-task:   https://bugs.openjdk.java.net/browse/JDK-8235280 The fix is to remove the test from the ProblemList.txt: diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot

Re: RFR (XXS): 8235280: UnProblemList vmTestbase/nsk/jvmti/GetThreadState/thrstat001/TestDescription.java

Thanks, Igor! Serguei On 12/3/19 11:32 AM, Igor Ignatyev wrote: LGTM -- Igor On Dec 3, 2019, at 11:29 AM, [email protected] wrote: Please, review a trivial fix for sub-task: https://bugs.openjdk.java.net/browse/JDK-8235280 The fix is to remove the test from the ProblemList.txt:

Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures

Hi Chris, It looks good. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results

Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures

Hi Chris, It looks good. I'm in favor to always run tests in verbose mode. It is not a good idea in general to optimize on it. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.jav

Re: RFR 8235360: Update JDWP, JDI and Instrumentation specs for Record attribute

Hi David, Agreed. I was thinking about the same. Thanks, Serguei On 12/5/19 2:52 PM, David Holmes wrote: Looks good Harold! If we get any more of these unmodifiable attributes we may have to look at a way to refer to them more abstractly and only define them in one place. Thanks, David O

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

Hi Chris and Alex, (I've also included Dan, David and Dean to the mailing list) We have to reach a consensus about this. We have 3 options: Option #1:   The JIT optimization to delete a code which "looks useless"   has to be disabled if can_pop_frame capability is enabled.   Than this problem

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

Hi David, Thank you for writing this down. Totally agree with you here. On 12/5/19 6:45 PM, David Holmes wrote: Hi Serguei, On 6/12/2019 11:31 am, [email protected] wrote: Hi Chris and Alex, (I've also included Dan, David and Dean to the mailing list) We have to reach a consensus

Re: RFR(T) 8235922: [TESTBUG]TestRecordAttrGenericSig.java and TestRecordAttr.java are failing

Hi Harold, +1 Thanks, Serguei On 12/13/19 11:45 AM, Daniel D. Daugherty wrote: On 12/13/19 2:35 PM, Harold Seigel wrote: Hi, Please review this trivial fix to prevent java/lang/instrument/... TestRecordAttr.java and TestRecordAttrGenericSig.java from failing.  The fix replaces hard-wired J

Re: RFR [XS]: 8234968: check calloc rv in libinstrument InvocationAdapter

Hi Matthias, +1 Thanks, Serguei On 12/12/19 2:00 AM, Langer, Christoph wrote: Hi Matthias, I think your current patch is good as it is – at least it wouldn’t make things worse, AFAICS. Further improvements can probably be done under another issue. Cheers Christoph *From:* serviceabilit

Re: RFR: 8234624: jstack mixed mode should refer DWARF

Hi Yasumasa, This is nice move in general. Thank you for working on this! http://cr.openjdk.java.net/~ysuenaga/JDK-8234624/webrev.01/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/LinuxCDebugger.java.frames.html 96 long libptr = dbg.findLibPtrByAddress(pc); 97 if (libptr ==

Re: RFR(s): 8235912: JvmtiBreakpoint remove oops_do and metadata_do

Hi Coleen, Is this webrev v2 right to look at? :   http://cr.openjdk.java.net/~coleenp/2019/8235829.02/webrev/ It looks good to me. Just one nit (sorry, if it is a duplicated comment): http://cr.openjdk.java.net/~coleenp/2019/8235829.02/webrev/src/hotspot/share/prims/jvmtiImpl.cpp.frames.html

RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

Please, review a trivial fix for bug:   https://bugs.openjdk.java.net/browse/JDK-8236124 Patch suggested by A. Shipilev: diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp --- a/src/hotspot/share/code/nmethod.cpp +++ b/src/hotspot/share/code/nmethod.cpp @@ -16

Re: RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

Hi Chris, Good catch. I agree, for consistency the enqueue_event() is better to follow the JvmtiDeferredEventQueue::enqueue() to avoid slowdebug minimal build failures. New patch is: diff --git a/src/hotspot/share/prims/jvmtiThreadState.hpp b/src/hotspot/share/prims/jvmtiThreadState.hpp --

Re: RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

Hi Coleen, Thank you for reviewing it! I've sent another trivial patch as suggested by Chris. It is for better consistency. Thanks, Seguei On 1/6/20 6:29 PM, [email protected] wrote: This looks trivial.  Thank you for fixing it. Coleen On 1/6/20 9:18 PM, [email protected]

Re: RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

Thanks, Coleen! Serguei On 1/6/20 8:26 PM, [email protected] wrote: On 1/6/20 11:09 PM, [email protected] wrote: Hi Coleen, Thank you for reviewing it! I've sent another trivial patch as suggested by Chris. It is for better consistency. Yes, Chris's suggestion is better.

Re: RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

Thank you for review, David! Serguei On 1/6/20 8:54 PM, David Holmes wrote: On 7/01/2020 2:09 pm, [email protected] wrote: Hi Coleen, Thank you for reviewing it! I've sent another trivial patch as suggested by Chris. It is for better consistency. Yes much better. It makes little sen

Re: RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

Hi Chris, The slowdebug minimal build does not fail without NOT_JVMTI_RETURN in the ServiceThread::enqueue_deferred_event(). I'm curious why and will check if it is really needed. If so, will add it as well. Thanks, Serguei On 1/6/20 8:05 PM, [email protected] wrote: Hi Chris, Good

Re: RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

Chris, The macro NOT_JVMTI_RETURN is never used outside of the prims/ folder. Also, there is more work to get rid of the JVMTI code in the ServiceThread. I don't know how important is this for the minimal build. So, I'd leave it alone for now and just fix the build issue. Thanks, Serguei On 1/7

Re: RFR(XXS): 8236124: Minimal VM slowdebug build failed after JDK-8212160

On 1/7/20 11:45 AM, [email protected] wrote: On 1/7/20 2:25 PM, [email protected] wrote: Chris, The macro NOT_JVMTI_RETURN is never used outside of the prims/ folder. Also, there is more work to get rid of the JVMTI code in the ServiceThread. I don't know how important

Re: JDK 14 RFR(XXS): 8229847: AttachProvider javadoc page needs an update

Hi David and Alex, The updated webrev is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/AttachProvider-doc.1/ Thanks, Serguei On 1/8/20 3:50 PM, [email protected] wrote: Hi David, On 1/8/20 14:54, David Holmes wrote: Hi Serguei, On 9/01/2020 7:42 am, [email protected]

Re: JDK 14 RFR(XXS): 8229847: AttachProvider javadoc page needs an update

Thanks, Alex! Serguei On 1/9/20 3:51 PM, Alex Menkov wrote: LGTM --alex On 01/09/2020 13:54, [email protected] wrote: Hi David and Alex, The updated webrev is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/AttachProvider-doc.1/ Thanks, Serguei On 1/8/20 3:50 PM, serguei.spit.

Re: JDK 14 RFR(XXS): 8229847: AttachProvider javadoc page needs an update

Thanks, David! Serguei On 1/9/20 5:22 PM, David Holmes wrote: +1 Thanks, David On 10/01/2020 9:51 am, Alex Menkov wrote: LGTM --alex On 01/09/2020 13:54, [email protected] wrote: Hi David and Alex, The updated webrev is: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/AttachPro

Re: RFR(M): 8236913: debug agent's jdwp command logging should include the command set name and command name

Hi Chris, Okay, thanks! Serguei On 1/14/20 10:39 AM, Chris Plummer wrote: Hi Serguei, I didn't want to return a Command because then the cmdSetNum and cmdNum would need to be checked by the caller before calling debugDispatch_getHandler()or have special handling for NULL being returned. T

Re: RFR: JDK-8235846: Improve WindbgDebuggerLocal implementation

Hi Alex, Thank you for the update! It looks good. Still incorrect indent: 103 ~AutoJavaString() { 104 if (m_buf) { 105 m_env->ReleaseStringUTFChars(m_str, m_buf); 106 } 107 } 108 109 operator const char* () const { 110 return m_buf; 111 } ... 133 void setReleaseMode(jint mode) { 134 release

Re: RFR [XS]: 8238602: remove obsolete functions from libinstrument/FileSystemSupport_md.c

Hi Matthias, It looks good. Thanks, Serguei On 2/6/20 8:06 AM, Baesken, Matthias wrote: Hello,   the link time section gc (see https://bugs.openjdk.java.net/browse/JDK-8236714 , on linux s390x it prints the removed sections) showed some obsolete / unused functions in FileSystemSupport_md.

Re: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump

Hi Ralf, I'd suggest for the option format something like this:   -gz[=level] where level is an int. The part [=level] is optional. The level is 0 by default (if it is not set). Thanks, Serguei On 2/11/20 7:35 AM, Schmelter, Ralf wrote: Hi Yasumasa, I think I've tried too much by using the -

Re: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump

Ralf, I see this feature adds a lot of code. In fact, I'm not sure, it is worth to add this kind of complexity (including new compressing threads) into the VM implementation. What is a real use case behind it? Could this compressing be done separately from VM implementation? Thanks, Serguei

Re: RFR: 8238196: tests that use SA Attach should not be allowed to run against signed binaries on Mac OS X 10.14.5 and later

Hi Chris, This looks okay to me. Thanks, Serguei On 2/11/20 1:49 PM, Chris Plummer wrote: Hi Igor, Here's an updated webrev: http://cr.openjdk.java.net/~cjplummer/8238196/webrev.01/index.html I rebased to JDK 15 and made all the changes you suggested except for (3). I did not think it is

Re: RFR: JDK-8238710: LingeredApp doesn't log stdout/stderr if exits with non-zero code

Hi Alex, LGTM++ Thanks, Serguei On 2/12/20 3:45 PM, Chris Plummer wrote: Ok. LGTM. Chris On 2/12/20 1:58 PM, Alex Menkov wrote: Hi Chris, thanks for the review. finishApp is also called from startApp(String... cmd) method and appProcess can be not initialized there. In the case finishApp w

Re: RFR 8232622: Technical debt in BadAttributeValueExpException

Hi Roger, It looks good to me. Thanks, Serguei On 2/13/20 7:52 AM, Roger Riggs wrote: Please review a minor cleanup to remove code long since unnecessary. The type of the BadAttributeValueExpException argument is String and if it is not a string in the serialized stream, a suitable replacemen

Re: RFR (S) 8239055: Wrong implementation of VMState.hasListener

Hi Fairoz, Looks good. Thanks, Serguei On 2/13/20 9:13 PM, Fairoz Matte wrote: Hi, Please review a tiny change to correct the VMState.hasListener implementation. JBS: https://bugs.openjdk.java.net/browse/JDK-8239055 Webrev: http://cr.openjdk.java.net/~fmatte/8239055/webrev.00/ Thanks,

Re: [15] RFR 8238633: JVMTI heap walk should consult GC for marking oops

Hi Zhengyu, It looks okay to me. The testing you do looks enough for verification. But I'm not sure about performance testing though. Thanks, Serguei On 2/17/20 6:51 AM, Zhengyu Gu wrote: Hi Stefan, Thanks for the review and suggestions, updated accordingly: http://cr.openjdk.java.net/~zgu/

Re: RFR: add parallel heap inspection support for jmap histo(G1)(Internet mail)

Hi Lin, Could you, please, re-post your RFR with the right enhancement number in the message subject? It will be more trackable this way. Thanks, Serguei On 2/17/20 10:29 PM, linzang(臧琳) wrote: Dear David,       Thanks a lot!       I have updated the refined code to  http://cr.openjdk.jav

Re: RFR(L) 8237354: Add option to jcmd to write a gzipped heap dump

I also like this proposal. Thanks, Serguei On 2/19/20 5:45 PM, Ioi Lam wrote: I like this proposal. It's simple, easily extensible and should work on all platforms. Thanks - Ioi On 2/19/20 3:59 PM, Yasumasa Suenaga wrote: Hi, Generally I agree with Ioi, but I think it is not a problem onl

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

Hi Daniil, Thank you for reply. I agree with the approach to avoid using system properties. Then it is better to be consistent. I'd consider adding an RMI registry port option as well. Will look at your comments in the CSR and reply there. Thanks, Serguei On 2/25/20 10:05 AM, Daniil Titov wrot

Re: RFR: 8196751: Add jhsdb option to specify debug server RMI connector port

Hi Daniil, Okay, thanks! Serguei On 2/25/20 11:38 AM, Daniil Titov wrote: Hi Serguei, I will update the CSR and the fix to include this change. Thank you, Daniil On 2/25/20, 11:07 AM, "[email protected]" wrote: Hi Daniil, Thank you for reply. I agree with

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

Hi Alex, Several minor suggestions. 77 new Thread(aRP, "jj1").start(); 78 new Thread(asRP, "jj2").start(); What mean aRP and asRP? In fact, it is confusing. Can they be renamed to something like obj1 and obj2. 79 // new Thread(aRP, "jj3").start(); 80 // new Thread(asRP, "jj4").start();  Thes

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

Hi Alex, It looks good to me. Could you, please, also remove the line? : 156 // No need in new webrev. Thanks, Serguei On 10/4/18 4:11 PM, Alex Menkov wrote: Hi Serguei, Updated webrev: http://cr.openjdk.java.net/~amenkov/sh2java/DeferredStep_final/webrev.02/ Fixed all issues

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

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: 8208686: [AOT] JVMTI ResourceExhausted event repeated for same allocation

+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

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, [email protected] wrote: In general, like

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

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-8211324: Link to java.lang.ThreadGroup in JDWP spec is broken

I'll push it. Thanks, Serguei On 10/9/18 4:39 AM, Gary Adams wrote: Patch attached. I'll need a sponsor for the push. On 10/8/18, 3:58 PM, [email protected] wrote: Hi Gary, +1 Thanks, Serguei On 10/8/18 11:30, Chris Hegarty wrote: The updated link looks ok to me. -Chris On 8

Re: RFR 8193879: Java debugger hangs on method invocation

Hi Severin, On 10/9/18 8:50 AM, Severin Gehwolf wrote: Hi Seguei, On Mon, 2018-10-08 at 17:57 -0700, [email protected] wrote: Hi Severin, You already fixed a couple of deadlocks in the debugger method invocation and have an expertise in this area. Do you have any time to review the w

Re: [8u-backport] RFR: JDK-8193879: Java debugger hangs on method invocation

Hi Fairoz, It looks good to me. Thanks, Serguei On 10/11/18 6:22 AM, Fairoz Matte wrote: Hi, Kindly review the backport of "JDK-8193879: Java debugger hangs on method invocation" to 8u Code is almost cleanly applied, test case has been modified to fit into the JDK8 test framework. Webrev

Re: crash in JvmtiExport::post_compiled_method_load

Hi Nezih, I've added a comment to the bug report with your details. Thanks! Serguei On 10/12/18 3:28 PM, nezih yigitbasi wrote: Hi, We have observed the same issue again on Java 10.0+46. I just wanted to share the new error file and the jvm args in case these provide additional information.

Re: The failure

Hi David and Robbin, On 10/23/18 7:38 AM, Robbin Ehn wrote: Hi, On 10/23/18 10:34 AM, David Holmes wrote: Hi Serguei, The VMThread is executing VM_HandshakeAllThreads which is not a safepoint operation. There's no real way to tell from the stacks what it's stuck on. Good point. We agreed

Re: The failure

Please, skip it - sorry for the noise. It is hard to prove anything with current dump. Thanks, Serguei On 10/23/18 9:09 AM, [email protected] wrote: Hi David and Robbin, I have an idea that needs to be checked. It can be almost the same deadlock scenario that I've already explained b

Re: The failure

Okay, thanks! Serguei On 10/23/18 4:58 PM, David Holmes wrote: I should have looked further before sending this. Many threads are in smr_delete. David On 24/10/2018 9:56 AM, David Holmes wrote: Hi Serguei, Robbin, One thing I noticed which Robbin should be able to expand upon is that Threa

Re: RFR 8195627: [Graal] nsk/jdi/VirtualMachine/redefineClasses/redefineclasses026 hangs with Graal in Xcomp mode

Hi Daniil, It looks good. Thanks! Serguei On 10/29/18 11:13 AM, Daniil Titov wrote: Please review the change that fixes the issue when the test hangs with Graal and XComp mode on. One of the test cases the test runs is to call com.sun.jdi.VirtualMachine.redefineClasses() while the target V

Re: RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

On 11/6/18 11:14 AM, Chris Plummer wrote: Hi JC, The exception changes looked ok to me, but it would be good to get a second approval before moving forward with them since they are pretty significant. The exception changes need to be discussed after a separate RFR is posted. Thanks, Sergu

Re: RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

Okay. I'm not sure I fully understandd what is current plan. My view is that we can do the following steps:  1) Jc can push what was already reviewed.     With this change we will miss extra tracing for JNI calls and results.  2) Work on using the ExceptionCheckingJni that will restore     this t

Re: RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

Hi Jc, On 11/6/18 1:10 PM, JC Beyler wrote: Hi Serguei, Thanks for looking at it. You are right that there are various ways of doing this: A) Continue removing the assignments via https://bugs.openjdk.java.net/browse/JDK-8210687     - This requires a few more webrevs but as you've said, we

Re: RFR (M) 8201655: Add thread-enabled support for the Heap Sampling

Hi Jc, Not sure, I understand a motivation for this change: - if (JvmtiExport::should_post_sampled_object_alloc()) { + { Also, I'm not sure this is still needed: +#include "prims/jvmtiEventController.inline.hpp" +#include "prims/jvmtiThreadState.inline.hpp" I expected you'd just revert all th

Re: RFR (M) 8080406: VM_GetOrSetLocal doesn't check local slot type against requested type

Hi Jc, Thank you a lot for the code review! On 11/6/18 9:22 AM, JC Beyler wrote: Hi Serguei, I saw this code: +    BasicType next_slot_type = locals->at(_index + 1)->type(); I think we are not worried about going out of bounds due to the work done in the check_slot_type, which is done in doi

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

Thanks a lot, Chris! Serguei On 11/13/18 10:55 AM, Chris Plummer wrote: Hi Serguei, Changes look good. Chris On 11/12/18 8:06 PM, [email protected] wrote: On 11/12/18 20:05, [email protected] wrote: Hi Jc, Thank you a lot for reviewing! On 11/12/18 09:35, JC Beyler wrot

Re: RFR 8203174: [Graal] JDI tests fail with Unexpected exception: com.sun.jdi.ObjectCollectedException

Hi Daniil, I do not see the latest webrev in this email anymore. Is it still the v3 version? If so then the fix looks good to me. I like the comments you added there as they help. I was also concerned about the compiler issue. Thank you for pointing that it was resolved with the JDK-8195627. T

Re: RFR 8203174: [Graal] JDI tests fail with Unexpected exception: com.sun.jdi.ObjectCollectedException

Then consider it reviewed. Thanks! Serguei On 11/13/18 12:50 PM, Daniil Titov wrote: Hi Serguei, Thank you for reviewing this change. You are right, the latest webrev is still v3. Webrev: http://cr.openjdk.java.net/~dtitov/8203174/webrev.03/ Bug: https://bugs.openjdk.java.net/browse/JDK-8203

Re: RFR(M): 8201375: Add the AllowArchivingWithJavaAgent diagnostic vm option to allow the use of the -javaagent option during CDS dumping

Hi Calvin, It looks good in general. New comment does not help much: 1362 // Java agents are allowed during run time. Therefore, the following condition is not 1363 // checked: !_allow_archiving_with_java_agent && AllowArchivingWithJavaAgent 1364 if (_allow_archiving_with_java_agent && !Allo

Re: RFR(M): 8201375: Add the AllowArchivingWithJavaAgent diagnostic vm option to allow the use of the -javaagent option during CDS dumping

Hi Calvin, It looks good to me. No need in new webrev if you update this comment as below. Thanks, Serguei On 11/16/18 2:00 PM, Calvin Cheung wrote: On 11/16/18, 1:04 PM, [email protected] wrote: Hi Calvin, It looks good in general. New comment does not help much: 1362   // Java

Re: RFR(XXS) 8213275 ReplaceCriticalClasses.java fails with jdk.internal.vm.PostVMInitHook not found

Hi Ioi, It looks good and trivial. Thanks, Serguei On 11/27/18 8:42 AM, Ioi Lam wrote: https://bugs.openjdk.java.net/browse/JDK-8213275 http://cr.openjdk.java.net/~iklam/jdk12/8213275-ReplaceCriticalClasses-missing-PostVMInitHook.v01/ Please review this simple fix. The jdk.internal.vm.Post

Re: RFR JDK-8195639: [Graal] nsk/jvmti/PopFrame/popframe009 fail with Graal in -Xcomp

Hi Dean, You also asked the questions: > Doesn't is_interp_only_mode() only apply to the current thread? Yes it does. > I don't think it's safe to assume no compiles will happen in other threads, > or that a method call target is not already compiled, because as far > as I can tell, JVMTI o

Re: RFR JDK-8195639: [Graal] nsk/jvmti/PopFrame/popframe009 fail with Graal in -Xcomp

Good summary with a list of solutions below! On 11/27/18 2:50 PM, [email protected] wrote: Let me list the proposed solutions: 1. short-circuit compiles in CompilationPolicy::event in is_interp_only_mode 2. detect is_interp_only_mode in resolve stubs and force a c2i call 3. don't offer a J

Re: RFR JDK-8195639: [Graal] nsk/jvmti/PopFrame/popframe009 fail with Graal in -Xcomp

On 11/27/18 3:38 PM, [email protected] wrote: Good summary with a list of solutions below! On 11/27/18 2:50 PM, [email protected] wrote: Let me list the proposed solutions: 1. short-circuit compiles in CompilationPolicy::event in is_interp_only_mode 2. detect is_interp_only_mo

Re: RFR (S) 8214408: Migrate EventsOnOff to using the same allocateAndCheck method

Hi Jc, LGTM Thanks, Serguei On 11/27/18 1:45 PM, JC Beyler wrote: Hi all, Could I get a review for this webrev, it is an attempt to fix JDK-8214192, which I cannot reproduce locally: Webrev: http://cr.openjdk.java.net/~jcbeyler/8214408/webrev.00/

Re: RFR (L) 8214417: Add space after/before {} in vmTestbase/nsk/jvmti/[A-I] tests

+1 Thanks, Serguei On 11/29/18 4:44 PM, David Holmes wrote: +1 This is burning a lot of cycles. :) David On 30/11/2018 9:57 am, Alex Menkov wrote: Looks ok --alex On 11/29/2018 15:00, JC Beyler wrote: Hi Alex, Yes that is true, I had initially only done the one-liner {} because I was u

Re: RFR 8214572: nsk/jvmti/unit/ForceEarlyReturn/earlyretbase should not suspend the thread when the top frame executes JVMCI code

Hi Daniil, It looks good in general. Thank you for the update! I have some minor comment though. http://cr.openjdk.java.net/%7Edtitov/8214572/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.h.udiff.html +/** +* This method suspends the thread while ensuring the top frame ex

Re: RFR 8214572: nsk/jvmti/unit/ForceEarlyReturn/earlyretbase should not suspend the thread when the top frame executes JVMCI code

On 12/4/18 4:54 PM, David Holmes wrote: Hi everyone, I'd actually argue that the comment not refer just to JVMCI but more generally: + // when the top frame belongs to the test rather than to incidental Java code (classloading, JVMCI, etc) Reasonable. Also note typo: then -> t

Re: RFR (T) 8215034: Remove old HOTSWAP conditionals

Hi Coleen, +1 Thanks, Serguei On 12/7/18 2:06 PM, Jiangli Zhou wrote: Looks good and trivial. Thanks, Jiangli On 12/7/18 1:57 PM, [email protected] wrote: I was in the area and found these few remaining conditionals. Tested with tier1 on Oracle platforms. open webrev at http:

Re: RFR (M) 8201655: Add thread-enabled support for the Heap Sampling

Hi Jc, Alex will take a look at the test update. Thanks, Serguei On 11/12/18 9:53 AM, JC Beyler wrote: Hi Serguei, Thanks for the update and thanks for testing mach5. Serguei sent me that the testing passed mach5 testing, could I get another review to be able to push it? Thanks! Jc On Tu

Re: RFR (M) 8201655: Add thread-enabled support for the Heap Sampling

Hi Alex, Nice catch! It is about the following fragment: 726 for (i = 0; i < thread_stats.number_threads; i++) { 727 if (strcmp(expected_name, thread_stats.threads[i])) { 728 return FALSE; 729 } else { 730 found_thread = TRUE; 731 } 732 } 733 return found_thread; 734 } Also, I'

Re: RFR JDK-8215425: vmTestbase/nsk/jvmti/PopFrame should provide more detailed output

Hi Alex, A couple of minor comments. http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/PopFrame/popframe002.java.frames.html http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/PopFrame/popfra

Re: RFR JDK-8215425: vmTestbase/nsk/jvmti/PopFrame should provide more detailed output

Alex, Thank you for the update! It looks good. There is another instance with incorrect spacing: 121 totRes=doPopFrame(false, Thread.currentThread()); No need in new webrev if you fix the above. Thanks, Serguei On 12/18/18 6:01 PM, Alex Menkov wrote: Hi Serguei, Thank you for the review.

Re: RFR JDK-8215716: PopFrame() was unexpectedly done

Hi Alex, LG++ Thanks, Serguei On 12/20/18 5:09 PM, David Holmes wrote: Looks good. Thanks, David On 21/12/2018 11:04 am, Alex Menkov wrote: Hi all, please review a small fix for https://bugs.openjdk.java.net/browse/JDK-8215716 webrev: http://cr.openjdk.java.net/~amenkov/popframe004/webrev.

Re: RFR 8215398: -Xlog option usage => Invalid decorator '\temp\app_cds.log'.

Hi Harold, It looks good to me. Thanks, Serguei On 12/20/18 1:30 PM, Harold David Seigel wrote: Hi David, Thanks for looking at this! Please review this updated webrev.  The fix is the same but the webrev contains a new test instead of modifying an existing test:    http://cr.openjdk.java

Re: RFR: JDK-8211343: nsk_jvmti_parseoptions should handle multiple suboptions

On 12/21/18 12:55 PM, [email protected] wrote: On 12/21/18 3:33 PM, [email protected] wrote: Hi Gary, Looks good in general. Thank you for taking care about this! A couple of comments. It seems, with your fixes the macro NSK_JVMTI_OPTION_VAL_SEP is not needed anymore. Also, I

Re: RFR JDK-8216386: vmTestbase/nsk/jvmti/PopFrame/popframe005/TestDescription.java fails

Hi Alex, It looks good to me. Thanks, Serguei On 1/17/19 12:21 PM, Alex Menkov wrote: Hi Gary, On 01/17/2019 10:53, Gary Adams wrote: I like the fact that test.timeout.factor is applied as a multiplier. It's not clear why an upper limit had to be added. As you noted there 3 cases where T

Re: RFR: JDK-8158066: SourceDebugExtensionTest fails to rename file

Hi Gary, It looks Okay in general. But I have a small concern/question about the line: 65 if (!inOutClassFile.renameTo(tmpInOutClassFile)) { What if the tmpInOutClassFile does exist in the system? Is it possible there can be some leftover from previous runs? Thanks, Serguei On 1/17/19 3:

Re: RFR(S) 8215113: Sampling interval not always correct

Hi Jc, It looks good to me too. Thanks, Serguei On 1/11/19 5:07 PM, Hohensee, Paul wrote: Nits: memAllocator.cpp: Copyright line should read * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved. The comment // Tell tlab to forget the bytes_since if we passed i

Re: RFR(S) 8215113: Sampling interval not always correct

On 1/22/19 12:54 PM, [email protected] wrote: Hi Jc, It looks good to me too. Forgot to say that I hope it was tested well. Thanks, Serguei Thanks, Serguei On 1/11/19 5:07 PM, Hohensee, Paul wrote: Nits: memAllocator.cpp: Copyright line should read * Copyright (c) 2018, 2

Re: RFR 8163127: Debugger classExclusionFilter does not work correctly with method references

Hi Daniil, It'd be nice if Chris has a chance to look at this fix. He has most recent experience in this area. Thanks, Serguei On 1/17/19 6:08 PM, Daniil Titov wrote: Please review the change that fixes JDB stepping issue for a specific case when the single step request was initiated earlier

Re: RFR 8163127: Debugger classExclusionFilter does not work correctly with method references

Hi Daniil, Must be good enough. Thanks! Serguei On 1/24/19 1:38 PM, Daniil Titov wrote: Hi Serguei, The testing is not fully completed yet. I ran locally jdk_jdi, vmTestbase_nsk_jdi, vmTestbase_nsk_jdb and have the same tests plus serviceability still running in Mach5. I am also starting t

Re: RFR: 8217618: JVM TI SuspendThread doesn't suspend the current thread before returning

Hi David, The fix looks good to me. Thank you for taking care about it! Thanks, Serguei On 1/24/19 6:46 PM, David Holmes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8217618 webrev: http://cr.openjdk.java.net/~dholmes/8217618/webrev/ Lots of analysis in the bug report. Bottom line: S

  1   2   3   4   5   6   7   8   9   >