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