On Wed, 10 Feb 2021 02:00:11 GMT, Hamlin Li <[email protected]> wrote:
>>> IMO, there can be two ways which may help solve the compatibility issue of
>>> introducing "parallel" option:
>>>
>>> * unblock socket with timeout. In this way, old Jmap could work with
>>> new JDK, but it maybe complicated because of different implementation for
>>> different OS.
>>>
>>> * Two times communication as you mentioned, it sounds a reasonable
>>> solution, I will work out a prototype and then discuss with you .
>>>
>> A 3rd would be to create a new `dumpheap` command for the Attach API, maybe
>> `dumpheap_parallel`. If you try using it with an old JVM, it should
>> immediately return an error:
>>
>> st.print("Operation %s not recognized!", op->name());
>> res = JNI_ERR;
>> If it does, you can fallback to the regular `dumpheap` command. If an old
>> JVM attaches and uses `dumpheap`, you can choose to make it parallel by
>> default if you wish. A slightly different variant of this idea is have
>> something like `dumpheap_extraargs` or `dumpheap_ext`, leaving room to add
>> even more args in the future (possibly a single argument with a list of args
>> as you suggested before)
>>
>> BTW, one thing to keep in mind that whatever we do with `dumpheap`, the jcmd
>> version will support fine control over parallelism. So for that reason it
>> wouldn't be so bad to just have `dumpheap` and `inspectheap` always use some
>> default parallelism, and allow tuning it with the jcmd versions. But since
>> the `inspectheap` parallel support already went into 16, that might be hard
>> to undo.
>
>> BTW, one thing to keep in mind that whatever we do with `dumpheap`, the jcmd
>> version will support fine control over parallelism. So for that reason it
>> wouldn't be so bad to just have `dumpheap` and `inspectheap` always use some
>> default parallelism, and allow tuning it with the jcmd versions. But since
>> the `inspectheap` parallel support already went into 16, that might be hard
>> to undo.
>
> Hi Chris, Lin,
> IMHO, 16 is not an LTS, maybe it's better for us to adjust the behavior of
> jmap -histo in 17 which is LTS? I just filed
> https://bugs.openjdk.java.net/browse/JDK-8261482 to address the issue if you
> both think it's OK to undo it.
> Thanks.
Hi Chris,
I have update the PR to add a new dumpheapext command that could handle more
arguments passed from JMap.java.
And I have tested with "jmap -dump:file=dump.bin,noparallel [pid]", with this
change, new jmap targeting on old hotspot would print:
Exception in thread "main" com.sun.tools.attach.AttachOperationFailedException:
Operation dumpheapext not recognized!
at
jdk.attach/sun.tools.attach.VirtualMachineImpl.execute(VirtualMachineImpl.java:227)
at
jdk.attach/sun.tools.attach.HotSpotVirtualMachine.executeCommand(HotSpotVirtualMachine.java:309)
at jdk.jcmd/sun.tools.jmap.JMap.executeCommandForPid(JMap.java:133)
at jdk.jcmd/sun.tools.jmap.JMap.dump(JMap.java:262)
at jdk.jcmd/sun.tools.jmap.JMap.main(JMap.java:114)
IMH, one problem is that it doesn't give out the reason clearly that
"dumpheapext" is actually caused by new option "noparallel". But I think having
this kind of error message is acceptable. Otherwise it seems the choice we left
is remove the "parallel/noparallel" for Jmap. What do you think?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2261
