On Wed, 5 Feb 2025 20:11:53 GMT, Alex Menkov <[email protected]> wrote:
>> The fix implements streaming output support for attach protocol.
>> See JBS issue for evaluation, summary of the changes in the 1st comment.
>> Testing: tier1..4,hs-tier5-svc
>
> Alex Menkov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> feedback
test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java
line 70:
> 68:
> 69: private static void attach(LingeredApp app, boolean clientStreaming,
> boolean vmStreaming) throws Exception {
> 70: HotSpotVirtualMachine vm =
> (HotSpotVirtualMachine)VirtualMachine.attach(String.valueOf(app.getPid()));
Q: It is confusing that both boolean arguments are not used. Would you like to
get rid of them?
test/hotspot/jtreg/serviceability/attach/AttachAPIv2/StreamingOutputTest.java
line 86:
> 84:
> 85: private static void verify(boolean clientStreaming, boolean
> vmStreaming, String out) throws Exception {
> 86: System.out.println("Target VM output:");
Q: It is confusing that the boolean argument vmStreaming is not used. Would you
like to get rid of it?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945156555
PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945158080
