On Sun, 28 Feb 2021 13:18:21 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:

> `pmap` and `pstack` CLHSDB command do not work on remote debugger, we can see 
> following error message:
> 
> hsdb> pmap
> not yet implemented (debugger does not support CDebugger)!
> 
> However, SA has different message for this purpose:
> 
> https://github.com/openjdk/jdk/blob/03d888f463c0a6e3fee70ed8ad606fc0a3082636/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PMap.java#L74-L78
> 
> SA should show "remote configuration is not yet implemented" when it works on 
> remote debugger.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/Tool.java line 69:

> 67:       }
> 68:    }
> 69: 

This section seems to be the key to your changes, but I have to admit that I 
don't follow how it all ties in together. Maybe you can explain the overall 
structure of the Tool subclasses to help explain. They seem to be overloaded 
with modes that make it hard to follow all the logic. For example, you've added 
the `Tool(HotSpotAgent agent)` constructor, and we already have `Tool()` and 
`Tool(JVMDebugger d)`. It's unclear to me what the use case is for each. Also, 
we have about 10 subclasses of Tool. Why do only `PMap` an `PStack` need this 
new constructor.

BTW, looking at Tool and its subclasses has made me realize we have a hole 
other area of historical tool baggage, similar to what we are discussing with 
CLHSDB and HSDB. My guess is all these tool subclasses used to be invoked 
directly from the java command line. Now they are all hidden behind `jhsdb` 
subcommands, but still carry the old java command line support. In addition, 
when adding `jhsdb` and it's subcommands, it looks like there was an effort to 
mimic the existing Attach API commands like jmap and jinfo, so the SA versions 
of these commands have multiple modes which invoke different Tool subclasses. 
For example, `jhdsb jmap` alone can invoke the ClassLoaderStats, FinalizerInfo, 
ObjectHistogram, PMap, and HeapSummary tools, in addition to dumping the heap 
graph in HProf or GXL format. I think things would have been simpler and 
cleaner if each of these were made separate `jhsdb` subcommands, but it's 
probably too late for that. I suppose we could deprecate the existing sub-com
 mands and add all these new  sub-commands, but that will just make things 
harder for us until the old commands are eventually removed, which might never 
happen.

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

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

Reply via email to