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

Jason Merrill jason@redhat.com
Wed Apr 17 19:18:00 GMT 2002


>>>>> "Jason" == Jason Merrill <jason@redhat.com> writes:

> Which is what libio does, and which would seem to be the only way to avoid
> this sort of problem.  Can anyone enlighten me as to the design
> goals/decisions of the current implementation?

The current implementation seems to be trying to allow interleaved reads
and writes on the same buffer, whereas the libio implementation requires a
flush when switching modes.  Was this difference a deliberate design
decision?  I'm a bit skeptical of it's usefulness; how often do people
really read a few characters, write a few characters, and then read a few
more characters without an intervening seek?

A problem with the current implementation of this is that if we do a read
on an input/output filebuf, we end up writing the contents of the buffer
back out to the file, even if we've never requested a write.  Oops.

It would be possible to avoid this by changing how we manage the various
pointers into the buffer; when reading, if _M_out_beg == _M_out_cur we can
bump them together.  If then we only write out the characters between them,
there won't be anything to write unless we actually requested a write.

I was thinking that we couldn't change this, because a lot of the
adjustment happens in non-virtual functions like sbumpc.  But then of
course the semantics specified in the standard (i.e. sbumpc just advances
the read pointer) don't support proper filebuf semantics on the existing
scheme either; if the read pointer is bumped, and the write pointer is also
active, it needs to be bumped as well.  In libio, this problem is avoided
by the mode switch; when it switches from read to write, the write pointer
is copied from the read pointer.

In v3, this problem is handled, basically, by contaminating streambuf with
information about filebuf semantics.  We need the streambuf member
functions to adjust _M_out_cur with _M_in_cur, so we add a flag
(_M_buf_unified) that says so, and handle the logic in the _M_*_cur_move
functions.  Similarly, for the benefit of stringbuf, we pretend that we can
just bump _M_out_end if we run up against it and we happen to know that
there's still room in the buffer.

It seems unfortunate to me that we need special hacks in streambuf to
support both of the standard derived streambufs.  Both seem to be for
optimization; the filebuf hack to allow reading and writing on the same
buffer, and the stringbuf hack to avoid having to call overflow() through
the vtable for each character we want to add to the end of the string.  Am
I right?

Anyway, here's a patch to clean up filebuf somewhat.  It involves these
design changes:

  1) The external file pointer always corresponds to _M_in_end.
     Previously, it corresponded to _M_in_beg (thus the seeks).
  2) _M_out_end always points to the end of the buffer.  Previously, it
     would point to the end of the valid contents, and we would rely on
     _M_out_cur_move to push it forward if there was still room in the
     buffer, as it still does for stringbufs.  We don't need to do this
     for filebufs, as _M_out_end doesn't have any special meaning like it
     does for stringbufs.
  3) _M_out_beg always points to the beginning of the write area.  If we
     haven't written anything to the buffer, _M_out_cur == _M_out_beg.
     Previously the area between the two could consist entirely of
     characters that were read.
  4) We now override uflow() in filebuf, so we don't have to unget the
     character read by underflow() in the sync_with_stdio case.

logauswerter.C times:
          synced   not synced
2.96       0:19       0:19
pre-patch  1:09       0:14
post-patch 0:26       0:13

Interesting that v3 is faster than v2 when not synced with stdio...
I feel like I know my way around streambufs a lot better now.

Tested i686-pc-linux-gnu, no regressions.  Any objections?

Jason

2002-04-18  Jason Merrill  <jason@redhat.com>

	PR libstdc++/4150
	* include/std/std_streambuf.h (basic_streambuf::_M_in_cur_move):
	Also bump _M_out_beg if _M_buf_unified.
	(basic_streambuf::_M_out_cur_move): Don't bump _M_in_cur past 
	_M_in_end.
	(basic_streambuf::_M_set_indeterminate): Move to filebuf.
	(basic_streambuf::_M_set_determinate): Likewise.
	(basic_streambuf::_M_is_indeterminate): Likewise.
	* include/bits/std_fstream.h (basic_filebuf::_M_really_underflow):
	New non-static member function.
	(basic_filebuf::_M_underflow, _M_uflow): Call it.
	(basic_filebuf::sync): seekpos will handle all the positioning.
	(basic_filebuf::_M_set_indeterminate): Move here from streambuf.
	Always set _M_out_end to the end of the buffer.
	(basic_filebuf::_M_set_determinate): Likewise.
	(basic_filebuf::_M_is_indeterminate): Likewise.
	* include/bits/fstream.tcc (basic_filebuf::close): Stuff to
	write is from _M_out_beg to _M_out_cur.
	(basic_filebuf::overflow): Likewise.
	(basic_filebuf::_M_really_overflow): Likewise.
	Seek back to _M_out_beg if necessary.
	(basic_filebuf::seekoff): Likewise.
	(basic_filebuf::_M_really_underflow): Generalization of old 
	underflow().  Don't seek back to _M_in_beg.
	* testsuite/27_io/filebuf_virtuals.cc (test05): Don't overspecify 
	ungetc test.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: text/x-patch
Size: 15878 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20020417/79cd004d/attachment.bin>


More information about the Libstdc++ mailing list