RFR: 8319055: JCMD should not buffer the whole output of commands [v3]

[Alex Menkov]

On Tue, 4 Feb 2025 10:10:41 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   feedback - test
>
> src/hotspot/share/services/attachListener.cpp line 53:
> 
>> 51: 
>> 52: // Stream for printing attach operation results.
>> 53: // Supports buffered and streaming output for commands which can produce lenghtly reply.
> 
> "lengthy"

Fixed

> src/hotspot/share/services/attachListener.cpp line 56:
> 
>> 54: //
>> 55: // To support streaming output platform implementation need to implement AttachOperation::get_reply_writer() method
>> 56: // and ctor allow_streaming argument should be set to true.
> 
> Strange mix of "need" and "should".
> 
> Can we split this into multiple sentences? Here's a suggestion, though I'm not sure if it's 100% correct :-).
> 
> 
> An output platform implementation supports streaming if it implements AttachOperation::get_reply_writer().
> Streaming is enabled if the allow_streaming in the constructor is set to true.

Updated the comment

> src/hotspot/share/services/diagnosticFramework.cpp line 389:
> 
>> 387:   int count = 0;
>> 388:   while (iter.has_next()) {
>> 389:     if(_source == DCmd_Source_MBean && count > 0) {
> 
> Pre-existing style: Space between `if` and condition missing.

Fixed

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945879751
PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945879963
PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945879828