Re: [PATCH] tee: let the terminal/OS buffer IO as needed

[Mike Zelik]

I see, the motivation to permit standard OS/terminal-level buffering
was primarily for performance reasons. For example, ignoring the UUOC
for simplicity,

the following command
# dd if=/dev/zero iflag=fullblock status=progress bs=8M | tee | cat > /dev/null

was reporting a much slower rate of output than the analogous
# dd if=/dev/zero iflag=fullblock status=progress bs=8M | cat | cat > /dev/null

>From the source, it appeared this performance difference was due to
the implementation differences between tee.c : tee_files(...) and
cat.c : simple_cat(...) respectively, wherein which tee was explicitly
un-buffering all output and thus forcing a flush on every write (while
cat went along with standard terminal/OS buffering).

However, upon further investigation, I've noticed that all my tests
were ran on Puppy Linux (v8.0), a distro that uses BusyBox (v1.30.1)
for these utilities. After trying this on an Arch Linux system, which
does not use BusyBox, I'm noticing no performance differences at all.

So, whatever the underlying reasons for the performance differences on
Puppy Linux may be, it doesn't seem related to the cat & tee
implementations here.

On a side note, however --- does your reasoning in the previous email
about the use of setvbuf(...) in tee.c then suggest that cat.c :
simple_cat(...) should be doing that too? (Since, no
translation/conversion is performed!) If so, I can submit a little
patch here for that change too^^

Thank you,
Mike

On 12/5/19, Pádraig Brady <address@hidden> wrote:
> On 05/12/2019 05:18, Mike Zelik wrote:
>> * src/tee.c: remove setvbuf(...) unbuffering
>> (Forcing a flush after every write is needlessly inefficient)
>> ---
>>   src/tee.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/src/tee.c b/src/tee.c
>> index d3aecc7b1..ec66a793e 100644
>> --- a/src/tee.c
>> +++ b/src/tee.c
>> @@ -205,7 +205,6 @@ tee_files (int nfiles, char **files)
>>     files--;
>>     descriptors[0] = stdout;
>>     files[0] = bad_cast (_("standard output"));
>> -  setvbuf (stdout, NULL, _IONBF, 0);
>>     n_outputs++;
>>
>>     for (i = 1; i <= nfiles; i++)
>> @@ -221,7 +220,6 @@ tee_files (int nfiles, char **files)
>>           }
>>         else
>>           {
>> -          setvbuf (descriptors[i], NULL, _IONBF, 0);
>>             n_outputs++;
>>           }
>>       }
>>
>
> Well the idea here is that tee is not generating/transforming data,
> so should propagate the data as soon as it gets it.
>
> As a side note, somewhat related to this, I've a local patch
> to change from using stdio streams to using files.
> (tee was converted to streams initially to support a compression option I
> think).
>
> cheers,
> Pádraig
>

DIFF
Description: Binary data