PING: RFR: 8209790: SA tools not providing option to connect to debug server
David Holmes
david.holmes at oracle.com
Thu Jul 4 21:44:23 UTC 2019
<trimming>
On 5/07/2019 7:41 am, Chris Plummer wrote:
> On 7/4/19 2:07 PM, David Holmes wrote:
>> I'm not aware of any automatic doc updating process driven by the CSR.
>> If there is a doc task to be done then a specific task needs to be
>> created for it. In this case it is the manpage for jhsdb that needs to
>> be updated, which is now a dev task.
> Ok, I thought I had heard that once. Must have been mistaken.
>
> What about the question of making minor tweaks to the help output so it
> is no longer the same as what is mentioned in the CSR. How does this
> affect the CSR process?
The changes should be finalized before the CSR is finalized and
approved. I think the CSR request would need to be re-opened, updated
and resubmitted.
David
-----
> Chris
>>
>> David
>>
>>>>
>>>> Also CSR of this issue has been approved (JDK-8224979).
>>>> Can we change it?
>>>>
>>> I'm not sure. I think that's a question for Joe Darcy. You would also
>>> need to update it for the suggested help output changes above.
>>>
>>> thanks,
>>>
>>> Chris
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>> thanks,
>>>>>
>>>>> Chris
>>>>>
>>>>> On 7/1/19 3:26 PM, Yasumasa Suenaga wrote:
>>>>>> PING: Could you review it?
>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.03/
>>>>>>
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2019/06/25 16:47, Yasumasa Suenaga wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This enhancement has been retargeted to 14, and the CSR has been
>>>>>>> approved.
>>>>>>> I uploaded a webrev for 14. Could you review it?
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.03/
>>>>>>>
>>>>>>> It includes the fix to rename `--remote` to `--connect` that is
>>>>>>> suggested in CSR.
>>>>>>> It passed tests on submit repo.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> 2019年6月17日(月) 22:11 Yasumasa Suenaga <yasuenag at gmail.com>:
>>>>>>>>
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> On 2019/06/17 21:42, David Holmes wrote:
>>>>>>>>> Hi Yasumasa,
>>>>>>>>>
>>>>>>>>> On 17/06/2019 6:50 pm, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi David,
>>>>>>>>>>
>>>>>>>>>> 8209790 is filed as a bug.
>>>>>>>>>
>>>>>>>>> I don't agree this is a "bug" - sorry. For this to be a bug
>>>>>>>>> there must
>>>>>>>>> be some specification of behaviour that the implementation is
>>>>>>>>> violating.
>>>>>>>>> Is that the case? To me this is missing functionality which
>>>>>>>>> makes it an
>>>>>>>>> enhancement.
>>>>>>>>
>>>>>>>> The feature for connecting to remote debug server has been
>>>>>>>> provided JDK
>>>>>>>> 8 or earlier. However it was missed since JDK 9. So I think we can
>>>>>>>> handle it as a "bug".
>>>>>>>> Anyway, I added jdk13-enhancement-request label to JBS. I'm
>>>>>>>> waiting for
>>>>>>>> the approval.
>>>>>>>>
>>>>>>>>
>>>>>>>>>> According to [1], I think we can push the fix to jdk/jdk13.
>>>>>>>>>> Does it
>>>>>>>>>> correct?
>>>>>>>>>>
>>>>>>>>>> I'm not sure which version (13 or 14) should be set on JBS before
>>>>>>>>>> pushing.
>>>>>>>>>
>>>>>>>>> Just to clarify the process here, you don't want to set the "Fix
>>>>>>>>> Version" to 13 or 14 in JBS before pushing. That will be set
>>>>>>>>> based on
>>>>>>>>> the repo you push to. If you push to jdk/jdk it will be 14. If
>>>>>>>>> you push
>>>>>>>>> to jdk/jdk13 it will be 13. Any fix pushed to jdk/jdk13 will be
>>>>>>>>> "automatically" forward-ported to jdk/jdk and thus 14.
>>>>>>>>
>>>>>>>> Thanks! I got it.
>>>>>>>>
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>>> David
>>>>>>>>> -----
>>>>>>>>>
>>>>>>>>>> (Of course I will push it after the CSR is approved.)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> [1]
>>>>>>>>>> http://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003051.html
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2019/06/17 17:06, David Holmes wrote:
>>>>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>
>>>>>>>>>>> On 17/06/2019 8:52 am, Yasumasa Suenaga wrote:
>>>>>>>>>>>> 2019年6月17日(月) 6:47 serguei.spitsyn at oracle.com
>>>>>>>>>>>> <mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com
>>>>>>>>>>>> <mailto:serguei.spitsyn at oracle.com>>:
>>>>>>>>>>>>
>>>>>>>>>>>> Forgot to tell...
>>>>>>>>>>>> This can be pushed only after the CSR is approved.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Sure!
>>>>>>>>>>>> And I will push it when I get second reviewer.
>>>>>>>>>>>>
>>>>>>>>>>>> BTW should I push this change to jdk/jdk? or push to jdk/jdk13
>>>>>>>>>>>> manually?
>>>>>>>>>>>
>>>>>>>>>>> JDK 13 has now entered RDP1 so if you want this to go into 13
>>>>>>>>>>> it will
>>>>>>>>>>> need special approval:
>>>>>>>>>>>
>>>>>>>>>>> http://openjdk.java.net/jeps/3#Late-Enhancement-Request-Process
>>>>>>>>>>>
>>>>>>>>>>> Otherwise push to jdk/jdk and it will be for JDK 14.
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Serguei
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 6/16/19 14:44, serguei.spitsyn at oracle.com
>>>>>>>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>>>>>>>> > Hi Yasumasa,
>>>>>>>>>>>> >
>>>>>>>>>>>> >
>>>>>>>>>>>> > On 6/16/19 07:22, Yasumasa Suenaga wrote:
>>>>>>>>>>>> >> Hi Serguei,
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> >>> One minor suggestion is to use the final field
>>>>>>>>>>>> NO_REMOTE
>>>>>>>>>>>> instead
>>>>>>>>>>>> >> >>> of null for initialization of the local
>>>>>>>>>>>> variable "remote".
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> I fixed it on new webrev. Could you check again?
>>>>>>>>>>>> >>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.02/
>>>>>>>>>>>> >
>>>>>>>>>>>> >
>>>>>>>>>>>> > It looks good.
>>>>>>>>>>>> > Thanks you for the update!
>>>>>>>>>>>> >
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> >> IMHO refactoring should be worked on another issue.
>>>>>>>>>>>> >> >
>>>>>>>>>>>> >> > Agreed.
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> I filed it to JBS:
>>>>>>>>>>>> >> https://bugs.openjdk.java.net/browse/JDK-8226204
>>>>>>>>>>>> >
>>>>>>>>>>>> > Thank you for filing the enhancement!
>>>>>>>>>>>> >
>>>>>>>>>>>> > Thanks.
>>>>>>>>>>>> > Serguei
>>>>>>>>>>>> >
>>>>>>>>>>>> >
>>>>>>>>>>>> >> Thanks,
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> Yasumasa
>>>>>>>>>>>> >>
>>>>>>>>>>>> >>
>>>>>>>>>>>> >> On 2019/06/15 15:10, serguei.spitsyn at oracle.com
>>>>>>>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>>>>>>>> >>> Hi Yasumasa,
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> On 6/14/19 21:11, Yasumasa Suenaga wrote:
>>>>>>>>>>>> >>>> Hi Serguei,
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>> Thank you for your comment!
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>> On 2019/06/15 8:00, serguei.spitsyn at oracle.com
>>>>>>>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>>>>>>>> >>>>> Hi Yasumasa,
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> I've added myself as a reviewer, so you can
>>>>>>>>>>>> finalize it now.
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>> I moved CSR to Finalized, and added a comment for
>>>>>>>>>>>> your
>>>>>>>>>>>> question.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> Okay, thanks!
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>>> The fix looks pretty good to me.
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> One minor suggestion is to use the final field
>>>>>>>>>>>> NO_REMOTE
>>>>>>>>>>>> instead
>>>>>>>>>>>> >>>>> of null for initialization of the local variable
>>>>>>>>>>>> "remote".
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>> I will fix that.
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>>> Also just an observation that there is some room
>>>>>>>>>>>> for option
>>>>>>>>>>>> >>>>> processing refactoring.
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>> Your suggestion handles all options in one parser
>>>>>>>>>>>> method.
>>>>>>>>>>>> >>>> I concern it might be complex for option validation.
>>>>>>>>>>>> >>>> (e.g. `jmap -heap` is allowed, but `jstack -heap`
>>>>>>>>>>>> is not
>>>>>>>>>>>> allowed.)
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> This concern is not valid as the list allowed
>>>>>>>>>>>> options allowed
>>>>>>>>>>>> for each
>>>>>>>>>>>> >>> jhsdb sub-command is controlled with the longOpts
>>>>>>>>>>>> argument.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> jmap has:
>>>>>>>>>>>> >>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid=",
>>>>>>>>>>>> "remote=",
>>>>>>>>>>>> >>> "heap", "binaryheap", "dumpfile=",
>>>>>>>>>>>> "histo",
>>>>>>>>>>>> >>> "clstats", "finalizerinfo"};
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> but jstack has:
>>>>>>>>>>>> >>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid=",
>>>>>>>>>>>> "remote=",
>>>>>>>>>>>> >>> "mixed", "locks"};
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>> IMHO refactoring should be worked on another issue.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> Agreed.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>> If you are OK in above, I will upload new webrev.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> Yes, I'm Okay with it.
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>> Thanks,
>>>>>>>>>>>> >>> Serguei
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >>>> Thanks,
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>> Yasumasa
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>>
>>>>>>>>>>>> >>>>> All the jhsdb sub-commands do execute similar
>>>>>>>>>>>> loops like
>>>>>>>>>>>> this:
>>>>>>>>>>>> >>>>> while((s = sg.next(null, longOpts)) != null) {
>>>>>>>>>>>> >>>>> . . .
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> It can be moved into a separate method instead.
>>>>>>>>>>>> >>>>> The longOpts can passed in arguments.
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> It can be something like this:
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private ArrayList<String>
>>>>>>>>>>>> processOptions(final String[]
>>>>>>>>>>>> oldArgs,
>>>>>>>>>>>> >>>>> final String[]
>>>>>>>>>>>> >>>>> longOpts,
>>>>>>>>>>>> >>>>> boolean
>>>>>>>>>>>> allowEmpty) {
>>>>>>>>>>>> >>>>> SAGetopt sg = new SAGetopt(oldArgs);
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs = new ArrayList();
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> String pid = null;
>>>>>>>>>>>> >>>>> String exe = null;
>>>>>>>>>>>> >>>>> String core = null;
>>>>>>>>>>>> >>>>> String s = null;
>>>>>>>>>>>> >>>>> String dumpfile = null;
>>>>>>>>>>>> >>>>> String remote = NO_REMOTE;
>>>>>>>>>>>> >>>>> boolean requestHeapdump = false;
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> while((s = sg.next(null, longOpts)) !=
>>>>>>>>>>>> null) {
>>>>>>>>>>>> >>>>> if (s.equals("exe")) {
>>>>>>>>>>>> >>>>> exe = sg.getOptarg();
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("core")) {
>>>>>>>>>>>> >>>>> core = sg.getOptarg();
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("pid")) {
>>>>>>>>>>>> >>>>> pid = sg.getOptarg();
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("remote")) {
>>>>>>>>>>>> >>>>> remote = sg.getOptarg();
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("mixed")) {
>>>>>>>>>>>> >>>>> newArgs.add("-m");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("locks")) {
>>>>>>>>>>>> >>>>> newArgs.add("-l");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("heap")) {
>>>>>>>>>>>> >>>>> newArgs.add("-heap");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("binaryheap")) {
>>>>>>>>>>>> >>>>> requestHeapdump = true;
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("dumpfile")) {
>>>>>>>>>>>> >>>>> dumpfile = sg.getOptarg();
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("histo")) {
>>>>>>>>>>>> >>>>> newArgs.add("-histo");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("clstats")) {
>>>>>>>>>>>> >>>>> newArgs.add("-clstats");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("finalizerinfo")) {
>>>>>>>>>>>> >>>>> newArgs.add("-finalizerinfo");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("flags")) {
>>>>>>>>>>>> >>>>> newArgs.add("-flags");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("sysprops")) {
>>>>>>>>>>>> >>>>> newArgs.add("-sysprops");
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (s.equals("serverid")) {
>>>>>>>>>>>> >>>>> String serverid = sg.getOptarg();
>>>>>>>>>>>> >>>>> if (serverid != null) {
>>>>>>>>>>>> >>>>> newArgs.add(serverid);
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> continue;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> if (!requestHeapdump && (dumpfile !=
>>>>>>>>>>>> null)) {
>>>>>>>>>>>> >>>>> throw new
>>>>>>>>>>>> IllegalArgumentException("Unexpected
>>>>>>>>>>>> >>>>> argument dumpfile");
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> if (requestHeapdump) {
>>>>>>>>>>>> >>>>> if (dumpfile == null) {
>>>>>>>>>>>> >>>>> newArgs.add("-heap:format=b");
>>>>>>>>>>>> >>>>> } else {
>>>>>>>>>>>> >>>>> newArgs.add("-heap:format=b,file=" +
>>>>>>>>>>>> dumpfile);
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>> buildAttachArgs(newArgs, pid, exe, core, remote,
>>>>>>>>>>>> >>>>> allowEmpty);
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> return newArgs;
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private static void runCLHSDB(String[]
>>>>>>>>>>>> oldArgs) {
>>>>>>>>>>>> >>>>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid="};
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>>>>>>>> >>>>> longOpts, true);
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> CLHSDB.main(newArgs.toArray(new
>>>>>>>>>>>> String[newArgs.size()]));
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private static void runHSDB(String[]
>>>>>>>>>>>> oldArgs) {
>>>>>>>>>>>> >>>>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid="};
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs =
>>>>>>>>>>>> processOptions(oldArgs,
>>>>>>>>>>>> >>>>> longOpts, true);
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> HSDB.main(newArgs.toArray(new
>>>>>>>>>>>> String[newArgs.size()]));
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private static void runJSTACK(String[]
>>>>>>>>>>>> oldArgs) {
>>>>>>>>>>>> >>>>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid=",
>>>>>>>>>>>> "remote=",
>>>>>>>>>>>> >>>>> "mixed", "locks"};
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>>>>>>>> >>>>> longOpts, false);
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> JStack jstack = new JStack(false, false);
>>>>>>>>>>>> >>>>> jstack.runWithArgs(newArgs.toArray(new
>>>>>>>>>>>> >>>>> String[newArgs.size()]));
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private static void runJMAP(String[]
>>>>>>>>>>>> oldArgs) {
>>>>>>>>>>>> >>>>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid=",
>>>>>>>>>>>> "remote=",
>>>>>>>>>>>> >>>>> "heap", "binaryheap", "dumpfile=", "histo",
>>>>>>>>>>>> >>>>> "clstats", "finalizerinfo"};
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>>>>>>>> >>>>> longOpts, false);
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> JMap.main(newArgs.toArray(new
>>>>>>>>>>>> String[newArgs.size()]));
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private static void runJINFO(String[]
>>>>>>>>>>>> oldArgs) {
>>>>>>>>>>>> >>>>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid=",
>>>>>>>>>>>> "remote=",
>>>>>>>>>>>> >>>>> "flags", "sysprops"};
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>>>>>>>> >>>>> longOpts, false);
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> JInfo.main(newArgs.toArray(new
>>>>>>>>>>>> String[newArgs.size()]));
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private static void runJSNAP(String[]
>>>>>>>>>>>> oldArgs) {
>>>>>>>>>>>> >>>>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid=",
>>>>>>>>>>>> "remote=",
>>>>>>>>>>>> >>>>> "all"};
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>>>>>>>> >>>>> longOpts, false);
>>>>>>>>>>>> >>>>> JSnap.main(newArgs.toArray(new
>>>>>>>>>>>> String[newArgs.size()]));
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> private static void runDEBUGD(String[]
>>>>>>>>>>>> oldArgs) {
>>>>>>>>>>>> >>>>> // By default SA agent classes prefer
>>>>>>>>>>>> Windows
>>>>>>>>>>>> process
>>>>>>>>>>>> >>>>> debugger
>>>>>>>>>>>> >>>>> // to windbg debugger. SA expects special
>>>>>>>>>>>> properties to
>>>>>>>>>>>> >>>>> be set
>>>>>>>>>>>> >>>>> // to choose other debuggers. We will
>>>>>>>>>>>> set those
>>>>>>>>>>>> here
>>>>>>>>>>>> before
>>>>>>>>>>>> >>>>> // attaching to SA agent.
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> System.setProperty("sun.jvm.hotspot.debugger.useWindbgDebugger",
>>>>>>>>>>>>
>>>>>>>>>>>> >>>>> "true");
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> String[] longOpts = {"exe=", "core=",
>>>>>>>>>>>> "pid=",
>>>>>>>>>>>> "serverid="};
>>>>>>>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>>>>>>>> >>>>> longOpts, false);
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> // delegate to the actual SA debug server.
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> sun.jvm.hotspot.DebugServer.main(newArgs.toArray(new
>>>>>>>>>>>> >>>>> String[newArgs.size()]));
>>>>>>>>>>>> >>>>> }
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> Please, let me know what do you think.
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> Thanks,
>>>>>>>>>>>> >>>>> Serguei
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>>> On 6/9/19 7:29 PM, Yasumasa Suenaga wrote:
>>>>>>>>>>>> >>>>>> Sorry, new webrev is here:
>>>>>>>>>>>> >>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.01/
>>>>>>>>>>>> >>>>>>
>>>>>>>>>>>> >>>>>> Yasumasa
>>>>>>>>>>>> >>>>>>
>>>>>>>>>>>> >>>>>> 2019年6月10日(月) 11:27 Yasumasa
>>>>>>>>>>>> Suenaga<yasuenag at gmail.com
>>>>>>>>>>>> <mailto:yasuenag at gmail.com>>:
>>>>>>>>>>>> >>>>>>> PING: Could you review them?
>>>>>>>>>>>> >>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8209790
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> CSR:https://bugs.openjdk.java.net/browse/JDK-8224979
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> webrev:http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>> It is P3 bug, but I want to fix it before JDK
>>>>>>>>>>>> 13 RDP 1 if
>>>>>>>>>>>> possible.
>>>>>>>>>>>> >>>>>>>
>>>>>>>>>>>> >>>>>>>
>>>>>>>>>>>> >>>>>>> Thanks,
>>>>>>>>>>>> >>>>>>>
>>>>>>>>>>>> >>>>>>> Yasumasa
>>>>>>>>>>>> >>>>>>>
>>>>>>>>>>>> >>>>>>>
>>>>>>>>>>>> >>>>>>> 2019年6月5日(水) 14:06 Yasumasa
>>>>>>>>>>>> Suenaga<yasuenag at gmail.com
>>>>>>>>>>>> <mailto:yasuenag at gmail.com>>:
>>>>>>>>>>>> >>>>>>>> Hi Jc,
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>> Thank you for your comment!
>>>>>>>>>>>> >>>>>>>> I updated a webrev:
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.01/
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>>> - In runTests; if DebugdUtils implemented
>>>>>>>>>>>> Closeable, you
>>>>>>>>>>>> >>>>>>>>> could just do a try-with-resources instead
>>>>>>>>>>>> of the
>>>>>>>>>>>> finally
>>>>>>>>>>>> >>>>>>>>> clause...
>>>>>>>>>>>> >>>>>>>> I created DebugdUtils for convenience class
>>>>>>>>>>>> for attach -
>>>>>>>>>>>> detach
>>>>>>>>>>>> >>>>>>>> mechanism of debug server.
>>>>>>>>>>>> >>>>>>>> IMHO it is prefer "detach" to "close" in this
>>>>>>>>>>>> case.
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>> Thanks,
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>> Yasumasa
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>> 2019年6月5日(水) 11:34 Jean Christophe
>>>>>>>>>>>> Beyler<jcbeyler at google.com <mailto:jcbeyler at google.com>>:
>>>>>>>>>>>> >>>>>>>>
>>>>>>>>>>>> >>>>>>>>> Hi Yasumasa,
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> I'm not an official reviewer but I don't see
>>>>>>>>>>>> an issue
>>>>>>>>>>>> with the
>>>>>>>>>>>> >>>>>>>>> CSR (except that this seems to be bringing a
>>>>>>>>>>>> fork in the
>>>>>>>>>>>> tools
>>>>>>>>>>>> >>>>>>>>> with some handling remote and others not).
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> However, this code is really repetitive and
>>>>>>>>>>>> this is
>>>>>>>>>>>> not the
>>>>>>>>>>>> >>>>>>>>> place to do a big refactor probably but we
>>>>>>>>>>>> could do a
>>>>>>>>>>>> few
>>>>>>>>>>>> nits
>>>>>>>>>>>> >>>>>>>>> perhaps:
>>>>>>>>>>>> >>>>>>>>> - Instead of every tool calling commonHelp
>>>>>>>>>>>> with an
>>>>>>>>>>>> >>>>>>>>> additional flag you could divide into
>>>>>>>>>>>> commonHelp and
>>>>>>>>>>>> >>>>>>>>> commonHelpWithRemote for the tools and they
>>>>>>>>>>>> both call
>>>>>>>>>>>> the
>>>>>>>>>>>> >>>>>>>>> current commonHelp with that boolean; so
>>>>>>>>>>>> that when we
>>>>>>>>>>>> are
>>>>>>>>>>>> >>>>>>>>> looking at the tool code we know what we are
>>>>>>>>>>>> getting... So
>>>>>>>>>>>> >>>>>>>>> something like:
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> private static boolean commonHelp(String
>>>>>>>>>>>> mode, boolean
>>>>>>>>>>>> >>>>>>>>> canConnectToRemote) {
>>>>>>>>>>>> >>>>>>>>> ..
>>>>>>>>>>>> >>>>>>>>> }
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> private static boolean commonHelp(String
>>>>>>>>>>>> mode) {
>>>>>>>>>>>> >>>>>>>>> return commonHelp(mode, false);
>>>>>>>>>>>> >>>>>>>>> }
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> private static boolean
>>>>>>>>>>>> commonHelpWithRemote(String
>>>>>>>>>>>> mode) {
>>>>>>>>>>>> >>>>>>>>> return commonHelp(mode, false);
>>>>>>>>>>>> >>>>>>>>> }
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> and that way the tools that change are just
>>>>>>>>>>>> going from:
>>>>>>>>>>>> >>>>>>>>> - return commonHelp("jmap");
>>>>>>>>>>>> >>>>>>>>> + return commonHelpWithRemote("jmap");
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> - In the same vein, instead of passing null
>>>>>>>>>>>> to the
>>>>>>>>>>>> >>>>>>>>> buildAttachArgs; you could make a variable
>>>>>>>>>>>> null:
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> - buildAttachArgs(newArgs, pid, exe, core,
>>>>>>>>>>>> true);
>>>>>>>>>>>> >>>>>>>>> + String noRemote = null;
>>>>>>>>>>>> >>>>>>>>> + buildAttachArgs(newArgs, pid, exe, core,
>>>>>>>>>>>> noRemote,
>>>>>>>>>>>> >>>>>>>>> true);
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> -http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/test/hotspot/jtreg/serviceability/sa/sadebugd/DebugdUtils.java.html
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> Nit: you have empty lines at l64 and l73
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> -http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/test/hotspot/jtreg/serviceability/sa/sadebugd/DebugdConnectTest.java.html
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> �� >>>>>>>>> Nit : you have an empty line at l110
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> - In runTests; if DebugdUtils implemented
>>>>>>>>>>>> Closeable, you
>>>>>>>>>>>> >>>>>>>>> could just do a try-with-resources instead
>>>>>>>>>>>> of the
>>>>>>>>>>>> finally
>>>>>>>>>>>> >>>>>>>>> clause...
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> All of these are details, I just thought I'd
>>>>>>>>>>>> mention
>>>>>>>>>>>> them :)
>>>>>>>>>>>> >>>>>>>>> Jc
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> On Tue, Jun 4, 2019 at 6:44 PM Yasumasa
>>>>>>>>>>>> >>>>>>>>> Suenaga<yasuenag at gmail.com
>>>>>>>>>>>> <mailto:yasuenag at gmail.com>> wrote:
>>>>>>>>>>>> >>>>>>>>>> PING: Could you review them?
>>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8209790
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> CSR:https://bugs.openjdk.java.net/browse/JDK-8224979
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> webrev:http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>> CSR status is provisional. So I need
>>>>>>>>>>>> reviewers both
>>>>>>>>>>>> CSR and
>>>>>>>>>>>> >>>>>>>>>> webrev.
>>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>> Thanks,
>>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>> Yasumasa
>>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>> 2019年5月29日(水) 22:37 Yasumasa
>>>>>>>>>>>> Suenaga<yasuenag at gmail.com <mailto:yasuenag at gmail.com>>:
>>>>>>>>>>>> >>>>>>>>>>> Hi all,
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>> Please review this change:
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8209790
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> CSR:https://bugs.openjdk.java.net/browse/JDK-8224979
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> webrev:http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>> In JDK 8 or earlier, some tools (jstack, jmap,
>>>>>>>>>>>> jinfo) can
>>>>>>>>>>>> >>>>>>>>>>> connect to
>>>>>>>>>>>> >>>>>>>>>>> debug server (jsadebugd). However it has not done
>>>>>>>>>>>> so since
>>>>>>>>>>>> >>>>>>>>>>> JDK 9 because
>>>>>>>>>>>> >>>>>>>>>>> jhsdb cannot accept the attach request to debug
>>>>>>>>>>>> server.
>>>>>>>>>>>> >>>>>>>>>>> So I want to introduce new option `--remote` to
>>>>>>>>>>>> connect to
>>>>>>>>>>>> >>>>>>>>>>> debug server.
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>> I created CSR for this issue. So please review it
>>>>>>>>>>>> together.
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>> Thanks,
>>>>>>>>>>>> >>>>>>>>>>>
>>>>>>>>>>>> >>>>>>>>>>> Yasumasa
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> --
>>>>>>>>>>>> >>>>>>>>>
>>>>>>>>>>>> >>>>>>>>> Thanks,
>>>>>>>>>>>> >>>>>>>>> Jc
>>>>>>>>>>>> >>>>>
>>>>>>>>>>>> >>>
>>>>>>>>>>>> >
>>>>>>>>>>>>
>>>>>
>>>
>
More information about the serviceability-dev
mailing list