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

2019-11-08 Thread Chris Plummer
Hi Serguei, I've inlined below the corresponding code in HeapDumper::dump() that covers the deleted output functionality from attachListener.cpp. Note it's not exactly the same, but I think in the end it includes at least the same info in all cases, and in some cases more info: -    int res

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

2019-11-08 Thread serguei.spit...@oracle.com
Okay, thanks. Agreed, this aspect was clear to me. The deleted fragments are about printing the summarizing conclusion about the dumping which is not present in the HeapDumper::dump(). I expected it to be moved into the HeapDumper::dump() but it was not. So, I wonder if there was such an

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

2019-11-08 Thread Chris Plummer
He also added the outputStream argument to HeapDumper::dump(). Both of the sections of code below already called dump(), and now they do so with the added outputStream argument. HeapDumper::dump() has been modified to print on the outputStream rather than to the tty. Chris On 11/8/19 5:02

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"

2019-11-08 Thread Alex Menkov
On 11/08/2019 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:

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

2019-11-08 Thread serguei.spit...@oracle.com
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 deleted code in these files into the HeapDumper::dump(). Probably, you wanted to say that the pre-existed error messages

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"

2019-11-08 Thread Chris Plummer
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/popframe_args/webrev/ I don't really see a resolution

Re: RFR: 8233868: Unproblem list sun/tools/jstat/jstatClassloadOutput1.sh

2019-11-08 Thread Chris Plummer
+1 On 11/8/19 4:40 PM, Alex Menkov wrote: LGTM --alex On 11/08/2019 14:58, Daniil Titov wrote: Please a review a changeset below that removes test sun/tools/jstat/jstatClassloadOutput1.sh  from test/jdk/ProblemList.txt. diff -r f92ef5d182b5 test/jdk/ProblemList.txt ---

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

2019-11-08 Thread Chris Plummer
Hi Ralf, Also looks good to me. Serguei, the removed 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

Re: RFR: 8233868: Unproblem list sun/tools/jstat/jstatClassloadOutput1.sh

2019-11-08 Thread Alex Menkov
LGTM --alex On 11/08/2019 14:58, Daniil Titov wrote: Please a review a changeset below that removes test sun/tools/jstat/jstatClassloadOutput1.sh from test/jdk/ProblemList.txt. diff -r f92ef5d182b5 test/jdk/ProblemList.txt --- a/test/jdk/ProblemList.txt Fri Nov 08 11:41:17 2019 -0500 +++

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"

2019-11-08 Thread Alex Menkov
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/popframe_args/webrev/ Currently PopFrame is disabled with JVMCI by [1], so for testing I reverted [1] changes. Just

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

2019-11-08 Thread serguei.spit...@oracle.com
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

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

2019-11-08 Thread serguei.spit...@oracle.com
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

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"

2019-11-08 Thread Alex Menkov
Hi all, Please review the fix for https://bugs.openjdk.java.net/browse/JDK-8215196 webrev: http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/ Currently PopFrame is disabled with JVMCI by [1], so for testing I reverted [1] changes. [1]

RFR: 8233868: Unproblem list sun/tools/jstat/jstatClassloadOutput1.sh

2019-11-08 Thread Daniil Titov
Please a review a changeset below that removes test sun/tools/jstat/jstatClassloadOutput1.sh from test/jdk/ProblemList.txt. diff -r f92ef5d182b5 test/jdk/ProblemList.txt --- a/test/jdk/ProblemList.txt Fri Nov 08 11:41:17 2019 -0500 +++ b/test/jdk/ProblemList.txt Fri Nov 08 22:37:11 2019

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

2019-11-08 Thread Chris Plummer
Ping! On 11/7/19 2:01 PM, Chris Plummer wrote: Hi, Please review the following fix for JDK-8231635: https://bugs.openjdk.java.net/browse/JDK-8231635 http://cr.openjdk.java.net/~cjplummer/8231635/webrev.00/ I've tried to explain below to the best of my ability what's is going on, but keep in

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

2019-11-08 Thread Thomas Stüfe
Hi Ralf, On Fri, Nov 8, 2019 at 4:45 PM Schmelter, Ralf wrote: > Hi Thomas, > > thanks for the review. > > > > Can you please add a comment about the new parameter? E.g. "optional > outputStream to which progress- and error messages will be written". > > Will do. > > > > - in

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

2019-11-08 Thread Schmelter, Ralf
Hi Thomas, thanks for the review. > Can you please add a comment about the new parameter? E.g. "optional > outputStream to which progress- and error messages will be written". Will do. > - in HeapDumper::dump(): while you are on it could you please initialize > my_path to NULL at function

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

2019-11-08 Thread Thomas Stüfe
Hi Ralf, this makes sense. Some small remarks: --- heapDumper.hpp // dumps the heap to the specified file, returns 0 if success. - int dump(const char* path); + int dump(const char* path, outputStream* out = NULL); Can you please add a comment about the new parameter? E.g. "optional

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

2019-11-08 Thread Schmelter, Ralf
This change forwards the output from the HeapDumper.dump() command to an optional output stream supplied by the caller. Until now the diagnositic command and the "dumpheap" command of the attach framework partly recreated the output by hand, but missing some information. Old output: Heap dump