RFR [6961766]: PrintStream.write() should flush at most once

[Ivan Gerasimov]

Hello David!

On 08.08.2013 6:18, David Holmes wrote:
> Hi Ivan,
>
> On 8/08/2013 3:53 AM, Ivan Gerasimov wrote:
>> Hello David!
>>
>> Thanks for review.
>>
>>> Yes - this is NOT A BUG this is the spec for this class:
>>>
>>> "Optionally, a PrintStream can be created so as to flush
>>> automatically; this means that the flush method is automatically
>>> invoked after a byte array is written, one of the println methods is
>>> invoked, or a newline character or byte ('\n') is written. "
>>>
>> Sorry, I don't see how the proposed change would contradict the spec .
>> The code first writes a char[] buffer to an OutputStream, and then
>> invokes flush() if the written array contained at least one '\n' char.
>>
>> In addition to that, the documentation for OutputStream#flush() says:
>> "Flushes this output stream and forces any buffered output bytes to be
>> written out." Thus, there should be no point to have several subsequent
>> calls to flush() with no data writes between them.
>
> Sorry I misread the original code - it actually violates the spec as 
> well in my opinion. As I read it there should be a flush as soon as 
> the \n is written, not simply a flush at some later point in time if a 
> \n happened to have been written. As written those additional flushes 
> will be no-ops as you rightly point out. So your change is a more 
> efficient version of the existing "incorrect" implementation.
>
It's not hard to rewrite the code, so it would flush after each '\n' 
occurrence.
But would it be a good change?
It would likely introduce a performance degradation and change the 
current behavior of write() function.

The fix as it was proposed (reduce number of flushes to at most one) 
doesn't make any difference in most cases.
As Alan said, in most implementations of OutputStream, flush() is no-op.
It can slightly improve performance with PipedOutputStream as it 
synchronizes on the sink in its flush implementation.

Sincerely yours,
Ivan

> That said the layering of the streams in this class is quite confusing 
> to me and it seems odd that if both textOut and charOut are 
> unconditionally flushed in this method, then why is 'out' only flushed 
> based on autoflush and the presence of the \n ??
>
> But I withdraw my objections as what you propose does not change the 
> behaviour.
>
> Thanks,
> David
>
>> I don't insist on pushing this change, but I think it's harmless and may
>> be useful.
>>
>> Sincerely yours,
>> Ivan
>>
>>> This bug report should be closed as "not an issue".
>>>
>>> David
>>> -----
>>>
>>>
>>>> -Alan
>>>
>>>
>>
>
>