On Tue, 2 Mar 2021 21:48:30 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Yasumasa Suenaga has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix comments
>
> src/jdk.hotspot.agent/doc/clhsdb.html line 39:
>
>> 37: Available commands:
>> 38: assert true | false <font color="red">turn on/off asserts in SA
>> code</font>
>> 39: attach pid | exec core | remote_server <font color="red">attach SA to
>> a process, core, or remote debug server</font>
>
> The `jhsdb` help says `jhsdb jstack --connect debugserver`, so maybe
> `debug_server` would be better than `remote_server`. I think "remote debug
> server" in the help text is fine.
Fixed it in new commit.
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CLHSDB.java line 53:
>
>> 51: // and coreFilename is the pathname of a core file we are
>> 52: // supposed to attach to.
>> 53: // Finally, if remoteMachineName != null, we are supposed to
>> connect to remote debug server.
>
> Split this line so its length is more consistent with the previous lines.
Fixed it in new commit.
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CLHSDB.java line 125:
>
>> 123: private String execPath;
>> 124: private String coreFilename;
>> 125: private String remoteMachineName;
>
> I think `debugServerName` or `remoteDebugServerName` would be better.
Rename to `debugServerName` in new commit.
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CLHSDB.java line 230:
>
>> 228: System.out.println("Connecting to debug server, please
>> wait...");
>> 229: agent.attach(remoteMachineName);
>> 230: this.remoteMachineName = remoteMachineName;
>
> I'm a little surprised to see that there is already some support for
> connecting to a debug server in CLHSDB.java. Was it previously used from
> anywhere?
I cannot find out caller of `CLHSDB::connect`. I guess CLHSDB.java is copied
from HSDB.java because they are similar - HSDB uses `connect()` to connect to
debug server.
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/HSDB.java line 84:
>
>> 82: private String execPath;
>> 83: private String coreFilename;
>> 84: private String remoteMachineName;
>
> `debugServerName` or `remoteDebugServerName`
Rename to `debugServerName` in new commit.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2773
