On Fri, 12 Feb 2021 06:25:41 GMT, Chris Plummer <[email protected]> wrote:
>> Hi Chris, >> >>> There is still one minor issue with this `noparallel` support, and that is >>> we still run into the problem of `jmap -dump` not being able to pass a 4th >>> argument via the attach API. We could just say that's fine, and require >>> using the `jcmd` to do the heap dump if the user wants to override the >>> default of having parallel enabled, but then it seems we should do the same >>> for `jmap -histo` to be consistent. So in other words, no support in `jmap` >>> for disabling parallel support, but we do have the support in `jcmd`. >>> Thoughts? >> >> I just updated the #2261 which introduce a new command "dumpheapext" that >> could handle more arguments (as you suggested the idea :-> ). I think maybe >> that kind of change is acceptable. Moreover, this way also allow us to add >> more options in future. >> >> So maybe we have two choice here: >> - Add a new command for argument extension. so jcmd and jmap could be >> consisitent. >> - Remove the "parallel" option in jmap, and leave the control to jcmd. >> >> I prefer the first one as it may be more clearer for users to have >> consisitent usage of jcmd and jmap tools as they were before. >> >> Thanks! >> Lin > >> So maybe we have two choice here: >> * Add a new command for argument extension. so jcmd and jmap could be >> consisitent. >> * Remove the "parallel" option in jmap, and leave the control to jcmd. > > Yes, I suppose we can still do the `dumpheapext` approach to allow for having > 4 args. It's a bit of a hack, but at least it is hidden from the user, and > allows `jmap` and `jcmd` to both support `noparallel` Sigh, I've keep debating this with myself, and keep having second thoughts on what is best. I'm now leaning towards just sticking with `parallel=<n>`, and for any n > 1, at most we just say the user will need to experiment to see what works best (if we even say that). This way we avoid any headaches with the the JDK 16 support for `parallel=<n>` with `jmap -histo`. I know this isn't ideal, but I'm guessing for the most part users will just leave this option alone anyway, and will get the default number of threads. But, my current feelings here also hinge on adding `dumpheapext` so `-dump` and `-histo` both support parallel options. ------------- PR: https://git.openjdk.java.net/jdk/pull/2379
