Thanks, Serguei! Yasumasa
2019年6月25日(火) 17:35 serguei.spit...@oracle.com <serguei.spit...@oracle.com>: > Hi Yasumasa, > > The fix looks good to me. > > Thanks, > Serguei > > > On 6/25/19 00: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 <yasue...@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.spit...@oracle.com > >>>>>> <mailto:serguei.spit...@oracle.com> <serguei.spit...@oracle.com > >>>>>> <mailto:serguei.spit...@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.spit...@oracle.com > >>>>>> <mailto:serguei.spit...@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.spit...@oracle.com > >>>>>> <mailto:serguei.spit...@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.spit...@oracle.com > >>>>>> <mailto:serguei.spit...@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< > yasue...@gmail.com > >>>>>> <mailto:yasue...@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< > yasue...@gmail.com > >>>>>> <mailto:yasue...@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<jcbey...@google.com <mailto:jcbey...@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<yasue...@gmail.com > >>>>>> <mailto:yasue...@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<yasue...@gmail.com <mailto:yasue...@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 > >>>>>> >>>>> > >>>>>> >>> > >>>>>> > > >>>>>> > >
