On Fri, 29 Jan 2021 22:54:57 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Lin Zang has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - fix an issue of double printing error message.
>> - fix jcmd jmap issue and add test in BascJMapTest and code refine
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/SALauncher.java line 130:
>
>> 128: System.out.println(" --binaryheap To dump java
>> heap in hprof binary format.");
>> 129: System.out.println(" --dumpfile <name> The name of the
>> dump file.");
>> 130: System.out.println(" --gz <1-9> The compression
>> level for gzipped dump file.");
>
> `--dumpfile` and `--gz` can only be used with `--binaryheap`. That should be
> made clear in the help text.
>
> I just noticed that `jhsdb pmap` does not support the equivalent of
> `format=x` that the jcmd version does. However, the support is actually there
> in the SA JMap.java. It's confusing because both `jhsdb jmap` and the clhsdb
> `dumpheap` command use the JMap class, but neither support passing `format=x`
> to it. In order to get SA to dump with `format=x`, you would need to launch
> `sun/jvm/hotspot/tools/JMap` directly and pass in `-heap:format=x`. I'm not
> suggesting you fix that, but just something for us to be aware of.
>
> One question though. Have you tried running JMap directly to make sure the
> help output looks right. You've made fixes to it in this webrev, but since
> there are so many different "jmap" commands (5), and each with their own
> help, I've lost sight of whether or not this is even tested. I don't believe
> it is, but I wouldn't worry about that. However, since made changes to the
> help output, it would be good to at least check it out.
>
> And if you are wonder what the 5 `jmap` commands are:
>
> - SA's `jhsdb jmap --binaryheap` (which uses JMap.java with `-heap:format=b`)
> - SA's clhsdb `dumpheap` command (which also uses JMap.java with
> `-heap:format=b`)
> - executing SA's `sun/jvm/hotspot/tools/JMap` class directly
> - `jmap -dump` command (Uses Attach API's `heapdump` command, which uses
> hotspot `HeapDump` class)
> - `GC.heap_dump` dcmd, which also uses the hotspot `HeapDumper` class
>
> So really there are two implementations of heap dumping, one in the VM and
> one in SA, but a total of 5 ways to get to them. I'm just starting to get my
> head wrapped around all this myself. We've created a very confusing situation
> for us, and unfortunately I think some of the changes that got us here was
> driven by a goal to simply things.
Thanks for the thorough list, I have tested most of them but not all. I will
cover them and update here later.
> So really there are two implementations of heap dumping, one in the VM and
> one in SA, but a total of 5 ways to get to them. I'm just starting to get my
> head wrapped around all this myself. We've created a very confusing situation
> for us, and unfortunately I think some of the changes that got us here was
> driven by a goal to simply things.
Yes, it makes me even curious about why there can be two different
implementation of them (one by C++ and one by Java), it seems we need double
the work of adding new options for those tools, like parallal for jmap histo
and dump, etc.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1712
