Re: libstdc++/4150: catastrophic performance decrease in C++ code

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

With Jason's proposed design change, I think this patch should finally
be OK as well (versions have been floated in the past, but they never
worked quite right):

(I thought that there was an outstanding PR for it, but I can't find
 it.  Perhaps it was only ever discussed on the mailing list.)

	* include/bits/fstream.tcc (basic_filebuf::_M_really_overflow):
	Optimize sync() behavior.

*** .snapshot/hourly.0/include/bits/fstream.tcc	Thu Apr 18 03:01:21 2002
--- include/bits/fstream.tcc	Thu Apr 18 07:19:26 2002
***************
*** 473,488 ****
  				   __elen, __plen);
  
  	  // Convert pending sequence to external representation, output.
  	  if (!traits_type::eq_int_type(__c, traits_type::eof()))
  	    {
  	      char_type __pending = traits_type::to_char_type(__c);
  	      _M_convert_to_external(&__pending, 1, __elen, __plen);
- 	    }
  
! 	  // Last, sync internal and external buffers.
! 	  // NB: Need this so that external byte sequence reflects
! 	  // internal buffer plus pending sequence.
! 	  if (__elen == __plen && !_M_file.sync())
  	    {
  	      _M_set_indeterminate();
  	      __ret = traits_type::not_eof(__c);
--- 473,492 ----
  				   __elen, __plen);
  
  	  // Convert pending sequence to external representation, output.
+ 	  // If eof, then just attempt sync.
  	  if (!traits_type::eq_int_type(__c, traits_type::eof()))
  	    {
  	      char_type __pending = traits_type::to_char_type(__c);
  	      _M_convert_to_external(&__pending, 1, __elen, __plen);
  
! 	      // User code must flush when switching modes (thus don't sync).
! 	      if (__elen == __plen)
! 		{
! 		  _M_set_indeterminate();
! 		  __ret = traits_type::not_eof(__c);
! 		}
! 	    }
! 	  else if (!_M_file.sync())
  	    {
  	      _M_set_indeterminate();
  	      __ret = traits_type::not_eof(__c);

Reason we want it: A whole lot of extra fflush() calls happen in the
sync_with_stdio(true) case (this is the flip side of the extra seeking
on input).  I once wrote in the comment of a past proposed version of
this patch: "We really want to make sure that the IO layer below us
has all the bytes from our buffer, not force them to do something
inefficient."  If the user must now flush to switch modes, this should
always be a safe optimization.

Demonstration of benefit:

#include <iostream>
#include <string>

main() {
std::cout << "hello" << ' ' << "world" <<std::endl;
std::cout << "Enter your name: ";
std::string s;
std::cin >> s;
std::cout << "hello " << s << std::endl;
}

before patch (one fflush() call per character written in sync_with_stdio):
41 write calls, 1 read call

after patch (line buffering enforced by libc):
write(1,0x804b000,12)                            = 12 (0xc)
write(1,0x804b000,17)                           = 17 (0x11)
read(0x0,0x804b400,0x400)                        = 6 (0x6)
write(1,0x804b000,12)                            = 12 (0xc)

No regressions seen in test suite on i386-unknown-freebsd4.5 with this
patch layered atop Jason's proposed patch.

Regards,
Loren