On Wed, 24 Feb 2021 01:08:58 GMT, Lin Zang <lz...@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
