On Wed, 2018-12-12 at 16:18 +0000, Andrew Hughes wrote: > On Wed, 12 Dec 2018 at 14:10, Severin Gehwolf <sgehw...@redhat.com> wrote: > > > > Hi, > > > > Can I get a review of this small 8u enhancement, please? It adds two > > new launchers for the serviceability agent, one CLI version and one GUI > > version: > > > > $ <image>/bin/clhsdb > > $ <image>/bin/hsdb > > > > The enhancement request has been approved here: > > http://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-December/008257.html > > > > During that discussion it has been suggested to use separate launchers > > for GUI and CLI. So this is the revised two-launcher-approach: > > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8059038/02/ > > bug: https://bugs.openjdk.java.net/browse/JDK-8059038 > > > > Note: The initial version of this patch[1] had one launcher "jhsdb" > > with sub-commands "hsdb" and "clhsdb" delegating to the old launcher > > classes. > > > > The patch has two simple tests verifying that the launchers work. > > Thoughts? > > > > Thanks, > > Severin > > > > [1] http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8059038/01/ > > > > The patch itself looks ok. But it looks completely different from the > changesets in 9: > > https://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/bf17c0a1c746 > https://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/24a8cbde76d8 > > Can you explain a little what's going on here?
Sure. The original patch for 9 added a new class, SALauncher, which launches many of the original stand-alone commands from 8. jstack from JDK 8, for example, became "jhsdb jstack" in 9+ via this class[I]. On the JDK 8 enhancement request approval thread a suggestion was made for the backport to JDK 8 to mimic the stand-alone launcher approach JDK 8 uses: "jstack" over "jhsdb jstack". So "clhsdb" (JDK 8) will be a separate launcher over "jhsdb clhsdb" in JDK 9+. The first webrev[1] above, mimic'ed JDK 9+, but not quite since it didn't change the arguments and options. jhsdb clhsdb --pid <pid> (JDK 9+) vs. jhsdb clhsdb <pid> (in [1]). That's basically the reason why the JDK 8 backport looks this different. Of course we can also go with the original version[1], but so far I've heard two +1's for the second version... Hence, I'm proposing that version. Thanks, Severin [I] https://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/24a8cbde76d8#l2.213
