PING: RFR: 8209790: SA tools not providing option to connect to debug server

Chris Plummer chris.plummer at oracle.com
Fri Jul 5 21:15:40 UTC 2019


On 7/4/19 3:55 PM, Yasumasa Suenaga wrote:
> On 2019/07/05 6:44, David Holmes wrote:
>> <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.
>
> Ok, I will do that when the webrev is reviewed.
> I will upload new one after the question of me is cleared.
Which question are you referring to?

Chris
>
>
> Yasumasa
>
>
>> 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