Re: PATCH: Improve efficiency (as measured by system calls made)

[Loren James Rittle <rittle at latour dot rsch dot comm dot mot dot com>]

In article <Pine.SOL.3.91.1011120195750.89C-100000@taarna.cygnus.com>,
Benjamin Kosnik <bkoz@redhat.com> writes:

>> Results with patch:
>> 
>> truss a.out             # 25 write system calls since line-buffered
>> truss >/dev/null a.out  #  2 write calls, one write call per C IO buffer

> Great news. Thanks again for the performance analysis. 

> If I remember correctly the sync was in so that redirection would work 
> correctly, ie

> a.out > logfile

> etc. 

> I couldn't figure out how to test this with dejagnu either, so it 
> consistently broke (look for scott [snyder] in the ChangeLog entries...)

[Sorry, I have been away from work e-mail due to holiday and Civ3
 addiction...]

OK, this ChangeLog entry looks reasonably related:

1999-11-09  Scott Snyder  <snyder@fnal.gov>

        * bits/std_fstream.h (basic_filebuf::sync): Unconditionally call
        _M_file->sync() so that redirection works correctly.

Here is the related e-mail with his test case:

http://gcc.gnu.org/ml/libstdc++/1999-q4/msg00108.html
http://gcc.gnu.org/ml/libstdc++/1999-q4/msg00154.html

Now, the code he patched, basic_filebuf::sync(), was basically
reworked since then:

1999-12-21  Benjamin Kosnik  <bkoz@kcygnus.com>

        * bits/std_fstream.h (sync): Move _M_file->sync() to
        _M_really_overflow().

(Although the current basic_filebuf::sync code still has a conditional
 call to _M_file->sync...)

I could find no mailing list message around that time (Dec 1999) with
a motivating test case, etc or even the patch related to this move
(the rules were different then and I am not complaining anyways ;-).
I will try to force the CVS Attic to cough up the exact patch for
further inspection.

My patch removes an unconditional _M_file->sync() call from
include/bits/fstream.tcc (basic_filebuf::_M_really_overflow).  As we
know, _M_really_overflow() is called *a lot* when the C++ output
buffer size is 1 and we are relying on the C buffer for reasonable
performance...

Scott thought that cout.flush() / cerr.flush() calls should
synchronize IO across low-level file handles.  You did not dispute and
I do not dispute his claim in terms of user expectation and I agree
that my patch breaks his ideal situation after manually testing his
case.  Scott also said that without any explicit ostream::flush()
calls he did not expect any particular interleaving between distinct
C++-layer handles that happen to map to a shared low-level file
handle.  I agree with that as well.  Should we not make the
infrequently-called ostream::flush() do this expensive (system
call-wise) operation instead of in the main line of
_M_really_overflow()?

Regards,
Loren