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 (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 webr

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

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: 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(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: [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) 8216352: SA: ClhsdbLauncher should throw errors on Unrecognized commands

Hi Fairoz, Looks good. Thanks, Serguei On 10/3/19 06:47, Fairoz Matte wrote: Hi, Please review a tiny change to handle unrecognized command options for ClhsdbLauncher test. This patch is already proposed by Gary Adams in comments section. JBS: https://bugs.openjdk.java.net/browse/JDK-82163

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

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 test and my AsyncMonitorDeflation bits... No problem - your contribution made the wait worthwhile :

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

Hi David, Thank you for replies! On 10/3/19 18:59, David Holmes wrote: Hi Serguei, Just coming back to your original review emails to ensure everything covered. On 2/10/2019 6:57 pm, serguei.spit...@oracle.com wrote: I forgot to say, the fix looks pretty good to me. Also, it is quite

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

Hi Dan, On 10/3/19 17:01, Daniel D. Daugherty wrote: On 10/3/19 7:35 PM, serguei.spit...@oracle.com wrote: On 10/3/19 3:33 PM, David Holmes wrote: On 4/10/2019 3:15 am, serguei.spit...@oracle.com wrote: On 10/3/19 03:13, David Holmes wrote: Hi Dan, On 3/10/2019 3:20 am, Daniel D

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

On 10/7/19 00:58, serguei.spit...@oracle.com wrote: Hi Dan, On 10/3/19 17:01, Daniel D. Daugherty wrote: On 10/3/19 7:35 PM, serguei.spit...@oracle.com wrote: On 10/3/19 3:33 PM, David Holmes wrote: On 4/10/2019 3:15 am, serguei.spit...@oracle.com wrote: On 10/3/19 03:13, David Holmes

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

Hi David, Suppose, T1 thread is blocked on a raw monitor hold by thread T3 in JVMTI MonitorContendedEntered callback on an object monitor hold by thread T2. In theory, if only raw monitor is used in further deadlock detection analysis then a possible dependency loop with T1 an T2 involved won'

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

Hi David, Got it, thanks! Serguei On 10/7/19 04:33, David Holmes wrote: Hi Serguei, One follow up ... On 7/10/2019 5:45 pm, serguei.spit...@oracle.com wrote: Hi David, Thank you for replies! On 10/3/19 18:59, David Holmes wrote: Hi Serguei, Just coming back to your original review

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

On 10/7/19 04:23, David Holmes wrote: Hi Serguei, On 7/10/2019 6:42 pm, serguei.spit...@oracle.com wrote: Hi David, Suppose, T1 thread is blocked on a raw monitor hold by thread T3 in JVMTI MonitorContendedEntered callback on an object monitor hold by thread T2. In theory, if only raw

Re: RFR: JDK-8199136: Dead code in src/jdk.jcmd/share/classes/sun/tools/common/ProcessArgumentMatcher.java

Hi Evgeny, Looks good. Thanks, Serguei On 9/24/19 01:30, Evgeny Mandrikov wrote: On Tue, Sep 24, 2019 at 8:15 AM David Holmes

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

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 on the suspender thread and the checkSuspendedStatus() calling native checkTestedThreadsSuspende

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

On 10/7/19 17:14, serguei.spit...@oracle.com 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 on the suspender

Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

Hi David, Looks good in general. A couple of places still need a cleanup. http://cr.openjdk.java.net/~dholmes/8231737/webrev/src/hotspot/share/prims/jvmtiRawMonitor.hpp.frames.html QNode pointer is not unified: 47 QNode * v

Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

)) { 308 simpleEnter(self); 309 } else { Thanks, Serguei On 10/7/19 21:42, David Holmes wrote: Hi Serguei, On 8/10/2019 2:18 pm, serguei.spit...@oracle.com wrote: Hi David,

Re: RFR (S/T): 8231737: Cleanup JvmtiRawMonitor code

Looks good. Thanks, Serguei On 10/8/19 06:28, Daniel D. Daugherty wrote: On 10/7/19 9:58 PM, David Holmes wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8231737 webrev: http://cr.openjdk.java.net/~dholmes/8231737/webrev/ Thanks for cleaning up this code!! src/hotspot/share/prims/jvmt

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

Hi David, Thank you a lot for re-review! Serguei On 10/8/19 20:29, David Holmes wrote: Hi Serguei, On 9/10/2019 4:50 am, serguei.spit...@oracle.com wrote: Ping... Sorry this one slipped through the cracks. The updates seem fine to me. Using the lock as a gate works fine in these kinds of

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

the lock you know the suspender thread is done (is that actually the case?). thanks, Chris On 10/8/19 11:50 AM, serguei.spit...@oracle.com wrote: Ping... Thanks, Serguei On 10/7/19 5:25 PM, serguei.spit...@oracle.com wrote: On 10/7/19 17:14, serguei.spit...@oracle.com wrote: Hi Alex, Ch

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

Hi Chris, Please, let me know if you have more thoughts on this or it is okay to push as it is. And thank you a lot for reviewing this! Thanks, Serguei On 10/8/19 13:20, serguei.spit...@oracle.com wrote: Hi

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

Thanks, Chris! Serguei On 10/9/19 10:14, Chris Plummer wrote: Hi Serguei, It's ok to push. thanks, Chris On 10/8/19 10:47 PM, serguei.spit...@oracle.com

Re: RFR(S) 8231986 [SA] Consolidate parts of the Linux and MacOSX versions of ps_core.c

Hi Ioi, It looks good to me - nice consolidation. Thanks, Serguei On 10/7/19 23:37, Ioi Lam wrote: https://bugs.openjdk.java.net/browse/JDK-8231986 http://cr.openjdk.java.net/~iklam/jdk14/8231986-consolidate-ps-core.v01/ One of my upcoming CDS changes (JDK-8231610) would affect the duplicat

Re: RFR: 8227231: JDWP help information shows use of obsolete Xdebug flag

Hi Daniil, Looks good. Thanks, Serguei On 10/9/19 15:45, Daniil Titov wrote: Please review a small fix [1] that removes references to obsolete XDebug flag from JDWP help returned by "java -Xrunjdwp:help" command. A new CSR [2] was created for these changes and needs to be reviewed. [1] Webr

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

Hi Yasumasa, The fix in src/hotspot/cpu/x86/macroAssembler_x86.hpp looks Okay. But I don't understand why you need this fix in src/hotspot/share/services/diagnosticArgument.cpp ?: char* buf = NEW_RESOURCE_ARRAY(char, len + 1); - strncpy(b

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

9 11:31 PM, serguei.spit...@oracle.com wrote: Hi Yasumasa, The fix in src/hotspot/cpu/x86/macroAssembler_x86.hpp looks Okay. But I don't understand why you need this fix in src/hotspot/share/services/diagnosticArgument.cpp ?:     char* buf = NEW_RESOURCE_ARRAY(char, len + 1); - strncp

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

On 10/10/19 00:42, David Holmes wrote: On 10/10/2019 5:25 pm, serguei.spit...@oracle.com wrote: On 10/9/19 23:39, Chris Plummer wrote: I think the +1 gets rid of the warning, but it also means there is no need for the following line

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

Hi Yasumasa, If this problem is local then I'd prefer variant A as this solution is local too. If this problem is observed in other places then variant C would be better. Thanks, Serguei On 10/10/19 18:34, Yasumasa Suenaga wrote: Hi, I want to get conclusion of this discussion. I understa

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

Hi Yasumasa, On 10/10/19 21:33, Yasumasa Suenaga wrote: Hi Serguei, On 2019/10/11 12:38, serguei.spit...@oracle.com wrote: Hi Yasumasa, If this problem is local then I'd prefer variant A as this solution is local too. If this problem is observed in other places then variant C wou

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

Hi Yasumasa, I'm Okay with both A and B-1. Personally, I like the simplicity of A but understand this issue is more broad. Thanks, Serguei On 10/11/19 20:22, Yasumasa Suenaga wrote: Hi, I updated Plan B-1 (#pragma) due to the comment from Chris [1].   A. Use memcpy()    http://cr.ope

Re: RFR (XS): 8232182: RedefineNestmateAttr/TestNestmateAttr.java failes due to ObjectCollectedException

Hi David, +1 Thanks, Serguei On 10/14/19 07:46, Hohensee, Paul wrote: Looks good. Paul On 10/14/19, 12:16 AM, "serviceability-dev on behalf of David Holmes" wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8232182 webrev: http://cr.openjdk.java.net/~dholmes/8232182/webrev/

Re: RFR: 8232370: Refactor some com.sun.jdi tests to enable IDE integration

Hi Christoph, It looks good. Thank you for catching and fixing it! Thanks, Serguei On 10/16/19 07:10, Langer, Christoph wrote: Hi,   please review this little test refact

Re: RFR: 8231943: ZGC: Enable serviceability/dcmd/gc/RunGCTest

Hi Per, Looks good. Thanks, Serguei On 10/15/19 23:56, Erik Osterlund wrote: +1 /Erik On 10 Oct 2019, at 14:28, Per Liden wrote: (CC:ing serviceability-dev) On 10/7/19 2:38 PM, Per Liden wrote: This test is currently disabled for ZGC, but it can easily be enabled by adjusting the expe

Re: RFR: 8232084: HotSpot build failed with GCC 9.2.1

Hi Yasumasa, It looks good. One tip: + // This code would be warned as "stringop-truncation" by GCC 8 or later. +PRAGMA_DIAG_PUSH +PRAGMA_STRINGOP_TRUNCATION_IGNORED strncpy(buf, str, len); +PRAGMA_DIAG_POP I'd suggest to place the commen

Re: JDK 14 RFR of JDK-8232448: Suppress warnings on non-serializable non-transient instance fields in jdk.jdi

Hi Joe, Looks good. Thanks, Serguei On 10/16/19 23:15, Joe Darcy wrote: Hello, Please review a serial warning update of three exceptions types in the jdk.jdi module.     JDK-8232448: Suppress warnings on non-serializable non-transient instance fields in jdk.jdi     http://cr.openjdk.jav

Re: RFR (XS) 8231968: getCurrentThreadAllocatedBytes default implementation s/b getThreadAllocatedBytes

+1 Thanks, Serguei On 10/22/19 21:52, Mandy Chung wrote: +1 Mandy On 10/22/19 8:25 PM, David Holmes wrote: Hi Paul, CSR updated and reviewed - please move to Finalized. Code changes Reviewed. Thanks, David On 23/10/2019 1:53 am, Hohensee, Paul wrote: Please review a fixup for https://bu

Re: RFR: JEP 359-Records: hotspot runtime and serviceability code

Hi Vicente and Harold, I'm still reviewing this, it looks pretty good so far. One quick comment about a pre-existed issue (not your fault). The jvmti.xml has an update to add the Record attribute into the list of attributes that must not be changed:

Re: JDK 14 RFR of JDK-8232442: Suppress warnings on non-serializable non-transient instance fields in java.management.*

Hi Joe, Looks good to me. Thanks, Serguei On 10/23/19 10:27, Joe Darcy wrote: Serguei or others working directly in java.management, any other comments on these changes? Thanks, -Joe On 10/17/2019 3:58 PM, Joe Darcy wrote: Hi Roger, I would certainly be in favor of a more thorough revie

Re: RFR: JEP 359-Records: hotspot runtime and serviceability code

Hi Vicente and Harold, The fix looks good to me. Nice set of tests! I have a couple of nits besides what other reviewers already commented.   http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/test/jdk/java/lang

Re: RFR: JDK-8232973: Potential infinite loop in macOS hotspot agent

Hi Simon, +1 Thanks, Serguei On 10/24/19 08:55, Chris Plummer wrote: Hi Simon, The changes look good. thanks, Chris On 10/24/19 7:16 AM, Simon Tooke wrote: Hello, While reviewing uses of strtok() with an eye to moving to strtok_r(), I came accross an inifinite loop in the macOS agent c

Re: RFR: JEP 359-Records: hotspot runtime and serviceability code

Hi Harold, The update looks good. Thanks, Serguei On 10/24/19 10:50, Harold Seigel wrote: Hi David, Thanks for reviewing this! Here's an updated webrev containing the below changes (and one change requested by Serguei).  Could you take a look? http://cr.openjdk.java.net/~hseigel/records.r

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

Hi David, The fix looks good to me. I did not pay much attention to the Graal related changes though. The test coverage for Serviceability is complete. Running java/lang/instrument tests is not necessary. Thanks, Serguei On 10/29/19 00:42, David Holmes wrote: Bug: https://bugs.openjdk.java.ne

Re: RFR: JDK-8231915: two JDI tests interfere with each other

Hi Alex, It looks good. Thanks, Serguei On 11/1/19 16:54, Alex Menkov wrote: Hi all, please review a small fix for https://bugs.openjdk.java.net/browse/JDK-8231915 webrev: http://cr.openjdk.java.net/~amenkov/jdk14/jdwp_test_interference/webrev/ The fix disables "negative" testing for JdwpLi

Re: RFR: 8233285: Demangling C++ symbols in jhsdb jstack --mixed

Hi Yasumasa, This change looks good. Even though it seems to be not impacting platforms other than Linux it is better to make sure it is built Okay on all platforms. Thanks, Serguei On 11/1/19 01:56, Yasumasa Suenaga wrote: (Changed subject to review request) Hi, I converted LinuxDebuggerLo

Re: RFR(S): JDK-8231635: SA Stackwalking code stuck in BasicTypeDataBase.findDynamicTypeForAddress()

Hi Chris, This seems to be a good fix to have in any case. This check and bail out is right thing to do and should not break anything. I understand, this also fixes the test failures. I only had some experience a long time ago with the support of pstack and DTrace jstack action implementation w

Re: RFR (XS) 8233790: Forward output from heap dumper to jcmd/jmap

Hi Ralf, The fix looks Okay in general. A couple of questions. The dumper.dump() returns int value. Returned value is not used anymore in the attachListener.cpp and diagnisticCommand.cpp. Is it still used somewhere els

Re: RFR (XS) 8233790: Forward output from heap dumper to jcmd/jmap

oved code is consolidated into HeapDumper::dump(). thanks, Chris On 11/8/19 4:25 PM, serguei.spit...@oracle.com wrote: Hi Ralf, The fix looks Okay in general. A couple of questions. The dumper.dump() returns int value. Returned value is not used anymore in the attachListener.cpp and diagn

Re: RFR (XS) 8233790: Forward output from heap dumper to jcmd/jmap

outputStream rather than to the tty. Chris On 11/8/19 5:02 PM, serguei.spit...@oracle.com wrote: Hi Chris, I'm a little bit confused. The Ralf's change in the HeapDumper::dump() is just replacement of 'tty' occurrences with 'out', so the change has not moved the de

Re: RFR (XS) 8233790: Forward output from heap dumper to jcmd/jmap

); 1986   out->print_cr("Unable to create %s: %s", path, 1987 (error() != NULL) ? error() : "reason unknown"); -  } else { -    out->print_cr("%s", error); -  } 2012   out->print_cr("Dump file is incomplete: %s", writer.error(

Re: RFR (XS) 8233790: Forward output from heap dumper to jcmd/jmap

Hi Ralf, Okay, thanks! Serguei On 11/11/19 00:48, Schmelter, Ralf wrote: Hi Serguei, Thanks for the review. The only open question seems to be: The dumper.dump() returns int value. Returned value is not used anymore in the attachListener.cpp and diagnisticCommand.cpp. Is it still used somew

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, On 11/8/19 16:55, Chris Plummer wrote: Hi Alex, Comments below: On 11/8/19 4:39 PM, Alex Menkov wrote: On 11/08/2019 15:22, Alex Menkov wrote: Hi all, Please review the fix for https://bugs.openjdk.java.net/browse/JDK-8215196 webrev: http://cr.openjdk.java.net/~amenkov/jdk14/po

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"

On 11/11/19 03:06, serguei.spit...@oracle.com wrote: Hi Chris, On 11/8/19 16:55, Chris Plummer wrote: Hi Alex, Comments below: On 11/8/19 4:39 PM, Alex Menkov wrote: On 11/08/2019 15:22, Alex Menkov wrote: Hi all, Please review the fix for https://bugs.openjdk.java.net/browse/JDK

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 Alex, The fix itself looks Okay. Minor: replace in the comment: "compiler don't drop" => "compiler doesn't drop". However, we still have to reach a consensus on how we treat this issue (as Chris already commented). T

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

Hi David, Just wanted to let you know I'm reviewing this. Thanks, Serguei On 11/11/19 20:52, David Holmes wrote: webrev: http://cr.openjdk.java.net/~dholmes/8233549/webrev/ bug: https://bugs.openjdk.java.net/browse/JDK-8233549 In JDK-8229516 I moved the interrupted state of a thread from the

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

Hi David, It looks good to me. A couple of nits below. http://cr.openjdk.java.net/~dholmes/8233549/webrev/src/hotspot/share/prims/jvmtiRawMonitor.cpp.frames.html 236 if (self->is_Java_thread()) { 237 JavaThread* jt = (JavaThread*) self; 238

Re: RFR(S) : 8233462 : serviceability/tmtools/jstat tests times out with -Xcomp

Hi Igor, Looks good. Thank you for taking care about this! Thanks, Serguei On 11/15/19 23:47, Igor Ignatyev wrote: http://cr.openjdk.java.net/~iignatyev//8233462/webrev.00 33 lines changed: 1 ins; 14 del; 18 mod; Hi all, could you please review this small fix for tmtools testlibrary? tmtoo

Re: RFR(XS): JDK-8234358: Update ProblemList entry for NashornPopFrameTest

+1 Thanks, Serguei On 11/18/19 12:47, Chris Plummer wrote: Looks good. Chris On 11/18/19 11:30 AM, Alex Menkov wrote: Hi Chris, It makes sense. I created new issue and closed JDK-8187143 as a dup of JDK-8225620 So changing this RFR to be RFR for the new issue: https://bugs.openjdk.java.ne

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

I put more debugging in the bug to show this crash was from an unloaded nmethod. Coleen On 11/15/19 4:45 PM, serguei.spit...@oracle.com wrote: Hi Col

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

wrote: On 11/15/19 11:17 PM, serguei.spit...@oracle.com wrote: Hi Coleen, On 11/15/19 2:12 PM, coleen.phillim...@oracle.com

Re: RFR: 8215355: Object monitor deadlock with no threads holding the monitor (using jemalloc 5.1)

Hi David, The fix looks good. It is besides the platform-dependent code that Thomas flagged. There can be similar broken code on other platforms. For instance, there is a suspicious spot in cpu/ppc/frame_ppc.cpp:     // sender_fp must be within the stack and above (but not     // equal) current

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

Please, review a fix for:   https://bugs.openjdk.java.net/browse/JDK-8169467 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8169467-jvmti-local-instance.1/ Summary:   The JVMTI GetLocalInstance function should return JVMTI_ERROR_INVALID_SLOT for static method frames.   Instead, it r

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

Hi Marcus, It looks good in general. A couple of comments though. http://cr.openjdk.java.net/~mgronlun/8233197/webrev01/src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp.frames.html 258 class JvmtiPhaseTransition { 259 private: 260 bool

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

Thank you, Chris! Serguei On 11/20/19 18:22, Chris Plummer wrote: +1 On 11/20/19 3:32 PM, Alex Menkov wrote: Looks good. --alex On 11/19/2019 15:36, serguei.spit...@oracle.com wrote: Please, review a fix for:    https://bugs.openjdk.java.net/browse/JDK-8169467 Webrev: http

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

33 PM, serguei.spit...@oracle.com wrote: Hi Coleen, Looks good in general. Serguei,  Thank you for reviewing this. Nice approach, thank you for working on this! It is great to get rid of the nmethodLocker

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

the GC people will like, and I like.  I'm testing it out now. Thanks! Coleen On 11/18/19 10:03 PM, serguei.spit...@oracle.com wrote: Hi Coleen,

Re: RFR (S) 8173658: JvmtiExport::post_class_unload() is broken for non-JavaThread initiators

Hi Coleen, It looks good in general. Just one minor request: http://cr.openjdk.java.net/~coleenp/2019/8173658.01/webrev/src/hotspot/share/prims/jvmtiImpl.hpp.frames.html http://cr.openjdk.java.net/~coleenp/2019/8173658.01/webrev/src/hotspot/share/prims/jvmtiImpl

Re: RFR (S) 8173658: JvmtiExport::post_class_unload() is broken for non-JavaThread initiators

On 11/22/19 17:35, coleen.phillim...@oracle.com wrote: On 11/22/19 6:45 PM, serguei.spit...@oracle.com wrote: Hi Coleen, It looks good in general. Just one minor request: http

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

Please, review a fix for test bug:   https://bugs.openjdk.java.net/browse/JDK-8221372 Webrev:   http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8221372-jvmti-thread-state.1/ Summary:   The test timeouts always happen with the JFR recording and

Re: RFR (XS) 8235273: nmethodLocker not needed for COMPILED_METHOD_UNLOAD events

Hi Coleen, +1 Thanks, Serguei On 12/3/19 20:49, David Holmes wrote: Hi Coleen, That all seems fine to me. Thanks, David On 4/12/2019 4:21 am, coleen.phillim...@oracle.com wrote: Summary: remove unnecessary nmethodLocker See bug for more details.  Tested with tier2-8. open webrev at htt

Re: RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

On 12/4/19 04:21, coleen.phillim...@oracle.com wrote: On 12/3/19 11:39 PM, David Holmes wrote: On 3/12/2019 11:35 pm, coleen.phillim...@oracle.com wrote: On 12/3/19 8:31 AM, David Holmes wrote: On 3/12/2019 11:08 pm, coleen.phillim...@oracle.com wrote: On 12/2/19 11:52 PM, David Holme

Re: RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

Hi Collen, It looks good in general. Thank you a lot for sorting this out! Just a couple of comments. http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev/src/hotspot/share/runtime/thread.hpp.frames.html 1993 protected: 1994 //

Re: RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

On 12/4/19 14:15, serguei.spit...@oracle.com wrote: Hi Collen, Sorry, I typed your name incorrectly. Thanks, Serguei It looks good in general. Thank you a lot for sorting this out

Re: RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

wrote: Hi Serguei, On 12/4/19 5:15 PM, serguei.spit...@oracle.com wrote: Hi Coleen, It looks good in general. Thank you a lot for sorting this out! Just a couple of comments.

Re: RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

n.phillim...@oracle.com wrote: Hi Serguei, On 12/4/19 5:15 PM, serguei.spit...@oracle.com wrote: Hi Collen, (no problem) It looks good in general.

Re: RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

On 12/5/19 10:36, coleen.phillim...@oracle.com wrote: On 12/5/19 11:00 AM, serguei.spit...@oracle.com wrote: Hi Collen, Thank you for making this update! It looks good to me. One nit

Re: RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

Got it, thanks! Serguei On 12/5/19 11:15, coleen.phillim...@oracle.com wrote: On 12/5/19 1:41 PM, serguei.spit...@oracle.com wrote: On 12/5/19 10:36, coleen.phillim...@oracle.com wrote

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

Hi Harold, Okay, thanks! Thanks, Serguei On 12/6/19 05:16, Harold Seigel wrote: There will be another unmodifiable attribute with sealed types called PermittedSubtypes. Harold On 12/5/2019 7:25 PM, serguei.spit...@oracle.com wrote: Hi David, Agreed. I was thinking about the same

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

Forgot to ask. Is this new attribute for 14? Will it also come from Amber? Thanks, Serguei On 12/6/19 10:21, serguei.spit...@oracle.com wrote: Hi Harold, Okay, thanks! Thanks, Serguei On 12/6/19 05:16, Harold Seigel wrote: There will be another unmodifiable attribute with sealed types

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

Thanks, Harold. Serguei On 12/6/19 10:29, Harold Seigel wrote: Hi Serguei, >> Is this new attribute for 14? No.  15, maybe? >>Will it also come from Amber? Yes. Harold On 12/6/2019 1:27 PM, serguei.spit...@oracle.com wrote: Forgot to ask. Is this new attribute for 14? Will

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"

On 12/6/19 11:07, Chris Plummer wrote: On 12/5/19 6:45 PM, David Holmes wrote: Hi Serguei, On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote: Hi Chris and Alex, (I've also included Dan, David and Dean to the mailing list) We have to reach a consensus about this. This is just pa

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"

On 12/6/19 13:52, Chris Plummer wrote: On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote: On 12/6/19 11:07, Chris Plummer wrote: On 12/5/19 6:45 PM, David Holmes wrote: Hi Serguei, On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote: Hi Chris and Alex, (I've also included Dan,

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"

The PopFrame together with RedefineClasses is a part of the JVM TI HotSwap feature. The use case is to hot patch the methods. If after class redefinition there are still some method frames then the PopFrame is an option to "refresh" such frames. I agree, this is unreliable and dangerous. But th

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"

On 12/6/19 17:24, Daniel D. Daugherty wrote: On 12/6/19 6:26 PM, serguei.spit...@oracle.com wrote: On 12/6/19 13:52, Chris Plummer wrote: On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote: On 12/6/19 11:07, Chris Plummer wrote: On 12/5/19 6:45 PM, David Holmes wrote: Hi Serguei, On 6/12

Re: RFR: 8235530: Removed duplicated threadByName methods in nsk/jdi tests

Hi Leonid, The fix looks good. Thank you for taking care about it! I agree, it is an awful duplication. Thanks, Serguei On 12/7/19 18:17, Leonid Mesnik wrote: Hi Could you please review following fix which just remove duplicated threadByName methods and JDITestRuntimeException exceptions i

Re: RFR: 8226575: OperatingSystemMXBean should be made container aware

Hi Daniil, It is not a full review, just some minor comments. In fact, I do not see real problems yet. http://cr.openjdk.java.net/~dtitov/8226575/webrev.05/src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java.frames.html   55 public long getTotalSwapSpaceSize

Re: RFR: 8226575: OperatingSystemMXBean should be made container aware

ns -1 if something went wrong). But we could revise it in the follow up issue I created for that [1]. [1] https://bugs.openjdk.java.net/browse/JDK-8235522 Thank you, Daniil On 12/9/19, 6:02 PM, "serguei.spit...@oracle.com" wrote: Hi Daniil, It is not a full re

Re: RFR: 8226575: OperatingSystemMXBean should be made container aware

ds. Thank you, Daniil On 12/11/19, 3:13 PM, "serguei.spit...@oracle.com" wrote: Hi Daniil, One my concerns was a non-atomic read of multiple metrics before comparison. It creates a potential to get a mismatch in result. However, the probability to get a neg

Re: RFR: 8226797: serviceability/tmtools/jstat/GcCapacityTest.java fails with Exception: java.lang.RuntimeException: OGCMN > OGCMX (min generation capacity > max generation capacity)

Hi Stefan, It looks good to me. Sorry, I was on the meeting, wrote this email and forgot to push 'send' button. Just now discovered that it has not been really sent. :( Thanks, Serguei On 12/12/19 07:23, Stefan Karlsson wrote: In the interest to get this integrated before the RDP cut-off I'

Re: RFR(S) 8235970 [TESTBUG] Remove dependency of sun.tools.jar from RedefineClassHelper

Hi Ioi, It looks good. It is nice to get rid of unneeded dependency. Thanks, Serguei On 12/15/19 23:22, Alan Bateman wrote: On 16/12/2019 06:21, Ioi Lam wrote: : The fix is to rewrite RedefineClassHelper to use ClassFileInstaller instead. This looks okay but just to point out that the jar

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

should be "RFR 8235829: graal crashes with Zombie.java test". On 12/17/19 8:17 PM, serguei.spit...@oracle.com wrote: Hi Coleen, Is this webrev v2 right to look at? :   http://cr.openjdk.java.net/~coleenp/20

Re: [14] RFR 8235829: graal crashes with Zombie.java test

Hi Coleen, Just wanted to confirm the webrev V2 version looks okay to me. Sorry for replying on the wrong mailing thread. Thanks, Serguei On 12/18/19 08:42, coleen.phillim...@oracle.com wrote: On 12/18/19 8:45 AM, David Holmes wrote: Thanks for the additional info Coleen! Just to add a bi

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

Hi Robbin, The fix looks good to me. At least, I do not see any issues with it. Thank you for removing the unused code! Could you be more precise about on what jvmti/jdi tests you run? For good test coverage we need these test suites: vmTestbase_nsk_jvmti, vmTestbase_nsk_jdi, vmTestbase_nsk_jdb,

Re: [14]RFR(XS): 8236062: Disable clhsdb initialization of SA javascript support since it will always fail, and will likely be removed soon

Hi Chris, It looks okay. I wonder if we have it documented anywhere. Do we also need any doc cleanup? Thanks, Serguei On 12/16/19 21:36, Chris Plummer wrote: Hello, Please review the fo

Re: [14]RFR(XS): 8236062: Disable clhsdb initialization of SA javascript support since it will always fail, and will likely be removed soon

only when javascript initialization was working. When it's broken you don't see these commands. Chris On 12/18/19 8:40 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks okay. I wonder if we have itdocumented anywhere. Do we also need any doc cleanup? Thanks, Serguei On

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

Please, review a trivial fix for the doc bug:   https://bugs.openjdk.java.net/browse/JDK-8229847 The patch is: diff --git a/src/jdk.attach/share/classes/com/sun/tools/attach/spi/AttachProvider.java b/src/jdk.attach/share/classes/com/sun/tools/attach/spi/AttachProvider.java --- a/src/jdk.attac

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

Hi David, On 1/8/20 14:54, David Holmes wrote: Hi Serguei, On 9/01/2020 7:42 am, serguei.spit...@oracle.com wrote: Please, review a trivial fix for the doc bug:    https://bugs.openjdk.java.net/browse/JDK-8229847 The patch is: diff --git a/src/jdk.attach/share/classes/com/sun/tools/attach

Re: RFR: JDK-8235846: Improve WindbgDebuggerLocal implementation

Hi Alex, It looks pretty good. Just some minor comments below. The class AutoCOMPtr has unfixed indents. I guess, the function AutoArrayPtr.asPtr() is not used anymore and can be deleted. I'd suggest to remove first level '()'

  1   2   3   4   5   6   7   8   9   10   >