On Thu, 4 Mar 2021 07:26:21 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> If I understanding correctly, we have two users of Tools.java. One is
>> `SALauncher` and the other is `CommandProcessor`. When we use `SALauncher`,
>> we determine the `debugeeType` based on the command line arguments used.
>> When using the `CommandProcessor`, we need access to the `HotSpotAgent` to
>> get the `debuggeeType`, since it has stored this in its `startupMode` field
>> . So the only reason for this new constructor is so we can set
>> `debugeeType`, and then as you've pointed out, that makes the following
>> existing code work correctly:
>>
>> if (getDebugeeType() == DEBUGEE_REMOTE) {
>> out.println("remote configuration is not yet implemented");
>> } else {
>> out.println("not yet implemented (debugger does not support
>> CDebugger)!");
>> }
>
>> I agree with you it is too late to happen.
>> As I commented in [#2773
>> (comment)](https://github.com/openjdk/jdk/pull/2773#issuecomment-790203478)
>> , we can do simplify for them as possible. For example, we can remove
>> `main()` and related methods (e.g. showing usage) from them. If we can
>> remove commandline support from all tool classes, It might help a little to
>> maintain.
>
> Yes, I'd like to see a lot of this excess baggage go away. I also wouldn't
> mind at least considering re-imagining the command line interface for all
> these Tool subclasses, but would only do so if we could quickly deprecate
> what we currently have. We already have enough cases of being able to do the
> same thing more than one way (I think in some cases 5 ways). We don't need
> one more, but on the other hand, as an example, having `jhsdb jmap` support
> dumping the java heap just doesn't make any sense. My guess is the first
> `jmap` just dumped the memory map of loaded libraries, and then more and more
> unrelated subcommands kept being added to it, but I'm not sure of the
> history. The Attach API `jmap` command suffers from the same issue, although
> oddly is does not support the library mapping output like the SA version
> does. I'm not sure which version of `jmap` came first and what it initially
> looked like.
I cannot agree with restructuring sub commands and options in jhsdb for
compatibility.
Since jhsdb is introduced, SA tools (jstack, jmap, and so on) has been removed
the feature to use SA, they are for live process only with Attach API. So I
guess we can ignore about SA interface for them. In addition, they are marked
as unsupported tool in [the
document](https://docs.oracle.com/en/java/javase/15/docs/specs/man/index.html).
Thus I guess we can think about jhsdb and jcmd (dcmd) interface only. If so, I
think we can remove some excess baggages from them (e.g. command line support),
and it is worth to work for maintenance in future.
But this PR does not aim it, it will be the next step.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2766
