RFR: 8252842: Extend jmap to support parallel heap dump [v10]

[Chris Plummer]

On Wed, 24 Feb 2021 01:08:58 GMT, Lin Zang <lzang at openjdk.org> wrote:

>>>  I just realized that my understand is a little different with the "retrying" - if we already know what is newly added argument, we can assume old jvm can not accept it. So it seems print error message is enough. This is consistent with JDK16.
>>> But for older jvm, since there is no error message for unrecogized arguments, we need to fall back as you mentioned.
>> 
>> It still not clear to me what is happening when `jmap -dump:gz=...` is used by JDK17 jmap to query and older JDK. When I tried it, it silently worked, but without compressing the output file. Since jmap doesn't know what JDK it is communicating with, I can only assume it tried passing `gz=`, and when that failed and retried without it. Or maybe the old JDK is just silently consuming the unrecognized `gz=` argument. I'd like to understand the implementation behind this behavior.
>> 
>>> So maybe we can also revert the argument parsing logic introduced in JDK16? 
>> I'm not sure what you are referring to here.
>> 
>>> But it is wierd to me that jmap dump command siliently accept all strings as arguments without any error message.
>> If the target VM cannot accept the argument, our choices are either silently ignore that and retry without the argument (which might mean using a different attach command), or produce an error message. I can see an argument for either approach. I tend to favor the error message (and therefore no retry), but if you have scripts that might target various VM versions, you might want the bad argument to be silently ignored. This is especially true for `parallel` since it doesn't impact the output, but for `gz=`, silently not compressing the output can also lead to issues for any scripts being run, since they might assume it is compressed.
>
> Hi @plummercj, 
> I think maybe you missed my previous reply just before the latest one :-) 
>> The older version for jmap dump only passing arguments that is recognized (file and live or all), and siliently drop the unrecognized arguments without any output.
> I added the logic of print error message in JDK16, with JBS https://bugs.openjdk.java.net/browse/JDK-8251347
> 
> To make it clear,  the jvm (attachListener) just accept what arguments are passed from attach API and parsing the known ones ("file" and "all, live") and ingore the others.
> 
> And the jmap command before JDK16 behaves nearly the same -  accept known arguments and ingore illegal ones. 
> However, in JDK16 , I introduced https://bugs.openjdk.java.net/browse/JDK-8251347, so the jmap command in JDK16 would print error message when unknown argument is parsed. 
> 
> but no matter what version is, the jvm part do the same thing as desribed before.
> 
>> It still not clear to me what is happening when `jmap -dump:gz=...` is used by JDK17 jmap to query and older JDK.
> 
> Let me give more info about this scenario: 
> The JDK17's jmap would then passing gzlevel as the 3rd arguments.  (filename, live or all for the first 2) and then pass it with attach API.  However, the old version JVM's attachlistener only parse the first 2 argument, and hence the gzlevel is ignored. So the jmap dump works but no compressed file generated.
> 
>> but for `gz=`, silently not compressing the output can also lead to issues for any scripts being run, since they might assume it is compressed.
> 
> Yes, I agree with this, because at present there is no info from the result that the generated dump file is gziped or not. And user can usually use ".gz" as suffix in the filename when specify `gz=`, which could be misleading.
> 
> BTW, just want to mention I have an idea for reusing jcmd for jmap -dump:parallel= in previous reply:
> 
>> And I also found in this way the "attachcmd" will be quite similar with jcmd command, it accept different command and pass all argument with one string. Then there maybe another solution -- for "parallel" argument of jmap -dump, just change it to the jcmd command and passing all arguments as a string. Then we don't bother with adding the dumpheapext command.
> 
> Not sure whether it is considerable :-)
> Thanks!

JDK-8251347 does not appear to be the correct bug. Can you find the one you are referring too?

It's too bad that the `dumpheap` command in previous JDKs has a slot for a 3rd argument, but doesn't look at it. Maybe that was intentional to allow for a new argument without causing a failure. What it means is if we want a failure, we need to either send a different command, or use the suggestion of having all the arguments in the first argument, both of which would result in a failure when used with an older JDK (in which case we would retry with older argument passing).

> And I also found in this way the "attachcmd" will be quite similar with jcmd command, it accept different command and pass all argument with one string. Then there maybe another solution -- for "parallel" argument of jmap -dump, just change it to the jcmd command and passing all arguments as a string. Then we don't bother with adding the dumpheapext command.

The irony here is that we've considered dropping `jstack`, `jmap`, and `jinfo` since their functionality is also supported by `jcmd`. But they are far more convenient and easier to understand than `jcmd`, so will probably stay around. But you are right in that they could still all be converted to use `jcmd` internally.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2261