Re: Thread Local Handshake in JVMTI functions

2020-04-09 Thread Robbin Ehn
Hi, adding the same comment as in the bug regarding GetThreadListStackTraces. Please note that there is a semantic difference taking samples in a safepoint and in handshakes, if there are mutiple thread sampled. With a safepoint; stacktraces are taken from the same exact moment (from a Java muta

Re: Thread Local Handshake in JVMTI functions

2020-04-09 Thread Yasumasa Suenaga
Hi Robbin, I think we can change GetThreadListStackTrace(VM_GetThreadListStackTraces) if the caller requests only 1 thread stack (thread_count == 1). It does not break JVMTI spec. In other case, we should use safepoint (VM Operation) for following JVMTI spec: ``` All stacks are collected simul

Re: Thread Local Handshake in JVMTI functions

2020-04-09 Thread Robbin Ehn
Hi Yasumasa, We have had internal requests doing GetThreadListStackTraces with multiple threads with handshakes. Since you can sample hundreds of times per second using handshakes with little interference with your application. The internal request sampled all threads ~10 times per second. So the

Re: Thread Local Handshake in JVMTI functions

2020-04-09 Thread Yasumasa Suenaga
On 2020/04/09 16:19, Robbin Ehn wrote: Hi Yasumasa, We have had internal requests doing GetThreadListStackTraces with multiple threads with handshakes. Since you can sample hundreds of times per second using handshakes with little interference with your application. The internal request sampled

Re: Thread Local Handshake in JVMTI functions

2020-04-09 Thread Robbin Ehn
Ok, thanks for looking into it! /Robbin On 2020-04-09 09:39, Yasumasa Suenaga wrote: On 2020/04/09 16:19, Robbin Ehn wrote: Hi Yasumasa, We have had internal requests doing GetThreadListStackTraces with multiple threads with handshakes. Since you can sample hundreds of times per second using

RFR 8242430: Correct links in javadoc of OperatingSystemMXBean

2020-04-09 Thread Daniil Titov
Please review a javadoc fix [1] that corrects the links in the description of getTotalPhysicalMemorySize() method in com.sun.management. OperatingSystemMXBean class. The CSR [2] needs a reviewer as well. [1] Webrev: http://cr.openjdk.java.net/~dtitov/8242430/webrev.01/ [2] CSR: https://bugs.

Re: RFR 8242430: Correct links in javadoc of OperatingSystemMXBean

2020-04-09 Thread David Holmes
On 9/04/2020 5:45 pm, Daniil Titov wrote: Please review a javadoc fix [1] that corrects the links in the description of getTotalPhysicalMemorySize() method in com.sun.management. OperatingSystemMXBean class. The CSR [2] needs a reviewer as well. [1] Webrev: http://cr.openjdk.java.net/~dtitov

Re: Thread Local Handshake in JVMTI functions

2020-04-09 Thread David Holmes
On 9/04/2020 5:39 pm, Yasumasa Suenaga wrote: On 2020/04/09 16:19, Robbin Ehn wrote: Hi Yasumasa, We have had internal requests doing GetThreadListStackTraces with multiple threads with handshakes. Since you can sample hundreds of times per second using handshakes with little interference with

Re: RFR (XS): 8242241: add assert to ClassUnloadEventImpl::className

2020-04-09 Thread Daniel D. Daugherty
On 4/7/20 6:39 PM, serguei.spit...@oracle.com wrote: Please, review a fix for minor JDI enhancement: https://bugs.openjdk.java.net/browse/JDK-8242241 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/8242241-jdi-add-assert.1/ src/jdk.jdi/share/classes/com/sun/tools/jdi/EventSetImpl.jav

Re: RFR (XS): 8242241: add assert to ClassUnloadEventImpl::className

2020-04-09 Thread serguei.spit...@oracle.com
Thanks you, Dan! Serguei On 4/9/20 06:34, Daniel D. Daugherty wrote: On 4/7/20 6:39 PM, serguei.spit...@oracle.com wrote: Please, review a fix for minor JDI enhancement: https://bugs.openjdk.java.net/browse/JDK-8242241 Webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/8242241-jdi-add-

RFR(T) 8184249: SA: clhsdb 'intConstant' throws a NullPointerException when not attached to a VM

2020-04-09 Thread Chris Plummer
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8184249 http://cr.openjdk.java.net/~cjplummer/8184249/webrev.00/index.html Details are in the CR. I'd like to consider this fix as trivial, and have also added the noreg-trivial label to the CR since there is no test

RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Alex Menkov
Hi all, Please review the fix for https://bugs.openjdk.java.net/browse/JDK-8242282 webrev: http://cr.openjdk.java.net/~amenkov/jdk15/jpsTest_ClsNotFound/webrev/ The test creates jar with test classes and run it with "java -jar ". The problem is single "@run Test" tag is executed by JTreg incon

Re: RFR(T) 8184249: SA: clhsdb 'intConstant' throws a NullPointerException when not attached to a VM

2020-04-09 Thread Alex Menkov
Hi Chris, Looks good and trivial. --alex On 04/09/2020 08:57, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8184249 http://cr.openjdk.java.net/~cjplummer/8184249/webrev.00/index.html Details are in the CR. I'd like to consider this fix as

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Daniel D. Daugherty
On 4/9/20 4:42 PM, Alex Menkov wrote: Hi all, Please review the fix for https://bugs.openjdk.java.net/browse/JDK-8242282 webrev: http://cr.openjdk.java.net/~amenkov/jdk15/jpsTest_ClsNotFound/webrev/ test/jdk/sun/tools/jps/LingeredAppForJps.java     L89:     manifestClasspath += " "

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Daniel D. Daugherty
Sorry, pressed "send" too soon. There's no information on how this fix was tested. Right now we're seeing a varying number of failures in almost every Tier5 CI job set. Please verify that you've tested Tier5. Dan On 4/9/20 5:01 PM, Daniel D. Daugherty wrote: On 4/9/20 4:42 PM, Alex Menkov wrot

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Alex Menkov
On 04/09/2020 14:01, Daniel D. Daugherty wrote: On 4/9/20 4:42 PM, Alex Menkov wrote: Hi all, Please review the fix for https://bugs.openjdk.java.net/browse/JDK-8242282 webrev: http://cr.openjdk.java.net/~amenkov/jdk15/jpsTest_ClsNotFound/webrev/ test/jdk/sun/tools/jps/LingeredAppForJps.ja

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Alex Menkov
On 04/09/2020 14:04, Daniel D. Daugherty wrote: Sorry, pressed "send" too soon. There's no information on how this fix was tested. Right now we're seeing a varying number of failures in almost every Tier5 CI job set. Please verify that you've tested Tier5. Tested open/test/jdk/sun/tools/jps/

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Daniel D. Daugherty
On 4/9/20 5:15 PM, Alex Menkov wrote: On 04/09/2020 14:01, Daniel D. Daugherty wrote: On 4/9/20 4:42 PM, Alex Menkov wrote: Hi all, Please review the fix for https://bugs.openjdk.java.net/browse/JDK-8242282 webrev: http://cr.openjdk.java.net/~amenkov/jdk15/jpsTest_ClsNotFound/webrev/ test/

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Daniel D. Daugherty
On 4/9/20 5:17 PM, Alex Menkov wrote: On 04/09/2020 14:04, Daniel D. Daugherty wrote: Sorry, pressed "send" too soon. There's no information on how this fix was tested. Right now we're seeing a varying number of failures in almost every Tier5 CI job set. Please verify that you've tested Tier

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Chris Plummer
Hi Alex, The fix looks good, but the test is in need of some commenting. It took a fair amount of staring at the code,  the CR, and the RFR to figure out what it is doing and why. Can you add a few comments? // Add the main class to the jar file. It should only be found in one classpath, the

RFR(S) : 8242471 : remove "temporarily" restrictions of nsk/jdi/Argument/value/value004

2020-04-09 Thread Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev/8242471/webrev.00/ > 38 lines changed: 8 ins; 23 del; 7 mod; Hi all, could you please review this small patch for nsk/jdi/Argument/value/value004 test? from JBS: > nsk/jdi/Argument/value/value004 test has restrictions to be run only on > solaris-sparc and

Re: RFR(S) : 8242471 : remove "temporarily" restrictions of nsk/jdi/Argument/value/value004

2020-04-09 Thread Chris Plummer
Hi Igor,   89 log.display("Connector's transport is: " + c.transport().name()); Would be nice if you printed each transport's name (even the skipped ones). That way when reading the log after getting SkippedException you can see which transports were attempted. You removed the

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Alex Menkov
Hi Chris, I tried to describe main idea of the code. updated webrev: http://cr.openjdk.java.net/~amenkov/jdk15/jpsTest_ClsNotFound/webrev.01/ the only change vs prev. webrev is added comment in LingeredAppForJps.java --alex On 04/09/2020 14:28, Chris Plummer wrote: Hi Alex, The fix looks go

RFR(XS) 8237250: pmap and pstack should do a better of making it clear that they are not supported on Mac OS X

2020-04-09 Thread Chris Plummer
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8237250 http://cr.openjdk.java.net/~cjplummer/8237250/webrev.01/index.html Details are in the CR. thanks, Chris

Re: RFR: JDK-8242282: Test sun/tools/jps/TestJps.java fails after JDK-8237572

2020-04-09 Thread Chris Plummer
Hi Alex, Please add a newline between lines 70 and 71.   77 // are writen the jar manifest. "writen" -> "written to" No need to see another review. thanks, Chris On 4/9/20 3:50 PM, Alex Menkov wrote: Hi Chris, I tried to describe main idea of the code. updated webrev: http://cr.o

Re: RFR(S) : 8242471 : remove "temporarily" restrictions of nsk/jdi/Argument/value/value004

2020-04-09 Thread Igor Ignatyev
Hi Chris, I looked what transport got skipped on Windows, and it's dt_shmem, as I didn't see any reasons why this test should not be run w/ this transport, I looked deeper and I found the place which needs to be updated to make the test work w/ dt_shmem. so now the test doesn't skip any transpo

RFR: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker

2020-04-09 Thread 傅杰
Hi all, JBS:https://bugs.openjdk.java.net/browse/JDK-8242480 Webrev: http://cr.openjdk.java.net/~jiefu/8242480/webrev.00/ Negative values were returned by getFreeSwapSpaceSize() in our docker testing. The reason is that current implementation doesn't consider the situation when memory.limit_

Re: RFR(S) : 8242471 : remove "temporarily" restrictions of nsk/jdi/Argument/value/value004

2020-04-09 Thread Chris Plummer
Hi Igor, The changes looks good. thanks, Chris On 4/9/20 4:13 PM, Igor Ignatyev wrote: Hi Chris, I looked what transport got skipped on Windows, and it's dt_shmem, as I didn't see any r

Re: RFR(S) : 8242471 : remove "temporarily" restrictions of nsk/jdi/Argument/value/value004

2020-04-09 Thread Igor Ignatev
Thanks Chris! Would you consider this trivial or should I wait for another review? — Igor > On Apr 9, 2020, at 8:35 PM, Chris Plummer wrote: > >  > Hi Igor, > > The changes looks good. > > thanks, > > Chris > >> On 4/9/20 4:13 PM, Igor Ignatyev wrote: >> Hi Chris, >> >> I looked what tra

RFR(S) 8235220: ClhsdbScanOops.java fails with sun.jvm.hotspot.types.WrongTypeException

2020-04-09 Thread Chris Plummer
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8235220 http://cr.openjdk.java.net/~cjplummer/8235220/webrev.00 First off, thanks to Ioi for tracking this one down and proposing the fix. The test is executing the clhsdb "scanoops" command, which scans the specifie

Re: RFR(S) : 8242471 : remove "temporarily" restrictions of nsk/jdi/Argument/value/value004

2020-04-09 Thread Chris Plummer
Hi Igor, I think it's best to get another review. thanks, Chris On 4/9/20 8:49 PM, Igor Ignatev wrote: Thanks Chris! Would you consider this trivial or should I wait for another review? — Igor

RFR: 8242425: JVMTI monitor operations should use Thread-Local Handshakes

2020-04-09 Thread Yasumasa Suenaga
Hi all, Please review this change: JBS: https://bugs.openjdk.java.net/browse/JDK-8242425 webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8242425/webrev.00/ We've discussed to use Thread-Local Handshake in some JVMTI functions [1]. This change is for monitor functions. It affects GetOwnedMo

Re: RFR(S) 8235220: ClhsdbScanOops.java fails with sun.jvm.hotspot.types.WrongTypeException

2020-04-09 Thread Ioi Lam
Hi Chris, Thanks for fixing this. It looks good to me. - Ioi On 4/9/20 9:44 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8235220 http://cr.openjdk.java.net/~cjplummer/8235220/webrev.00 First off, thanks to Ioi for tracking this one do

Re: RFR: 8242425: JVMTI monitor operations should use Thread-Local Handshakes

2020-04-09 Thread serguei.spit...@oracle.com
Hi Yasumasa, It looks pretty good in general. A couple of comments though. http://cr.openjdk.java.net/~ysuenaga/JDK-8242425/webrev.00/src/hotspot/share/prims/jvmtiEnvBase.cpp.frames.html 650 JvmtiEnvBase::get_current_contended_monitor(JavaThread *java_thr