PING: RFR: 8209790: SA tools not providing option to connect to debug server
Yasumasa Suenaga
yasuenag at gmail.com
Wed Jul 10 23:15:17 UTC 2019
Thanks Serguei!
Yasumasa
On 2019/07/11 2:55, serguei.spitsyn at oracle.com wrote:
> Hi Yasumasa,
>
> Thank you for adding your comment to the CSR!
> It looks good to me.
>
> Thanks,
> Serguei
>
>
> On 7/9/19 18:20, Yasumasa Suenaga wrote:
>> Hi Serguei,
>>
>> I added a comment that the change from the latest (approved) proposal to the CSR. Could you check it again?
>>
>> Please tell me if it is not enough.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>
>> 2019年7月10日(水) 8:12 <serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com>>:
>>
>> Hi Yasumasa,
>>
>> I'd like you to be more precise.
>> Providing the webrev, email exchange and saying about help messages is
>> not precise.
>> It requires some non-trivial extra work.
>> Could you add a comment with this:
>> What help message has been changed to what?
>>
>> Thanks,
>> Serguei
>>
>> On 7/9/19 3:51 PM, Yasumasa Suenaga wrote:
>> > Hi Serguei,
>> >
>> > I added a comment to the CSR.
>> > I've updated help messages in Specification section of the CSR.
>> >
>> >
>> > Thanks,
>> >
>> > Yasumasa
>> >
>> >
>> > On 2019/07/10 7:42, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> wrote:
>> >> Hi Yasumasa,
>> >>
>> >> Could you be more precise?
>> >> What was the latest change in this CSR?
>> >> Could you add it as a comment?
>> >> It will safe time for reviewers and approvers.
>> >>
>> >> Thanks,
>> >> Serguei
>> >>
>> >> On 7/9/19 3:35 PM, Yasumasa Suenaga wrote:
>> >>> Thanks Serguei!
>> >>>
>> >>> I reopened the CSR and update the description.
>> >>> Could you review again it?
>> >>>
>> >>> https://bugs.openjdk.java.net/browse/JDK-8224979
>> >>>
>> >>>
>> >>> Thanks,
>> >>>
>> >>> Yasumasa
>> >>>
>> >>>
>> >>> On 2019/07/10 4:11, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> wrote:
>> >>>> Hi Yasumasa,
>> >>>>
>> >>>> The update looks Okay to me.
>> >>>>
>> >>>> Thanks,
>> >>>> Serguei
>> >>>>
>> >>>>
>> >>>> On 7/8/19 4:16 PM, Yasumasa Suenaga wrote:
>> >>>>> Thank you Chris!
>> >>>>>
>> >>>>> Serguei, do you agree with this change?
>> >>>>>
>> >>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.07/
>> >>>>>
>> >>>>> If you are ok, I will reopen the CSR (JDK-8224979).
>> >>>>>
>> >>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-July/028624.html
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> Thanks,
>> >>>>>
>> >>>>> Yasumasa
>> >>>>>
>> >>>>>
>> >>>>> On 2019/07/09 8:08, Chris Plummer wrote:
>> >>>>>> Looks good! Sorry about the high number of iterations. I should
>> >>>>>> have caught some things sooner.
>> >>>>>>
>> >>>>>> thanks,
>> >>>>>>
>> >>>>>> Chris
>> >>>>>>
>> >>>>>> On 7/8/19 3:43 PM, Yasumasa Suenaga wrote:
>> >>>>>>> Hi Chris,
>> >>>>>>>
>> >>>>>>> Thank you for your comment.
>> >>>>>>> I fixed it in new webrev:
>> >>>>>>>
>> >>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.07/
>> >>>>>>>
>> >>>>>>> Could you check again?
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> Yasumasa
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> On 2019/07/09 4:11, Chris Plummer wrote:
>> >>>>>>>> Hi Yasumasa,
>> >>>>>>>>
>> >>>>>>>> Sorry, just noticed one other thing:
>> >>>>>>>>
>> >>>>>>>>> <--core --exe> and <--pid> and <--connect> are mutually
>> >>>>>>>>> exclusive.
>> >>>>>>>> I'm not sure why you are using angled brackets here. It's not
>> >>>>>>>> done in other parts of the help output that reference options
>> >>>>>>>> by name. Also, no need to mention --exe since there is an
>> >>>>>>>> earlier comment in the help saying in must be used together
>> >>>>>>>> with --core. This should help to simplify the help message. I
>> >>>>>>>> would suggest:
>> >>>>>>>>
>> >>>>>>>> --core, --pid, and --connect are mutually exclusive.
>> >>>>>>>>
>> >>>>>>>> There's also a typo in the version of the help that does not
>> >>>>>>>> include --connect:
>> >>>>>>>>
>> >>>>>>>> 76 System.out.println(" <--core --exe> and <--pid> are
>> >>>>>>>> mutually eexclusive.");
>> >>>>>>>>
>> >>>>>>>> Notice "eexclusive".
>> >>>>>>>>
>> >>>>>>>> Otherwise it looks good.
>> >>>>>>>>
>> >>>>>>>> thanks,
>> >>>>>>>>
>> >>>>>>>> Chris
>> >>>>>>>>
>> >>>>>>>> On 7/8/19 5:06 AM, Yasumasa Suenaga wrote:
>> >>>>>>>>> Hi Chris,
>> >>>>>>>>>
>> >>>>>>>>> I uploaded new webrev:
>> >>>>>>>>>
>> >>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.06/
>> >>>>>>>>>
>> >>>>>>>>> It shows help messages as below:
>> >>>>>>>>>
>> >>>>>>>>> ```
>> >>>>>>>>> $ jhsdb jstack --help
>> >>>>>>>>> --locks To print java.util.concurrent locks.
>> >>>>>>>>> --mixed To print both Java and native
>> >>>>>>>>> frames (mixed mode).
>> >>>>>>>>> --pid <pid> To attach to and operate on the
>> >>>>>>>>> given live process.
>> >>>>>>>>> --core <corefile> To operate on the given core file.
>> >>>>>>>>> --exe <executable for corefile>
>> >>>>>>>>> --connect [<id>@]<host> To connect to a remote debug
>> >>>>>>>>> server (debugd).
>> >>>>>>>>>
>> >>>>>>>>> The --core and --exe options must be set together to give
>> >>>>>>>>> the core
>> >>>>>>>>> file, and associated executable, to operate on. They can use
>> >>>>>>>>> absolute or relative paths.
>> >>>>>>>>> The --pid option can be set to operate on a live process.
>> >>>>>>>>> The --connect option can be set to connect to a debug
>> >>>>>>>>> server (debugd).
>> >>>>>>>>> <--core --exe> and <--pid> and <--connect> are mutually
>> >>>>>>>>> exclusive.
>> >>>>>>>>>
>> >>>>>>>>> Examples: jhsdb jstack --pid 1234
>> >>>>>>>>> or jhsdb jstack --core ./core.1234 --exe ./myexe
>> >>>>>>>>> or jhsdb jstack --connect debugserver
>> >>>>>>>>> or jhsdb jstack --connect id at debugserver
>> >>>>>>>>> ```
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> Thanks,
>> >>>>>>>>>
>> >>>>>>>>> Yasumasa
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> On 2019/07/08 16:14, Chris Plummer wrote:
>> >>>>>>>>>> Sorry, I meant "jhsdb debugd", not "clhsdb debugd".
>> >>>>>>>>>>
>> >>>>>>>>>> Chris
>> >>>>>>>>>>
>> >>>>>>>>>> On 7/8/19 12:09 AM, Chris Plummer wrote:
>> >>>>>>>>>>> Hi Yasumasa,
>> >>>>>>>>>>>
>> >>>>>>>>>>> I just noticed one more thing:
>> >>>>>>>>>>>
>> >>>>>>>>>>> 65 System.out.println(" --connect <[id@]host>
>> >>>>>>>>>>> To connect to a remote debug server (sadebugd).");
>> >>>>>>>>>>>
>> >>>>>>>>>>> Earlier on I incorrectly said that jstack and other tools
>> >>>>>>>>>>> had a references to "sadebugd" in the help output. It is
>> >>>>>>>>>>> actually "jsadebugd". But that was the name of the command
>> >>>>>>>>>>> line tool, which is now gone. Instead now we have "clhsdb
>> >>>>>>>>>>> debugd ...", so maybe it should just mention "debugd".
>> >>>>>>>>>>>
>> >>>>>>>>>>> thanks,
>> >>>>>>>>>>>
>> >>>>>>>>>>> Chris
>> >>>>>>>>>>>
>> >>>>>>>>>>> On 7/7/19 10:24 PM, Chris Plummer wrote:
>> >>>>>>>>>>>> Hi Yasumasa,
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> I think it should be "[<id>@]<host>". Angle brackets are
>> >>>>>>>>>>>> used to indicate non-literal values (ones to be replaced by
>> >>>>>>>>>>>> something the user chooses), but the '@' is a literal
>> >>>>>>>>>>>> value, so should not be enclosed in the angle brackets.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Also one minor thing. You ended up duplicating the //:
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> 60 // // --connect <[id@]host>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> thanks,
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Chris
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> On 7/6/19 5:55 PM, Yasumasa Suenaga wrote:
>> >>>>>>>>>>>>> Hi Chris,
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> Thank you for your advise.
>> >>>>>>>>>>>>> I uploaded new webrev. How about this?
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.04/
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> I replaced "server" to "host" as you said.
>> >>>>>>>>>>>>> Other comments from you also have been fixed in it.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> For example, `jhsdb jstack --help` shows as below:
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> ```
>> >>>>>>>>>>>>> $ jhsdb jstack --help
>> >>>>>>>>>>>>> --locks To print java.util.concurrent
>> >>>>>>>>>>>>> locks.
>> >>>>>>>>>>>>> --mixed To print both Java and native
>> >>>>>>>>>>>>> frames (mixed mode).
>> >>>>>>>>>>>>> --pid <pid> To attach to and operate on
>> >>>>>>>>>>>>> the given live process.
>> >>>>>>>>>>>>> --core <corefile> To operate on the given core
>> >>>>>>>>>>>>> file.
>> >>>>>>>>>>>>> --exe <executable for corefile>
>> >>>>>>>>>>>>> --connect <[id@]host> To connect to a remote debug
>> >>>>>>>>>>>>> server (sadebugd).
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> The --core and --exe options must be set together to
>> >>>>>>>>>>>>> give the core
>> >>>>>>>>>>>>> file, and associated executable, to operate on. They
>> >>>>>>>>>>>>> can use
>> >>>>>>>>>>>>> absolute or relative paths.
>> >>>>>>>>>>>>> The --pid option can be set to operate on a live process.
>> >>>>>>>>>>>>> The --connect option can be set to connect to a debug
>> >>>>>>>>>>>>> server (sadebugd).
>> >>>>>>>>>>>>> <--core --exe> and <--pid> and <--connect> are exclusive.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> Examples: jhsdb jstack --pid 1234
>> >>>>>>>>>>>>> or jhsdb jstack --core ./core.1234 --exe ./myexe
>> >>>>>>>>>>>>> or jhsdb jstack --connect debugserver
>> >>>>>>>>>>>>> or jhsdb jstack --connect id at debugserver
>> >>>>>>>>>>>>> ```
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> If you are ok, I will reopen CSR (JDK-8224979).
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> Yasumasa
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> On 2019/07/06 6:26, Chris Plummer wrote:
>> >>>>>>>>>>>>>> On 7/4/19 3:53 PM, Yasumasa Suenaga wrote:
>> >>>>>>>>>>>>>>> Hi Chris,
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> On 2019/07/05 4:24, Chris Plummer wrote:
>> >>>>>>>>>>>>>>>> Hi Yasumasa,
>> >>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>> On 7/4/19 5:30 AM, Yasumasa Suenaga wrote:
>> >>>>>>>>>>>>>>>>> Hi Chris,
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>> Thank you for your review.
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>> On 2019/07/04 8:07, Chris Plummer wrote:
>> >>>>>>>>>>>>>>>>>> Hi Yasumasa,
>> >>>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> Overall these changes look good, but I think there is
>> >>>>>>>>>>>>>>>>>> a bit of cleanup still needed.
>> >>>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> You've changed the indentation of the help to always
>> >>>>>>>>>>>>>>>>>> have a few spaces before the /t. If you are going to
>> >>>>>>>>>>>>>>>>>> do this, then perhaps you don't need the /t anymore.
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>> Ok, I will replace '\t' to whitespace.
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> There are 3 checks for "remote != null". Shouldn't
>> >>>>>>>>>>>>>>>>>> they be "remote != NO_REMOTE"?
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>> I will fix them.
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> The old jstack help output is a bit more informative
>> >>>>>>>>>>>>>>>>>> than your output:
>> >>>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> jstack [-m] [-l] [server_id@]<remote server IP
>> >>>>>>>>>>>>>>>>>> or hostname>
>> >>>>>>>>>>>>>>>>>> (to connect to a remote debug server)
>> >>>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> vs
>> >>>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> --connect <[id@]server> To operate on the remote
>> >>>>>>>>>>>>>>>>>> debug server.
>> >>>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>>> More specifically, you replaced <remote server IP or
>> >>>>>>>>>>>>>>>>>> hostname> with "server". I think the former is a bit
>> >>>>>>>>>>>>>>>>>> more self explanatory.
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>> Exactly, but it is too long to show on the console.
>> >>>>>>>>>>>>>>>>> So I used short word: server.
>> >>>>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>>> If we can ignore long line, or can add newline (\n) on
>> >>>>>>>>>>>>>>>>> help message,
>> >>>>>>>>>>>>>>>>> I can change it. What do you think?
>> >>>>>>>>>>>>>>>> I don't see why not. Isn't there already help output
>> >>>>>>>>>>>>>>>> that relies on newlines?
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> As you can see on the CSR (JDK-8224979), format of jhsdb
>> >>>>>>>>>>>>>>> help messages is:
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> <option name> <blank (includes tab)> <description>
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> Thus I think long option / description is not comfortable.
>> >>>>>>>>>>>>>>> If we need to describe more, I think we should write it
>> >>>>>>>>>>>>>>> as below:
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> --connect <[server-id@]remote server IP or hostname>
>> >>>>>>>>>>>>>>> To connect to a remote debug server.
>> >>>>>>>>>>>>>> Hi Yasumasa,
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> We already have multiline examples like:
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> |--pid <pid> To attach to and operate on the given live
>> >>>>>>>>>>>>>> process.
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> Is your concerned that the long option name will extend
>> >>>>>>>>>>>>>> into the columns where the description normally is? If
>> >>>>>>>>>>>>>> so, you can start the description on a new line as you
>> >>>>>>>>>>>>>> suggest, or maybe come up with a different name for the
>> >>>>>>>>>>>>>> option that isn't quite as long. My main objection to
>> >>>>>>>>>>>>>> just saying "server" is it in no way conveys we are
>> >>>>>>>>>>>>>> talking about an IP host. Even just saying <host> would
>> >>>>>>>>>>>>>> be better. It is also more correct since the host isn't
>> >>>>>>>>>>>>>> necessarily what someone would consider to be a server
>> >>>>>>>>>>>>>> (it could just be someone's desktop). <server-id> on the
>> >>>>>>>>>>>>>> other hand is the correct term because we are talking
>> >>>>>>>>>>>>>> about the id of the debug server running on a particular
>> >>>>>>>>>>>>>> host.
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> Chris
>> >>>>>>>>>>>>>> |
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>
>> >>
>>
>
More information about the serviceability-dev
mailing list