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

[Lin Zang]

On Wed, 24 Feb 2021 04:12:13 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>>> 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).
>> 
>> Yes, I believe the argument max set to 3 was based on assumption that there can be at most 3 arguments from client side, no matter which tool it is (jmap, jstack, jinfo). 
>> 
>> And the jcmd command actually already passed all arguemnt as a single string, so there is no issue for accepting more arguments.
>>  
>>> 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.
>> 
>> Maybe we could consider using `jcmd` internally to replace the current implementation of these tools, and keep their usage as before, then we get same result from `jcmd` and `jmap` alike tools, get rid of the argument passing limitation (because jcmd make all argument as a string), and also reduce the overhead of maintanence (e.g. don't bothering adding new options for both jmap and jcmd )
>> 
>>> Ok, but that fix is on the client side, not the target VM side.
>> 
>> Yes, it only change the client side to reject illegal arguments, the JVM side still work as it is -  ingore unknown argument silicently. However, the client side could at least reject unknown option and hence pass correct ones as expected to JVM.
>> So if we agree on this change, at least for "parallel=" option, we don't need to retry when get the exception. :-)
>
>> > 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.
>> 
>> Maybe we could consider using `jcmd` internally to replace the current implementation of these tools, and keep their usage as before, then we get same result from `jcmd` and `jmap` alike tools, get rid of the argument passing limitation (because jcmd make all argument as a string), and also reduce the overhead of maintanence (e.g. don't bothering adding new options for both jmap and jcmd )
> 
> It does reduce the maintenance overhead, but you still need to parse the arguments on the client side and package them up the way jcmd likes to see them.
> 
>> 
>> > Ok, but that fix is on the client side, not the target VM side.
>> 
>> Yes, it only change the client side to reject illegal arguments, the JVM side still work as it is - ingore unknown argument silicently. However, the client side could at least reject unknown option and hence pass correct ones as expected to JVM.
>> So if we agree on this change, at least for "parallel=" option, we don't need to retry when get the exception. :-)
> 
> Actually you should still see an error when using `parallel` or `gz` when the target VM doesn't understand them. This is because `jcmd` will reject them on the target VM side. Then the question is whether to retry without them, or pass the error to the user and let them adjust their `jmap` command. The problem with the latter is that the arguments in the `jcmd` error message might not look exactly the same as the way they were specified by the user to `jmap`. We'd have take a closer look at the `jcmd` error messages to see if they are appropriate for `jmap` users.

Hi @plummercj, 
I agree, the output data must already be used by some parsing tools I believe. So the output format must keep the same no matter what change we want to make.
I will mark CSR to this PR as we agreed the on introducing the `parallel=` option.  

Thanks

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

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