[8u] RFR: 8059038: Create new launcher for SA tools

[Kevin Walls]

Hi Severin,

Looks good to me.

This could have merited a distinct bug ID, I think you could argue 
either way on that, they are clearly strongly related - in 9, this bug 
ID creates a new launcher for all SA tools, here in 8 we're creating the 
missing launchers, within the existing 8u launcher system.

Thanks
Kevin


On 12/12/2018 17:28, Severin Gehwolf wrote:
> On Wed, 2018-12-12 at 16:18 +0000, Andrew Hughes wrote:
>> On Wed, 12 Dec 2018 at 14:10, Severin Gehwolf <sgehwolf at 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
>