PING: RFR: 8209790: SA tools not providing option to connect to debug server
Yasumasa Suenaga
yasuenag at gmail.com
Wed Jul 10 01:20:06 UTC 2019
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>:
> 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 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 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
> >>>>>>>>>>>>>> |
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190710/7aeba106/attachment-0001.html>
More information about the serviceability-dev
mailing list