PATCH: Avoid excessive flushing on lower-layer handle

Loren James Rittle rittle@latour.rsch.comm.mot.com
Mon Apr 22 22:46:00 GMT 2002


(This is an update that addresses Benjamin's concern about breaking
 this test:  http://gcc.gnu.org/ml/libstdc++/1999-q4/msg00108.html)

With Jason's 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 - including my last attempt which broke Scott's test):

	* include/std/std_fstream.h (basic_filebuf::sync): Hoist
	unconditional flush on lower-layer handle to here...
	* include/bits/fstream.tcc (basic_filebuf::_M_really_overflow):
	...from here.  Optimize remaining _M_file.sync() call pattern.

[If we are willing to entertain a larger patch, other options exist.]

Index: include/std/std_fstream.h
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/include/std/std_fstream.h,v
retrieving revision 1.9
diff -c -w -r1.9 std_fstream.h
*** include/std/std_fstream.h	22 Apr 2002 20:28:04 -0000	1.9
--- include/std/std_fstream.h	23 Apr 2002 05:01:03 -0000
***************
*** 204,217 ****
  
  	// Make sure that the internal buffer resyncs its idea of
  	// the file position with the external file.
! 	if (__testput && !_M_file.sync())
  	  {
  	    // Need to restore current position after the write.
  	    off_type __off = _M_out_cur - _M_out_end;
! 	    _M_really_overflow();
  	    if (__off)
  	      _M_file.seekoff(__off, ios_base::cur);
  	  }
  	_M_last_overflowed = false;
  	return 0;
        }
--- 204,219 ----
  
  	// Make sure that the internal buffer resyncs its idea of
  	// the file position with the external file.
! 	if (__testput)
  	  {
  	    // Need to restore current position after the write.
  	    off_type __off = _M_out_cur - _M_out_end;
! 	    _M_really_overflow(); // _M_file.sync() will be called within
  	    if (__off)
  	      _M_file.seekoff(__off, ios_base::cur);
  	  }
+ 	else
+ 	  _M_file.sync();
  	_M_last_overflowed = false;
  	return 0;
        }
Index: include/bits/fstream.tcc
===================================================================
RCS file: /cvs/gcc/gcc/libstdc++-v3/include/bits/fstream.tcc,v
retrieving revision 1.30
diff -c -w -r1.30 fstream.tcc
*** include/bits/fstream.tcc	22 Apr 2002 20:28:04 -0000	1.30
--- include/bits/fstream.tcc	23 Apr 2002 05:01:03 -0000
***************
*** 476,491 ****
  				   __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);
--- 476,495 ----
  				   __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(true) mode): 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 (against mainline).  The following test case continues to
operate as users expect when stdout is tied to stderr by the shell
before program execution:

#include <iostream>
using namespace std;

main ()
{
  cout << "hello ";
  cout.flush ();
  cerr << "fine ";
  cerr.flush ();
  cout << "world" << endl;
  cout.flush ();
}

Many PRs were closed by telling people to just add sync_with_stdio(false);
this patch addresses an underlying reason why many users found and posted
about once write call per character.

Regards,
Loren



More information about the Libstdc++ mailing list