Discussion:
[v3] Fix PR libstdc++/10276
(too old to reply)
Jerry Quinn
2003-04-03 05:48:13 UTC
Permalink
OK, the reason we're getting memory leaks is that the locale cache was never
getting deleted. The callback got registered within the first call to
basic_ios::init, but the callback list was blown away by the second call,
hence no deletion.

This doesn't address the zero allocation issues so we still leak the bytes for
the standard stream caches.

Simple fix, no regressions on 3.3.

2003-04-03 Jerry Quinn <***@optonline.net>

* src/ios.cc (ios_base::_M_init): Remove _M_callbacks initialization.

*** ios.cc.~1.33.2.4.~ Thu Mar 6 20:44:11 2003
--- ios.cc Thu Apr 3 00:40:30 2003
***************
*** 287,293 ****
_M_precision = 6;
_M_width = 0;
_M_flags = skipws | dec;
- _M_callbacks = 0;
_M_ios_locale = locale();
}

--- 287,292 ----
Carlo Wood
2003-04-03 10:30:08 UTC
Permalink
Jerry, are you planning to implement the zero-allocation
fix before the release of 3.3? Because if you don't,
they I'll have to write a difficult work-around for my
project (libcwd), especially for 3.3 :/

Please let me know - I was waiting with further attempts
to get things working in the hope that the zero-allocation
patch would also go into 3.3.
Post by Jerry Quinn
This doesn't address the zero allocation issues so we still leak the bytes for
the standard stream caches.
--
Carlo Wood <***@alinoe.com>
Jerry Quinn
2003-04-03 14:19:41 UTC
Permalink
Post by Carlo Wood
Jerry, are you planning to implement the zero-allocation
fix before the release of 3.3? Because if you don't,
they I'll have to write a difficult work-around for my
project (libcwd), especially for 3.3 :/
Please let me know - I was waiting with further attempts
to get things working in the hope that the zero-allocation
patch would also go into 3.3.
Benjamin posted the fix he's trying on the mainline. I'll give it a
spin soon and see how it goes for me.

Jerry Quinn
Carlo Wood
2003-04-11 10:29:28 UTC
Permalink
Post by Jerry Quinn
Post by Carlo Wood
Jerry, are you planning to implement the zero-allocation
fix before the release of 3.3? Because if you don't,
they I'll have to write a difficult work-around for my
project (libcwd), especially for 3.3 :/
Please let me know - I was waiting with further attempts
to get things working in the hope that the zero-allocation
patch would also go into 3.3.
Benjamin posted the fix he's trying on the mainline. I'll give it a
spin soon and see how it goes for me.
Jerry Quinn
Hiya Jerry,

how is this going? I synced with cvs yesterday, and the last
patch is still from a week ago (2003-04-04).

Can you give me an estimation of when you think the zero-alloc
cache patch will be finished and applied?

I am willing to help, if you tell me what needs to be done :).
--
Carlo Wood <***@alinoe.com>
B. Kosnik
2003-04-11 18:02:21 UTC
Permalink
I don't know.

Jerry, I'm back from wandering around in the desert of the
testsuite work, so I can help on this too. I'm not quite sure what's up.

My plan now is to read/figure out what is up with the exception issue as
discussed previously in the meantime.

-benjamin
Jerry Quinn
2003-04-14 05:44:20 UTC
Permalink
Post by B. Kosnik
I don't know.
Jerry, I'm back from wandering around in the desert of the
testsuite work, so I can help on this too. I'm not quite sure what's up.
My plan now is to read/figure out what is up with the exception issue as
discussed previously in the meantime.
-benjamin
I've been stuck in the real world for a while, but I'm now looking at
this again. Then I'll get back to reworking the caches to be indexed
by charT for a locale.

I just tried on an Athlon with the current 3.3 branch and valgrind
1.9.5 is not showing any leaks. Very odd ... The caches for the
stdio objects should still leak, no? I have to go back and check on
the other machine.

Jerry
B. Kosnik
2003-04-14 17:20:14 UTC
Permalink
Post by Jerry Quinn
I've been stuck in the real world for a while, but I'm now looking at
this again. Then I'll get back to reworking the caches to be indexed
by charT for a locale.
Great!
Post by Jerry Quinn
I just tried on an Athlon with the current 3.3 branch and valgrind
1.9.5 is not showing any leaks. Very odd ... The caches for the
stdio objects should still leak, no? I have to go back and check on
the other machine.
I see the leak on mainline.

-benjamin
Jerry Quinn
2003-04-15 04:11:45 UTC
Permalink
Post by B. Kosnik
Post by Jerry Quinn
I just tried on an Athlon with the current 3.3 branch and valgrind
1.9.5 is not showing any leaks. Very odd ... The caches for the
stdio objects should still leak, no? I have to go back and check on
the other machine.
I see the leak on mainline.
Does this mean that 3.3 calls destructors on the standard streams and
mainline does not?

Jerry
B. Kosnik
2003-04-15 16:36:36 UTC
Permalink
Post by Jerry Quinn
Does this mean that 3.3 calls destructors on the standard streams and
mainline does not?
I don't think so.

I don't know what is going on, sorry. I haven't looked too closely at 3.3.

-benjamin
Jerry Quinn
2003-04-18 04:29:32 UTC
Permalink
Post by Carlo Wood
Hiya Jerry,
how is this going? I synced with cvs yesterday, and the last
patch is still from a week ago (2003-04-04).
Can you give me an estimation of when you think the zero-alloc
cache patch will be finished and applied?
Hi, Carlo. I took BenK's patch and fleshed it out some. This is the
potential patch for 3.3. I'm working up one for mainline as well.

What do you think? Does this do the job for 3.3?

04-18-2003 Jerry Quinn <***@optonline.net>
Benjamin Kosnik <***@redhat.com>

* include/bits/basic_ios.h (ios_base::Init::_S_ios_create):
Declare friend.
(basic_ios::init, basic_ios::_M_cache_locale): Add locale
cache argument.
* include/bits/basic_ios.tcc (basic_ios::init): Pass cache to
_M_cache_locale.
(basic_ios::_M_cache_locale): Use placement new if cache is
provided. Track the distinction in iword(0).
* include/bits/locale_facets.tcc
(__locale_cache::_S_callback): Only delete cache if iword(0)
is 0, i.e. not static.
* src/globals.cc: Allocate space for __locale_cache objects.
* src/ios.cc (__gnu_cxx): Declare extern __locale_cache objects
for standard wide and narrow stream objects.
(ios_base::Init::_S_ios_create): Use them.


Index: include/bits/basic_ios.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.h,v
retrieving revision 1.14.2.3
diff -u -r1.14.2.3 basic_ios.h
--- include/bits/basic_ios.h 9 Mar 2003 02:00:51 -0000 1.14.2.3
+++ include/bits/basic_ios.h 18 Apr 2003 03:41:34 -0000
@@ -81,6 +81,8 @@
typedef istreambuf_iterator<_CharT, _Traits> __istreambuf_iter;
typedef num_get<_CharT, __istreambuf_iter> __numget_type;
//@}
+
+ friend void ios_base::Init::_S_ios_create(bool);

// Data members:
protected:
@@ -418,10 +420,13 @@
* @brief All setup is performed here.
*
* This is called from the public constructor. It is not virtual and
- * cannot be redefined.
+ * cannot be redefined. The second argument, __cache, is used
+ * to initialize the standard streams without allocating
+ * memory.
*/
void
- init(basic_streambuf<_CharT, _Traits>* __sb);
+ init(basic_streambuf<_CharT, _Traits>* __sb,
+ __locale_cache<_CharT>* __cache=0);

bool
_M_check_facet(const locale::facet* __f) const
@@ -432,7 +437,7 @@
}

void
- _M_cache_locale(const locale& __loc);
+ _M_cache_locale(const locale& __loc,__locale_cache<_CharT>* __cache = 0);

#if 1
// XXX GLIBCXX_ABI Deprecated, compatibility only.
Index: include/bits/basic_ios.tcc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.tcc,v
retrieving revision 1.17.4.2
diff -u -r1.17.4.2 basic_ios.tcc
--- include/bits/basic_ios.tcc 5 Mar 2003 04:40:07 -0000 1.17.4.2
+++ include/bits/basic_ios.tcc 18 Apr 2003 03:41:35 -0000
@@ -144,11 +144,12 @@

template<typename _CharT, typename _Traits>
void
- basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb)
+ basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb,
+ __locale_cache<_CharT>* __cache)
{
// NB: This may be called more than once on the same object.
ios_base::_M_init();
- _M_cache_locale(_M_ios_locale);
+ _M_cache_locale(_M_ios_locale, __cache);
_M_tie = 0;

// NB: The 27.4.4.1 Postconditions Table specifies requirements
@@ -173,7 +174,8 @@

template<typename _CharT, typename _Traits>
void
- basic_ios<_CharT, _Traits>::_M_cache_locale(const locale& __loc)
+ basic_ios<_CharT, _Traits>::_M_cache_locale(const locale& __loc,
+ __locale_cache<_CharT>* __cache)
{
if (__builtin_expect(has_facet<__ctype_type>(__loc), true))
_M_fctype = &use_facet<__ctype_type>(__loc);
@@ -189,7 +191,16 @@
_M_fnumget = 0;
typedef __locale_cache<_CharT> __cache_t;
if (!pword(0)) {
- pword(0) = auto_ptr<__cache_t>(new __cache_t()).release();
+ // We store the cache at pword(0). iword(0) is set to 1 to
+ // indicate that this is a static storage cache that shouldn't
+ // be deleted.
+ if (__cache)
+ {
+ pword(0) = auto_ptr<__cache_t>(new (__cache) __cache_t()).release();
+ iword(0) = 1; // so we don't try to clobber static cache
+ }
+ else
+ pword(0) = auto_ptr<__cache_t>(new __cache_t()).release();
register_callback(__cache_t::_S_callback, 0);
}
static_cast<__cache_t&>(_M_cache())._M_init(__loc);
Index: include/bits/locale_facets.tcc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/locale_facets.tcc,v
retrieving revision 1.82.4.8
diff -u -r1.82.4.8 locale_facets.tcc
--- include/bits/locale_facets.tcc 7 Mar 2003 17:26:26 -0000 1.82.4.8
+++ include/bits/locale_facets.tcc 18 Apr 2003 03:41:35 -0000
@@ -2293,7 +2293,7 @@
switch (__ev)
{
case ios_base::erase_event:
- if (__io.pword(0))
+ if (__io.pword(0) && !__io.iword(0))
delete &__io._M_cache();
break;

@@ -2307,6 +2307,7 @@
// basic_ios::_M_cache_locale.
typedef __locale_cache<_CharT> __cache_t;
__io.pword(0) = auto_ptr<__cache_t>(new __cache_t()).release();
+ __io.iword(0) = 0;
break;
}
}
Index: src/globals.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/src/globals.cc,v
retrieving revision 1.12.4.1
diff -u -r1.12.4.1 globals.cc
--- src/globals.cc 4 Mar 2003 02:17:56 -0000 1.12.4.1
+++ src/globals.cc 18 Apr 2003 03:41:36 -0000
@@ -48,6 +48,23 @@
{
using namespace std;

+ // Also, need locale caches for standard stream objects...
+ typedef char fake_locale_cache[sizeof(__locale_cache<char>)]
+ __attribute__ ((aligned(__alignof__(__locale_cache<char>))));
+ fake_locale_cache locale_cache_cout;
+ fake_locale_cache locale_cache_cin;
+ fake_locale_cache locale_cache_cerr;
+ fake_locale_cache locale_cache_clog;
+
+#ifdef _GLIBCPP_USE_WCHAR_T
+ typedef char fake_wlocale_cache[sizeof(__locale_cache<wchar_t>)]
+ __attribute__ ((aligned(__alignof__(__locale_cache<wchar_t>))));
+ fake_wlocale_cache locale_cache_wcout;
+ fake_wlocale_cache locale_cache_wcin;
+ fake_wlocale_cache locale_cache_wcerr;
+ fake_wlocale_cache locale_cache_wclog;
+#endif
+
typedef char fake_facet_name[sizeof(char*)]
__attribute__ ((aligned(__alignof__(char*))));
fake_facet_name facet_name[6 + _GLIBCPP_NUM_CATEGORIES];
Index: src/ios.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/src/ios.cc,v
retrieving revision 1.33.2.5
diff -u -r1.33.2.5 ios.cc
--- src/ios.cc 4 Apr 2003 14:59:59 -0000 1.33.2.5
+++ src/ios.cc 18 Apr 2003 03:41:36 -0000
@@ -48,11 +48,19 @@
extern stdio_filebuf<char> buf_cout;
extern stdio_filebuf<char> buf_cin;
extern stdio_filebuf<char> buf_cerr;
+ extern std::__locale_cache<char> locale_cache_cout;
+ extern std::__locale_cache<char> locale_cache_cin;
+ extern std::__locale_cache<char> locale_cache_cerr;
+ extern std::__locale_cache<char> locale_cache_clog;

#ifdef _GLIBCPP_USE_WCHAR_T
extern stdio_filebuf<wchar_t> buf_wcout;
extern stdio_filebuf<wchar_t> buf_wcin;
extern stdio_filebuf<wchar_t> buf_wcerr;
+ extern std::__locale_cache<wchar_t> locale_cache_wcout;
+ extern std::__locale_cache<wchar_t> locale_cache_wcin;
+ extern std::__locale_cache<wchar_t> locale_cache_wcerr;
+ extern std::__locale_cache<wchar_t> locale_cache_wclog;
#endif
} // namespace __gnu_cxx

@@ -173,10 +181,15 @@
new (&buf_cout) stdio_filebuf<char>(stdout, ios_base::out, __out_size);
new (&buf_cin) stdio_filebuf<char>(stdin, ios_base::in, __in_size);
new (&buf_cerr) stdio_filebuf<char>(stderr, ios_base::out, __out_size);
+
new (&cout) ostream(&buf_cout);
new (&cin) istream(&buf_cin);
new (&cerr) ostream(&buf_cerr);
new (&clog) ostream(&buf_cerr);
+ cout.init(&buf_cout, &locale_cache_cout);
+ cin.init(&buf_cin, &locale_cache_cin);
+ cerr.init(&buf_cerr, &locale_cache_cerr);
+ clog.init(&buf_cerr, &locale_cache_clog);
cin.tie(&cout);
cerr.flags(ios_base::unitbuf);

@@ -188,6 +201,10 @@
new (&wcin) wistream(&buf_wcin);
new (&wcerr) wostream(&buf_wcerr);
new (&wclog) wostream(&buf_wcerr);
+ wcout.init(&buf_wcout, &locale_cache_wcout);
+ wcin.init(&buf_wcin, &locale_cache_wcin);
+ wcerr.init(&buf_wcerr, &locale_cache_wcerr);
+ wclog.init(&buf_wcerr, &locale_cache_wclog);
wcin.tie(&wcout);
wcerr.flags(ios_base::unitbuf);
#endif
Benjamin Kosnik
2003-04-18 17:12:36 UTC
Permalink
Post by Jerry Quinn
Declare friend.
(basic_ios::init, basic_ios::_M_cache_locale): Add locale
cache argument.
* include/bits/basic_ios.tcc (basic_ios::init): Pass cache to
_M_cache_locale.
(basic_ios::_M_cache_locale): Use placement new if cache is
provided. Track the distinction in iword(0).
* include/bits/locale_facets.tcc
(__locale_cache::_S_callback): Only delete cache if iword(0)
is 0, i.e. not static.
* src/globals.cc: Allocate space for __locale_cache objects.
* src/ios.cc (__gnu_cxx): Declare extern __locale_cache objects
for standard wide and narrow stream objects.
(ios_base::Init::_S_ios_create): Use them.
This looks good!

-benjamin
Carlo Wood
2003-04-21 15:22:45 UTC
Permalink
Post by Jerry Quinn
Hi, Carlo. I took BenK's patch and fleshed it out some. This is the
potential patch for 3.3. I'm working up one for mainline as well.
What do you think? Does this do the job for 3.3?
Thanks Jerry,

I just got back from a long weekend.

I *did* start in the meantime with the difficult way
to "fix" the problem - and therefore messed up my source tree
too much (didn't get it to work yet). The best way out is
probably to forget about it again and start to use libstdc++
with your patch.

I think it might take one or two weeks before I finished
testing it, because there are a lot of loose ends right
now in my project :(
--
Carlo Wood <***@alinoe.com>
Carlo Wood
2003-04-24 02:34:02 UTC
Permalink
Post by Jerry Quinn
What do you think? Does this do the job for 3.3?
There seems to be a problem:

/usr/local/gcc-cvs-3.3/lib/gcc-lib/i686-pc-linux-gnu/3.3/../../../libstdc++.so: undefined reference to `std::basic_ios<wchar_t, std::char_traits<wchar_t> >::init(std::basic_streambuf<wchar_t, std::char_traits<wchar_t> >*, std::__locale_cache<wchar_t>*)'
/usr/local/gcc-cvs-3.3/lib/gcc-lib/i686-pc-linux-gnu/3.3/../../../libstdc++.so: undefined reference to `std::basic_ios<char, std::char_traits<char> >::init(std::basic_streambuf<char, std::char_traits<char> >*, std::__locale_cache<char>*)'
collect2: ld returned 1 exit status
--
Carlo Wood <***@alinoe.com>
Jerry Quinn
2003-04-24 05:11:13 UTC
Permalink
Post by Carlo Wood
Post by Jerry Quinn
What do you think? Does this do the job for 3.3?
/usr/local/gcc-cvs-3.3/lib/gcc-lib/i686-pc-linux-gnu/3.3/../../../libstdc++.so: undefined reference to `std::basic_ios<wchar_t, std::char_traits<wchar_t> >::init(std::basic_streambuf<wchar_t, std::char_traits<wchar_t> >*, std::__locale_cache<wchar_t>*)'
/usr/local/gcc-cvs-3.3/lib/gcc-lib/i686-pc-linux-gnu/3.3/../../../libstdc++.so: undefined reference to `std::basic_ios<char, std::char_traits<char> >::init(std::basic_streambuf<char, std::char_traits<char> >*, std::__locale_cache<char>*)'
collect2: ld returned 1 exit status
--
You need to completely rebuild the v3 library. The following works
for me:

cd ~/gcc/build/i686-pc-linux-gnu/libstdc++-v3
rm -r include src
./config.status
make install

Someday if I finish up the various speedup-related stuff, maybe I'll
attempt to wade into the configury and figure out how to make the
dependencies work correctly <shiver/>

Jerry
Carlo Wood
2003-04-24 10:41:07 UTC
Permalink
Post by Jerry Quinn
You need to completely rebuild the v3 library. The following works
Thanks.

I wrote the previous mail just before I went to bed
and immedeately after my post I started a new
rebuild, including a cvs update.

The problem went away. (I didn't get to *testing* yet).

However - now I ran into a ICE, a regression in every way.

I'll try to make a PR for it and mail everyone who applied
a patch since the last cvs sync that I did :/
--
Carlo Wood <***@alinoe.com>
Carlo Wood
2003-04-27 15:39:15 UTC
Permalink
Post by Jerry Quinn
Hi, Carlo. I took BenK's patch and fleshed it out some. This is the
potential patch for 3.3. I'm working up one for mainline as well.
What do you think? Does this do the job for 3.3?
Hi again Jerry,

I started testing with your patch now (already (in) CVS)
and there seems to be another problem.

Every class that is derived from 'ostream' somehow WILL
allocate memory - which is what we are trying to avoid no?

For example, I have this class:

class bufferstream_ct : public std::basic_ostream<char, std::char_traits<char> > { ... };

this class should not allocate memory (for the locale stuff),
but what happens is this: basic_ostream only has a single
constructor. If we look at $PREFIX/include/c++/3.3/ostream
we see only:

explicit
basic_ostream(__streambuf_type* __sb)
{ this->init(__sb); }

therefore, it is garanteed that this will call
std::basic_ios<char, std::char_traits<char> >::init(__sb, 0)

by not having the possibility to pass a cache, nothing has
changed and std::basic_ios will do the two allocations we
are trying to avoid.

The same holds for the standard streams as you can see
for example in the following backtrace:

#1 0x400913be in operator new(unsigned) (size=60) at debugmalloc.cc:3482
#2 0x40109bb3 in std::basic_ios<char, std::char_traits<char> >::_M_cache_locale(std::locale const&, std::__locale_cache<char>*) (this=0x8052908, __loc=@0x8052970, __cache=0x0) at memory:183
#3 0x401099d1 in std::basic_ios<char, std::char_traits<char> >::init(std::basic_streambuf<char, std::char_traits<char> >*, std::__locale_cache<char>*) (this=0x8052904, __sb=0x40175140, __cache=0x3c) at basic_ios.tcc:152
#4 0x4010b1e5 in std::ios_base::Init::_S_ios_create(bool) (__sync=64) at ostream:106
#5 0x4010b865 in Init (this=0x401759f0) at /usr/src/gcc/gcc-cvs-3.3/libstdc++-v3/src/ios.cc:236
#6 0x4010aed7 in __static_initialization_and_destruction_0 (__initialize_p=1075254032, __priority=65535) at iostream:77

this is simply because in ios_base::Init::_S_ios_create(bool __sync)
in ios.cc, you still do:

new (&cout) ostream(&buf_cout);

which calls the above constructor
and therefore allocates memory for new
locale.

A bit later you call cout.init:

new (&cout) ostream(&buf_cout);
new (&cin) istream(&buf_cin);
new (&cerr) ostream(&buf_cerr);
new (&clog) ostream(&buf_cerr);
cout.init(&buf_cout, &locale_cache_cout);

but the "harm" is already done.
--
Carlo Wood <***@alinoe.com>
Jerry Quinn
2003-04-28 06:22:09 UTC
Permalink
Post by Carlo Wood
Post by Jerry Quinn
Hi, Carlo. I took BenK's patch and fleshed it out some. This is the
potential patch for 3.3. I'm working up one for mainline as well.
What do you think? Does this do the job for 3.3?
Hi again Jerry,
I started testing with your patch now (already (in) CVS)
and there seems to be another problem.
Every class that is derived from 'ostream' somehow WILL
allocate memory - which is what we are trying to avoid no?
class bufferstream_ct : public std::basic_ostream<char, std::char_traits<char> > { ... };
this class should not allocate memory (for the locale stuff),
but what happens is this: basic_ostream only has a single
constructor. If we look at $PREFIX/include/c++/3.3/ostream
explicit
basic_ostream(__streambuf_type* __sb)
{ this->init(__sb); }
therefore, it is garanteed that this will call
std::basic_ios<char, std::char_traits<char> >::init(__sb, 0)
by not having the possibility to pass a cache, nothing has
changed and std::basic_ios will do the two allocations we
are trying to avoid.
Oops. The following patch moves the hook to pass the static cache to
the stream constructors. It passes make check-abi, but is it actually
safe?

The following is for a prog that prints a string to cout. It still
allocs (and leaks) memory in ios_base::register_callback():

==31788== 128 bytes in 8 blocks are still reachable in loss record 1 of 2
==31788== at 0x401651B8: malloc (vg_clientfuncs.c:103)
==31788== by 0x808B1BE: operator new(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x804AEA5: std::ios_base::register_callback(void (*)(std::ios_base::event, std::ios_base&, int), int) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x809599E: std::basic_ios<char, std::char_traits<char> >::_M_cache_locale(std::locale const&, std::__locale_cache<char>*) (in /home/jlquinn/gcc/test/a.out)
==31788==

The callback_list object gets created, but never deleted. I can
work around this one, but it's going to get very ugly with switch
statements looking explicitly for the static cache objects in places.

Umm, ICK! Actually, there is another leak waiting to happen here,
unrelated to locale cache. The above leak happens because the
destructors are never called for the static streams. If someone
happens to store a new'd object in e.g. cout.pword(10) and register a
callback to clean up the storage, this will never get cleaned up.

This scenario makes me think that we MUST call the standard stream
destructors.


In addition, there are strings in the cache. How do these avoid
allocating? There is also another issue flagged by valgrind, but I
don't know what it is from:


==31788== 2304 bytes in 2 blocks are possibly lost in loss record 2 of 2
==31788== at 0x401651B8: malloc (vg_clientfuncs.c:103)
==31788== by 0x808B1BE: operator new(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x8079996: std::__new_alloc::allocate(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x80795DE: std::__default_alloc_template<true, 0>::_S_chunk_alloc(unsigned, int&) (in /home/jlquinn/gcc/test/a.out)



2003-04-28 Jerry Quinn <***@optonline.net>

* include/std/std_istream.h (basic_istream::basic_istream):
Call init with provided locale cache if supplied.
* include/std/std_ostream.h (basic_ostream::basic_ostream):
Call init with provided locale cache if supplied.
* src/ios.cc: Supply static locale cache to stream
constructor instead of init().


Index: include/std/std_istream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_istream.h,v
retrieving revision 1.5.2.2
diff -u -r1.5.2.2 std_istream.h
--- include/std/std_istream.h 23 Apr 2003 16:44:15 -0000 1.5.2.2
+++ include/std/std_istream.h 28 Apr 2003 05:49:10 -0000
@@ -101,9 +101,9 @@
* their own stream buffer.
*/
explicit
- basic_istream(__streambuf_type* __sb)
+ basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc=0)
{
- this->init(__sb);
+ this->init(__sb, __lc);
_M_gcount = streamsize(0);
}

Index: include/std/std_ostream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_ostream.h,v
retrieving revision 1.5.2.1
diff -u -r1.5.2.1 std_ostream.h
--- include/std/std_ostream.h 9 Mar 2003 02:00:53 -0000 1.5.2.1
+++ include/std/std_ostream.h 28 Apr 2003 05:49:10 -0000
@@ -102,8 +102,8 @@
* their own stream buffer.
*/
explicit
- basic_ostream(__streambuf_type* __sb)
- { this->init(__sb); }
+ basic_ostream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc=0)
+ { this->init(__sb, __lc); }

/**
* @brief Base destructor.
Index: src/ios.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/src/ios.cc,v
retrieving revision 1.33.2.6
diff -u -r1.33.2.6 ios.cc
--- src/ios.cc 22 Apr 2003 15:40:34 -0000 1.33.2.6
+++ src/ios.cc 28 Apr 2003 05:49:10 -0000
@@ -182,14 +182,10 @@
new (&buf_cin) stdio_filebuf<char>(stdin, ios_base::in, __in_size);
new (&buf_cerr) stdio_filebuf<char>(stderr, ios_base::out, __out_size);

- new (&cout) ostream(&buf_cout);
- new (&cin) istream(&buf_cin);
- new (&cerr) ostream(&buf_cerr);
- new (&clog) ostream(&buf_cerr);
- cout.init(&buf_cout, &locale_cache_cout);
- cin.init(&buf_cin, &locale_cache_cin);
- cerr.init(&buf_cerr, &locale_cache_cerr);
- clog.init(&buf_cerr, &locale_cache_clog);
+ new (&cout) ostream(&buf_cout, &locale_cache_cout);
+ new (&cin) istream(&buf_cin, &locale_cache_cin);
+ new (&cerr) ostream(&buf_cerr, &locale_cache_cerr);
+ new (&clog) ostream(&buf_cerr, &locale_cache_clog);
cin.tie(&cout);
cerr.flags(ios_base::unitbuf);

@@ -197,14 +193,10 @@
new (&buf_wcout) stdio_filebuf<wchar_t>(stdout, ios_base::out, __out_size);
new (&buf_wcin) stdio_filebuf<wchar_t>(stdin, ios_base::in, __in_size);
new (&buf_wcerr) stdio_filebuf<wchar_t>(stderr, ios_base::out, __out_size);
- new (&wcout) wostream(&buf_wcout);
- new (&wcin) wistream(&buf_wcin);
- new (&wcerr) wostream(&buf_wcerr);
- new (&wclog) wostream(&buf_wcerr);
- wcout.init(&buf_wcout, &locale_cache_wcout);
- wcin.init(&buf_wcin, &locale_cache_wcin);
- wcerr.init(&buf_wcerr, &locale_cache_wcerr);
- wclog.init(&buf_wcerr, &locale_cache_wclog);
+ new (&wcout) wostream(&buf_wcout, &locale_cache_wcout);
+ new (&wcin) wistream(&buf_wcin, &locale_cache_wcin);
+ new (&wcerr) wostream(&buf_wcerr, &locale_cache_wcerr);
+ new (&wclog) wostream(&buf_wcerr, &locale_cache_wclog);
wcin.tie(&wcout);
wcerr.flags(ios_base::unitbuf);
#endif
Carlo Wood
2003-04-28 10:56:24 UTC
Permalink
Post by Jerry Quinn
Oops. The following patch moves the hook to pass the static cache to
the stream constructors. It passes make check-abi, but is it actually
safe?
It seems to "break" the binary API - that is, the object name of
the init function without a locale_cache being passed is changed;
as a result, programs compiled with libstdc++ version 5.0.x with
x <= 2 will not link anymore with libstdc++-5.0.3.

While this solution might be ok for libstdc++-6 (although I'd ask
someone else about that; I am not an API expert), I think we should
use a totally new constructor for this one.

Thus:

keep explicit basic_istream(__streambuf_type* __sb);

add explicit basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc);

What do you think?
Post by Jerry Quinn
The following is for a prog that prints a string to cout. It still
==31788== 128 bytes in 8 blocks are still reachable in loss record 1 of 2
==31788== at 0x401651B8: malloc (vg_clientfuncs.c:103)
==31788== by 0x808B1BE: operator new(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x804AEA5: std::ios_base::register_callback(void (*)(std::ios_base::event, std::ios_base&, int), int) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x809599E: std::basic_ios<char, std::char_traits<char> >::_M_cache_locale(std::locale const&, std::__locale_cache<char>*) (in /home/jlquinn/gcc/test/a.out)
==31788==
The callback_list object gets created, but never deleted. I can
work around this one, but it's going to get very ugly with switch
statements looking explicitly for the static cache objects in places.
Please don't :), I am hoping for an interface that can be used for
other (global) ostream objects too, not just the standard streams.
Post by Jerry Quinn
Umm, ICK! Actually, there is another leak waiting to happen here,
unrelated to locale cache. The above leak happens because the
destructors are never called for the static streams. If someone
happens to store a new'd object in e.g. cout.pword(10) and register a
callback to clean up the storage, this will never get cleaned up.
This scenario makes me think that we MUST call the standard stream
destructors.
In addition, there are strings in the cache. How do these avoid
allocating? There is also another issue flagged by valgrind, but I
==31788== 2304 bytes in 2 blocks are possibly lost in loss record 2 of 2
==31788== at 0x401651B8: malloc (vg_clientfuncs.c:103)
==31788== by 0x808B1BE: operator new(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x8079996: std::__new_alloc::allocate(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x80795DE: std::__default_alloc_template<true, 0>::_S_chunk_alloc(unsigned, int&) (in /home/jlquinn/gcc/test/a.out)
That is the normal __default_alloc_template<true, 0> memory pool.
It doesn't really leak, it just grows till there is enough
to serve the memory of STL containers.

I've thought of this before though; it would be nice to have
an extention that allows one to free these memory pools,
solely for memory leak testers.
--
Carlo Wood <***@alinoe.com>
Jerry Quinn
2003-04-28 19:51:52 UTC
Permalink
Post by Carlo Wood
Post by Jerry Quinn
Oops. The following patch moves the hook to pass the static cache to
the stream constructors. It passes make check-abi, but is it actually
safe?
It seems to "break" the binary API - that is, the object name of
the init function without a locale_cache being passed is changed;
as a result, programs compiled with libstdc++ version 5.0.x with
x <= 2 will not link anymore with libstdc++-5.0.3.
While this solution might be ok for libstdc++-6 (although I'd ask
someone else about that; I am not an API expert), I think we should
use a totally new constructor for this one.
keep explicit basic_istream(__streambuf_type* __sb);
add explicit basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc);
What do you think?
This is also fine by me. I wasn't sure if it was an issue or not.

Does anyone know why make check-abi doesn't complain about it?
Post by Carlo Wood
Post by Jerry Quinn
The callback_list object gets created, but never deleted. I can
work around this one, but it's going to get very ugly with switch
statements looking explicitly for the static cache objects in places.
Please don't :), I am hoping for an interface that can be used for
other (global) ostream objects too, not just the standard streams.
OK, I'll see if I can conjure another way of dealing with it.

Jerry
B. Kosnik
2003-04-29 16:58:51 UTC
Permalink
Post by Jerry Quinn
This is also fine by me. I wasn't sure if it was an issue or not.
Does anyone know why make check-abi doesn't complain about it?
The link maps are probably exporting all basic_ios, basic_istream bits
right now, so adding symbols means that they probably are added in under
the old version names.

I remarked to Paolo a bit ago that this was a problem. Instead, new
symbols should be checked to come in under the most recent version.

Also, missing symbols should be flagged as incompatible.

Also, all standard types should be instantiated and checked, in addition
to the library binary.

Keep in mind that check-abi is still an evolving target... it was
designed as a second-check for people doing work, and can definitely
stand some improvement. It's my hope that all this ABI stuff will become
less like reading tea leaves and more like catching the Muni J train:
mostly works, but some delays...

best,
benjamin
Nathan Myers
2003-04-28 19:59:54 UTC
Permalink
Post by Carlo Wood
Post by Jerry Quinn
Oops. The following patch moves the hook to pass the static cache to
the stream constructors. It passes make check-abi, but is it actually
safe?
It seems to "break" the binary API - that is, the object name of
the init function without a locale_cache being passed is changed;
as a result, programs compiled with libstdc++ version 5.0.x with
x <= 2 will not link anymore with libstdc++-5.0.3.
While this solution might be ok for libstdc++-6 (although I'd ask
someone else about that; I am not an API expert), I think we should
use a totally new constructor for this one.
keep explicit basic_istream(__streambuf_type* __sb);
add explicit basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc);
What do you think?
As a matter of design policy, it's better to overload than to
add default arguments. Even where the standard explicitly declares
default arguments, we should take the opportunity to make them
overloads instead.

BTW, the second ctor above doesn't need to be declared explicit
(although it's allowed).

Nathan Myers
ncm-***@cantrip.org
Benjamin Kosnik
2003-04-29 16:29:44 UTC
Permalink
Post by Carlo Wood
keep explicit basic_istream(__streambuf_type* __sb);
add explicit basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc);
What do you think?
You are right. This would be the way to go, if this is the way things
are going to be done.
Post by Carlo Wood
Please don't :), I am hoping for an interface that can be used for
other (global) ostream objects too, not just the standard streams.
Yep. I believe I understand what you want.
Post by Carlo Wood
I've thought of this before though; it would be nice to have
an extention that allows one to free these memory pools,
solely for memory leak testers.
More allocations choices would really be nice. If there was a persistent
allocator (perhaps even an __extern_alloc(size, alignment) things could
be cleaner.)

-benjamin
B. Kosnik
2003-04-29 04:16:06 UTC
Permalink
Jerry, I'm taking this as your white flag.
Post by Jerry Quinn
In addition, there are strings in the cache. How do these avoid
allocating? There is also another issue flagged by valgrind, but I
This is the internal string memory managment. You'll need to use an
allocator in your strings that is pre-allocated, or not use strings.

-benjamin
Jerry Quinn
2003-04-29 15:50:59 UTC
Permalink
Post by B. Kosnik
Jerry, I'm taking this as your white flag.
I think I'm not done yet, but will certainly accept help, since my
work rate isn't very fast.
Post by B. Kosnik
Post by Jerry Quinn
In addition, there are strings in the cache. How do these avoid
allocating? There is also another issue flagged by valgrind, but I
This is the internal string memory managment. You'll need to use an
allocator in your strings that is pre-allocated, or not use strings.
Which do you recommend here? I'm not familiar with custom
allocators. I also see potential difficulties, because you probably
only want the preallocated strings for the static standard streams,
no?

I'm going to work up a char buffer approach for now.

Here's a solution for the _Callback_list objects. It seems to work,
but it's not pretty. This is against 3.3.

Opinions?

2003-04-29 Jerry Quinn <***@optonline.net>

* config/linker-map.gnu (basic_ios::init(basic_streambuf,
__locale_cache)): Export new symbol.
* include/bits/basic_ios.[h,tcc] (basic_ios::init(basic_streambuf,
__locale_cache)): Replace default argument with overloaded
function.
* include/bits/ios_base.h (ios_base): Make friends with
__locale_cache_base.
* include/bits/locale_facets.h
(__locale_cache_base::_M_static_cblist): New member.
* include/std/std_istream.h
(basic_istream::basic_istream(__streambuf_type*,
__locale_cache*)): New.
* include/std/std_ostream.h
(basic_ostream::basic_ostream(__streambuf_type*,
__locale_cache*)): New.
* src/ios.cc (cout, cin, cerr, clog, wcout, wcin, wcerr,
wclog): Change initialization of __locale_cache.
(ios_base::register_callback): Use buffer in locale cache for
static _Callback_list objects.
(ios_base::dispose_callbacks): Don't delete _Callback_list for
static locale caches.


Index: config/linker-map.gnu
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/config/linker-map.gnu,v)
retrieving revision 1.25.2.7
diff -u -r1.25.2.7 linker-map.gnu
--- config/linker-map.gnu 7 Mar 2003 22:04:18 -0000 1.25.2.7
+++ config/linker-map.gnu 29 Apr 2003 15:31:26 -0000
@@ -374,6 +374,9 @@

_ZNKSt7num_putI[wc]St19ostreambuf_iteratorI[wc]St11char_traitsI[wc]EEE12_M_group_int*;

+ _ZNSt9basic_iosI[cw]St11char_traitsI[cw]EE4initEPSt15basic_streambufI[cw]S1_EPSt14__locale_cacheI[cw]E;
+
+
# vtable
_ZTVSt19__locale_cache_base;
_ZTVSt14__locale_cacheI[cw]E;
Index: include/bits/basic_ios.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.h,v
retrieving revision 1.14.2.4
diff -u -r1.14.2.4 basic_ios.h
--- include/bits/basic_ios.h 22 Apr 2003 15:40:34 -0000 1.14.2.4
+++ include/bits/basic_ios.h 29 Apr 2003 15:31:27 -0000
@@ -425,8 +425,11 @@
* memory.
*/
void
+ init(basic_streambuf<_CharT, _Traits>* __sb);
+
+ void
init(basic_streambuf<_CharT, _Traits>* __sb,
- __locale_cache<_CharT>* __cache=0);
+ __locale_cache<_CharT>* __cache);

bool
_M_check_facet(const locale::facet* __f) const
Index: include/bits/basic_ios.tcc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.tcc,v
retrieving revision 1.17.4.3
diff -u -r1.17.4.3 basic_ios.tcc
--- include/bits/basic_ios.tcc 22 Apr 2003 15:40:34 -0000 1.17.4.3
+++ include/bits/basic_ios.tcc 29 Apr 2003 15:31:27 -0000
@@ -144,6 +144,11 @@

template<typename _CharT, typename _Traits>
void
+ basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb)
+ { init(__sb, 0); }
+
+ template<typename _CharT, typename _Traits>
+ void
basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb,
__locale_cache<_CharT>* __cache)
{
Index: include/bits/ios_base.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/ios_base.h,v
retrieving revision 1.21.2.5
diff -u -r1.21.2.5 ios_base.h
--- include/bits/ios_base.h 5 Mar 2003 19:09:45 -0000 1.21.2.5
+++ include/bits/ios_base.h 29 Apr 2003 15:31:27 -0000
@@ -376,6 +376,8 @@
iostate _M_streambuf_state;
//@}

+ friend class __locale_cache_base;
+
// 27.4.2.6 Members for callbacks
// 27.4.2.6 ios_base callbacks
struct _Callback_list
Index: include/bits/locale_facets.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/locale_facets.h,v
retrieving revision 1.49.4.6
diff -u -r1.49.4.6 locale_facets.h
--- include/bits/locale_facets.h 5 Mar 2003 04:40:07 -0000 1.49.4.6
+++ include/bits/locale_facets.h 29 Apr 2003 15:31:27 -0000
@@ -1964,6 +1964,11 @@
// calling the virtual functions in locale facets.
class __locale_cache_base
{
+ friend class ios_base;
+
+ // Used to provide space for a callback list entry for static caches.
+ char _M_static_cblist[sizeof(ios_base::_Callback_list)];
+
public:
virtual
~__locale_cache_base() {}
Index: include/std/std_istream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_istream.h,v
retrieving revision 1.5.2.2
diff -u -r1.5.2.2 std_istream.h
--- include/std/std_istream.h 23 Apr 2003 16:44:15 -0000 1.5.2.2
+++ include/std/std_istream.h 29 Apr 2003 15:31:28 -0000
@@ -107,6 +107,12 @@
_M_gcount = streamsize(0);
}

+ basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc)
+ {
+ this->init(__sb, __lc);
+ _M_gcount = streamsize(0);
+ }
+
/**
* @brief Base destructor.
*
Index: include/std/std_ostream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_ostream.h,v
retrieving revision 1.5.2.1
diff -u -r1.5.2.1 std_ostream.h
--- include/std/std_ostream.h 9 Mar 2003 02:00:53 -0000 1.5.2.1
+++ include/std/std_ostream.h 29 Apr 2003 15:31:28 -0000
@@ -105,6 +105,9 @@
basic_ostream(__streambuf_type* __sb)
{ this->init(__sb); }

+ basic_ostream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc)
+ { this->init(__sb, __lc); }
+
/**
* @brief Base destructor.
*
Index: src/ios.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/src/ios.cc,v
retrieving revision 1.33.2.6
diff -u -r1.33.2.6 ios.cc
--- src/ios.cc 22 Apr 2003 15:40:34 -0000 1.33.2.6
+++ src/ios.cc 29 Apr 2003 15:31:28 -0000
@@ -182,14 +182,10 @@
new (&buf_cin) stdio_filebuf<char>(stdin, ios_base::in, __in_size);
new (&buf_cerr) stdio_filebuf<char>(stderr, ios_base::out, __out_size);

- new (&cout) ostream(&buf_cout);
- new (&cin) istream(&buf_cin);
- new (&cerr) ostream(&buf_cerr);
- new (&clog) ostream(&buf_cerr);
- cout.init(&buf_cout, &locale_cache_cout);
- cin.init(&buf_cin, &locale_cache_cin);
- cerr.init(&buf_cerr, &locale_cache_cerr);
- clog.init(&buf_cerr, &locale_cache_clog);
+ new (&cout) ostream(&buf_cout, &locale_cache_cout);
+ new (&cin) istream(&buf_cin, &locale_cache_cin);
+ new (&cerr) ostream(&buf_cerr, &locale_cache_cerr);
+ new (&clog) ostream(&buf_cerr, &locale_cache_clog);
cin.tie(&cout);
cerr.flags(ios_base::unitbuf);

@@ -197,14 +193,10 @@
new (&buf_wcout) stdio_filebuf<wchar_t>(stdout, ios_base::out, __out_size);
new (&buf_wcin) stdio_filebuf<wchar_t>(stdin, ios_base::in, __in_size);
new (&buf_wcerr) stdio_filebuf<wchar_t>(stderr, ios_base::out, __out_size);
- new (&wcout) wostream(&buf_wcout);
- new (&wcin) wistream(&buf_wcin);
- new (&wcerr) wostream(&buf_wcerr);
- new (&wclog) wostream(&buf_wcerr);
- wcout.init(&buf_wcout, &locale_cache_wcout);
- wcin.init(&buf_wcin, &locale_cache_wcin);
- wcerr.init(&buf_wcerr, &locale_cache_wcerr);
- wclog.init(&buf_wcerr, &locale_cache_wclog);
+ new (&wcout) wostream(&buf_wcout, &locale_cache_wcout);
+ new (&wcin) wistream(&buf_wcin, &locale_cache_wcin);
+ new (&wcerr) wostream(&buf_wcerr, &locale_cache_wcerr);
+ new (&wclog) wostream(&buf_wcerr, &locale_cache_wclog);
wcin.tie(&wcout);
wcerr.flags(ios_base::unitbuf);
#endif
@@ -339,7 +331,18 @@

void
ios_base::register_callback(event_callback __fn, int __index)
- { _M_callbacks = new _Callback_list(__fn, __index, _M_callbacks); }
+ {
+ // Deal with static standard streams. For these, we didn't want
+ // Callback_list objects to be created on the heap. Instead we
+ // use the buffer in the cache.
+ if (__index == 0 && iword(0) == 1)
+ {
+ char* __cb_array = _M_cache()._M_static_cblist;
+ _M_callbacks = new (__cb_array) _Callback_list(__fn, __index, _M_callbacks);
+ }
+ else
+ _M_callbacks = new _Callback_list(__fn, __index, _M_callbacks);
+ }

void
ios_base::_M_call_callbacks(event __e) throw()
@@ -362,7 +365,9 @@
while (__p && __p->_M_remove_reference() == 0)
{
_Callback_list* __next = __p->_M_next;
- delete __p;
+ // Don't delete _Callback_list that is part of a static cache.
+ if (!(__p->_M_index == 0 && iword(0) == 1))
+ delete __p;
__p = __next;
}
_M_callbacks = 0;
Benjamin Kosnik
2003-04-29 16:27:02 UTC
Permalink
Jerry, I'm just going to assume you read this list.

I think the end goal should be to have something like this work:

#include <ostream>

using namespace std;

class bufferstream : public ostream
{
// Types.
typedef char char_type;
typedef __locale_cache<char_type> cache_type;

// Data.
//cache_type data;

public:

bufferstream() : ostream()
{
// Zero alloc.
//this->init(NULL, &data);
this->init(NULL);
}
};

However, the problem is more than allocating the __locale_cache, it's
what is inside of the __locale_cache.

Fixing that problem is what I thought you were up to.... however, no
worries. Now that the gcc-3.3 deadline is looming, let's try to get this
wrapped up.
Post by Jerry Quinn
Oops. The following patch moves the hook to pass the static cache to
the stream constructors. It passes make check-abi, but is it actually
safe?
Additional non-virtual functions and be appropriately versioned.
Post by Jerry Quinn
Umm, ICK! Actually, there is another leak waiting to happen here,
unrelated to locale cache. The above leak happens because the
destructors are never called for the static streams. If someone
happens to store a new'd object in e.g. cout.pword(10) and register a
callback to clean up the storage, this will never get cleaned up.
This scenario makes me think that we MUST call the standard stream
destructors.
No. See libstdc++/10132 for more info on why this is not possible.

-benjamin
Jerry Quinn
2003-04-30 02:35:35 UTC
Permalink
Post by Benjamin Kosnik
Jerry, I'm just going to assume you read this list.
#include <ostream>
using namespace std;
class bufferstream : public ostream
{
// Types.
typedef char char_type;
typedef __locale_cache<char_type> cache_type;
// Data.
//cache_type data;
bufferstream() : ostream()
{
// Zero alloc.
//this->init(NULL, &data);
this->init(NULL);
}
};
However, the problem is more than allocating the __locale_cache, it's
what is inside of the __locale_cache.
Fixing that problem is what I thought you were up to.... however, no
worries. Now that the gcc-3.3 deadline is looming, let's try to get this
wrapped up.
OK, I had originaly thought the goal was to have the standard streams
never call new. Making the above work is similar, but not identical.
The above will work as is on 3.3 even if the cache has strings in it.

To get to zero alloc, we need to remove any calls to new when a locale
cache is provided, right? The last patch I sent eliminates the
allocation within register_callback. Now we have to eliminate the
strings in the cache.

As I mentioned in another email, I'm not familiar with allocators, so
I'm not sure about using a custom allocator to handle string memory in
the cache. I'm working on a straight pointer solution, but a simple
string wrapper would be nicer.
Post by Benjamin Kosnik
Post by Jerry Quinn
Umm, ICK! Actually, there is another leak waiting to happen here,
unrelated to locale cache. The above leak happens because the
destructors are never called for the static streams. If someone
happens to store a new'd object in e.g. cout.pword(10) and register a
callback to clean up the storage, this will never get cleaned up.
This scenario makes me think that we MUST call the standard stream
destructors.
No. See libstdc++/10132 for more info on why this is not possible.
Double ick. It seems like there's a bit of inconsistency in which
stream ops are supposed to catch exceptions and which ones are not.

The problem I indicated above still exists, though. Is it a defect in
the standard? Even if not, perhaps we should add it as a caveat to
the (nonexistent) pword docs?

Jerry
Benjamin Kosnik
2003-05-01 23:53:23 UTC
Permalink
[problem with adding iword/pword to standard streams not getting deleted]
Post by Jerry Quinn
The problem I indicated above still exists, though. Is it a defect in
the standard? Even if not, perhaps we should add it as a caveat to
the (nonexistent) pword docs?
I don't know if it's a defect, but it might bite people. Let's add it on
page 2 of the imaginary docs!

-benjamin
Jerry Quinn
2003-04-30 03:59:41 UTC
Permalink
Post by Jerry Quinn
There is also another issue flagged by valgrind, but I
==31788== 2304 bytes in 2 blocks are possibly lost in loss record 2 of 2
==31788== at 0x401651B8: malloc (vg_clientfuncs.c:103)
==31788== by 0x808B1BE: operator new(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x8079996: std::__new_alloc::allocate(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x80795DE: std::__default_alloc_template<true, 0>::_S_chunk_alloc(unsigned, int&) (in /home/jlquinn/gcc/test/a.out)
With the patch in
http://gcc.gnu.org/ml/libstdc++/2003-04/msg00443.html, I still have
the above issue. I went ahead and remove strings from the locale
cache, using a wrapper around string buffers. Even after doing that,
I still have the above allocations.

I finally traced this to __locale_cache:_M_init. When
filling out the cache, there is a call to numpunct::truename(), which
is required to create a temporary string. In general, this can't be
bypassed, because it calls a virtual function that does an unknown
computation to generate this string. Only if there is no subclassing
can we avoid the creation of the string here.

To get around this, we either have to trash the cache (I REALLY hope
this doesn't happen), or restrict zero alloc to posix locale and hope
for the best.

At this point, I'm stymied. Help?

Jerry
Jerry Quinn
2003-04-30 04:58:00 UTC
Permalink
Post by Jerry Quinn
Post by Jerry Quinn
There is also another issue flagged by valgrind, but I
==31788== 2304 bytes in 2 blocks are possibly lost in loss record 2 of 2
==31788== at 0x401651B8: malloc (vg_clientfuncs.c:103)
==31788== by 0x808B1BE: operator new(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x8079996: std::__new_alloc::allocate(unsigned) (in /home/jlquinn/gcc/test/a.out)
==31788== by 0x80795DE: std::__default_alloc_template<true, 0>::_S_chunk_alloc(unsigned, int&) (in /home/jlquinn/gcc/test/a.out)
With the patch in
http://gcc.gnu.org/ml/libstdc++/2003-04/msg00443.html, I still have
the above issue. I went ahead and remove strings from the locale
cache, using a wrapper around string buffers. Even after doing that,
I still have the above allocations.
I finally traced this to __locale_cache:_M_init. When
filling out the cache, there is a call to numpunct::truename(), which
is required to create a temporary string. In general, this can't be
bypassed, because it calls a virtual function that does an unknown
computation to generate this string. Only if there is no subclassing
can we avoid the creation of the string here.
To get around this, we either have to trash the cache (I REALLY hope
this doesn't happen), or restrict zero alloc to posix locale and hope
for the best.
At this point, I'm stymied. Help?
Jerry
I just want to point out that the leak in the original PR is now
plugged. What is still left is trying to make no calls to new. So
perhaps 3.3 release doesn't have to be gated by this issue.

Jerry
Carlo Wood
2003-04-30 10:29:44 UTC
Permalink
Post by Jerry Quinn
I just want to point out that the leak in the original PR is now
plugged. What is still left is trying to make no calls to new. So
perhaps 3.3 release doesn't have to be gated by this issue.
I think I can come up with a workaround for libcwd
provided that any allocations done are NOT allocated with
the standard allocator (thus: not using std::string!).
Otherwise there is no way for me to get things working
in the threaded case.

When only calls to 'new' and/or 'malloc' are made then
its possible to get things working - when these allocations
are also deleted again before the function returns (temporary
allocations thus) then there is hardly any problem.

I'd really like this to be fixed - therefore, can you send
me the diff relative to the current CVS that you have so far,
OR apply the patch to CVS, and then tell me the source file
locations where std::string is being used that need to be
eliminated? Then I'll have a shot at it.

Thanks,
--
Carlo Wood <***@alinoe.com>

PS When is the deadline for 3.3?
Jerry Quinn
2003-04-30 13:23:20 UTC
Permalink
Post by Carlo Wood
Post by Jerry Quinn
I just want to point out that the leak in the original PR is now
plugged. What is still left is trying to make no calls to new. So
perhaps 3.3 release doesn't have to be gated by this issue.
I think I can come up with a workaround for libcwd
provided that any allocations done are NOT allocated with
the standard allocator (thus: not using std::string!).
Otherwise there is no way for me to get things working
in the threaded case.
When only calls to 'new' and/or 'malloc' are made then
its possible to get things working - when these allocations
are also deleted again before the function returns (temporary
allocations thus) then there is hardly any problem.
I'd really like this to be fixed - therefore, can you send
me the diff relative to the current CVS that you have so far,
OR apply the patch to CVS, and then tell me the source file
locations where std::string is being used that need to be
eliminated? Then I'll have a shot at it.
Here's what I have at the moment. It includes the changes in
http://gcc.gnu.org/ml/libstdc++/2003-04/msg00443.html.
On top of that, I cobbled up a simple string-like wrapper for char
bufs that includes non-allocated storage for up to 8 chars -
sufficient for POSIX true, false, and grouping.

This is (obviously :-) NOT in finished form.

Jerry


Index: config/linker-map.gnu
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/config/linker-map.gnu,v
retrieving revision 1.25.2.7
diff -u -r1.25.2.7 linker-map.gnu
--- config/linker-map.gnu 7 Mar 2003 22:04:18 -0000 1.25.2.7
+++ config/linker-map.gnu 30 Apr 2003 13:16:31 -0000
@@ -374,6 +374,9 @@

_ZNKSt7num_putI[wc]St19ostreambuf_iteratorI[wc]St11char_traitsI[wc]EEE12_M_group_int*;

+ _ZNSt9basic_iosI[cw]St11char_traitsI[cw]EE4initEPSt15basic_streambufI[cw]S1_EPSt14__locale_cacheI[cw]E;
+
+
# vtable
_ZTVSt19__locale_cache_base;
_ZTVSt14__locale_cacheI[cw]E;
Index: include/bits/basic_ios.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.h,v
retrieving revision 1.14.2.4
diff -u -r1.14.2.4 basic_ios.h
--- include/bits/basic_ios.h 22 Apr 2003 15:40:34 -0000 1.14.2.4
+++ include/bits/basic_ios.h 30 Apr 2003 13:16:31 -0000
@@ -425,8 +425,11 @@
* memory.
*/
void
+ init(basic_streambuf<_CharT, _Traits>* __sb);
+
+ void
init(basic_streambuf<_CharT, _Traits>* __sb,
- __locale_cache<_CharT>* __cache=0);
+ __locale_cache<_CharT>* __cache);

bool
_M_check_facet(const locale::facet* __f) const
Index: include/bits/basic_ios.tcc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.tcc,v
retrieving revision 1.17.4.3
diff -u -r1.17.4.3 basic_ios.tcc
--- include/bits/basic_ios.tcc 22 Apr 2003 15:40:34 -0000 1.17.4.3
+++ include/bits/basic_ios.tcc 30 Apr 2003 13:16:31 -0000
@@ -144,6 +144,11 @@

template<typename _CharT, typename _Traits>
void
+ basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb)
+ { init(__sb, 0); }
+
+ template<typename _CharT, typename _Traits>
+ void
basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb,
__locale_cache<_CharT>* __cache)
{
@@ -196,7 +201,7 @@
// be deleted.
if (__cache)
{
- pword(0) = auto_ptr<__cache_t>(new (__cache) __cache_t()).release();
+ pword(0) = auto_ptr<__cache_t>(new (__cache) __cache_t(true)).release();
iword(0) = 1; // so we don't try to clobber static cache
}
else
Index: include/bits/ios_base.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/ios_base.h,v
retrieving revision 1.21.2.5
diff -u -r1.21.2.5 ios_base.h
--- include/bits/ios_base.h 5 Mar 2003 19:09:45 -0000 1.21.2.5
+++ include/bits/ios_base.h 30 Apr 2003 13:16:32 -0000
@@ -376,6 +376,8 @@
iostate _M_streambuf_state;
//@}

+ friend class __locale_cache_base;
+
// 27.4.2.6 Members for callbacks
// 27.4.2.6 ios_base callbacks
struct _Callback_list
Index: include/bits/locale_facets.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/locale_facets.h,v
retrieving revision 1.49.4.6
diff -u -r1.49.4.6 locale_facets.h
--- include/bits/locale_facets.h 5 Mar 2003 04:40:07 -0000 1.49.4.6
+++ include/bits/locale_facets.h 30 Apr 2003 13:16:32 -0000
@@ -845,6 +845,9 @@
do_get(iter_type, iter_type, ios_base&, ios_base::iostate&, bool&) const;
#endif

+ template <typename _CharT>
+ class __string;
+
template<typename _CharT, typename _OutIter>
class num_put : public locale::facet, public __num_base
{
@@ -902,7 +905,8 @@
char __mod, _ValueT __v) const;

void
- _M_group_float(const string& __grouping, char_type __sep,
+// _M_group_float(const string& __grouping, char_type __sep,
+ _M_group_float(const __string<char>& __grouping, char_type __sep,
const char_type* __p, char_type* __new, char_type* __cs,
int& __len) const;

@@ -912,7 +916,8 @@
_ValueT __v) const;

void
- _M_group_int(const string& __grouping, char_type __sep,
+// _M_group_int(const string& __grouping, char_type __sep,
+ _M_group_int(const __string<char>& __grouping, char_type __sep,
ios_base& __io, char_type* __new, char_type* __cs,
int& __len) const;

@@ -1964,11 +1969,66 @@
// calling the virtual functions in locale facets.
class __locale_cache_base
{
+ friend class ios_base;
+
+ // Used to provide space for a callback list entry for static caches.
+ char _M_static_cblist[sizeof(ios_base::_Callback_list)];
+
public:
virtual
~__locale_cache_base() {}
};

+ // Simple string wrapper. Assumes strings are null-terminated
+ // as necessary.
+ template <typename _CharT>
+ class __string
+ {
+ _CharT _M_buf[8];
+ _CharT* _M_bufptr;
+ int _M_buflen;
+ int _M_len;
+ public:
+ __string() : _M_bufptr(_M_buf), _M_buflen(8), _M_len(0) { _M_buf[0] = 0; }
+ __string(const _CharT* __s, int __len)
+ : _M_bufptr(_M_buf), _M_buflen(8), _M_len(__len)
+ { assign(__s, __len); }
+ __string(const __string& __s)
+ : _M_bufptr(_M_buf), _M_buflen(8), _M_len(__len)
+ { assign(__s._M_bufptr, __s._M_len); }
+ ~__string() { if (_M_bufptr != _M_buf) delete[] _M_bufptr; }
+
+ __string& operator=(const __string& __s)
+ {
+ assign(__s._M_bufptr, __s._M_len);
+ return *this;
+ }
+ __string& operator=(const basic_string<_CharT>& __s)
+ {
+ assign(__s.c_str(), __s.length()+1);
+ return *this;
+ }
+
+ void assign(const _CharT* __s, int __len)
+ {
+ if (__len >= _M_buflen) {
+ if (_M_bufptr != _M_buf)
+ delete[] _M_bufptr;
+ _M_bufptr = new _CharT[__len+1];
+ _M_buflen = __len+1;
+ }
+ memcpy(_M_bufptr, __s, sizeof(_CharT)*(__len+1));
+ _M_len = __len;
+ }
+
+ int length() const { return _M_len; }
+ int size() const { return _M_len; }
+
+ const _CharT* c_str() const { return _M_bufptr; }
+ _CharT operator[](int __index) const { return _M_bufptr[__index]; }
+
+ };
+
template<typename _CharT>
class __locale_cache : public __locale_cache_base
{
@@ -1977,6 +2037,14 @@
typedef char_traits<_CharT> traits_type;
typedef basic_string<_CharT> string_type;

+ // Used to play ugly games for static standard streams
+ int _M_is_static;
+
+ _CharT _M_true_buf[8];
+ _CharT _M_false_buf[8];
+ _CharT _M_group_buf[8];
+
+ public:

public:
// Data Members:
@@ -2001,8 +2069,10 @@

// However the US's "false" and "true" are translated.
// From numpunct::truename() and numpunct::falsename(), respectively.
- string_type _M_truename;
- string_type _M_falsename;
+// string_type _M_truename;
+// string_type _M_falsename;
+ __string<_CharT> _M_truename;
+ __string<_CharT> _M_falsename;

// If we are checking groupings. This should be equivalent to
// numpunct::groupings().size() != 0
@@ -2010,10 +2080,18 @@

// If we are using numpunct's groupings, this is the current grouping
// string in effect (from numpunct::grouping()).
- string _M_grouping;
+ // string _M_grouping;
+ __string<char> _M_grouping;

- __locale_cache() : _M_use_grouping(false)
+ __locale_cache(bool __is_static=false) :
+// _M_is_static(__is_static), _M_truename(_M_true_buf), _M_falsename(_M_false_buf),
+// _M_use_grouping(false), _M_grouping(_M_group_buf)
+ _M_is_static(__is_static),_M_use_grouping(false)
{ };
+
+// ~__locale_cache()
+// {
+// }

__locale_cache&
operator=(const __locale_cache& __lc);
Index: include/bits/locale_facets.tcc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/locale_facets.tcc,v
retrieving revision 1.82.4.9
diff -u -r1.82.4.9 locale_facets.tcc
--- include/bits/locale_facets.tcc 22 Apr 2003 15:40:34 -0000 1.82.4.9
+++ include/bits/locale_facets.tcc 30 Apr 2003 13:16:33 -0000
@@ -730,7 +730,7 @@
template<typename _CharT, typename _OutIter>
void
num_put<_CharT, _OutIter>::
- _M_group_int(const string& __grouping, _CharT __sep, ios_base& __io,
+ _M_group_int(const __string<char>& __grouping, _CharT __sep, ios_base& __io,
_CharT* __new, _CharT* __cs, int& __len) const
{
// By itself __add_grouping cannot deal correctly with __ws when
@@ -816,7 +816,7 @@
template<typename _CharT, typename _OutIter>
void
num_put<_CharT, _OutIter>::
- _M_group_float(const string& __grouping, _CharT __sep, const _CharT* __p,
+ _M_group_float(const __string<char>& __grouping, _CharT __sep, const _CharT* __p,
_CharT* __new, _CharT* __cs, int& __len) const
{
#ifdef _GLIBCPP_RESOLVE_LIB_DEFECTS
@@ -977,15 +977,20 @@
{
typedef __locale_cache<_CharT> __cache_type;
__cache_type& __lc = static_cast<__cache_type&>(__io._M_cache());
- typedef basic_string<_CharT> __string_type;
- __string_type __name;
- if (__v)
- __name = __lc._M_truename;
- else
- __name = __lc._M_falsename;
+// typedef basic_string<_CharT> __string_type;
+// __string_type __name;
+// if (__v)
+// __name = __lc._M_truename;
+// else
+// __name = __lc._M_falsename;

+// const _CharT* __cs = __name.c_str();
+// int __len = __name.size();
+
+ __string<_CharT>& __name = __v ? __lc._M_truename : __lc._M_falsename;
const _CharT* __cs = __name.c_str();
int __len = __name.size();
+
_CharT* __cs3;
streamsize __w = __io.width();
if (__w > static_cast<streamsize>(__len))
@@ -2275,7 +2280,8 @@
_M_decimal_point = __np.decimal_point();
_M_grouping = __np.grouping();
_M_use_grouping = _M_grouping.size() != 0
- && _M_grouping.data()[0] != 0;
+ && _M_grouping[0] != 0;
+// && _M_grouping.data()[0] != 0;
}
if (__builtin_expect(has_facet<ctype<_CharT> >(__loc), true))
{
Index: include/std/std_istream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_istream.h,v
retrieving revision 1.5.2.2
diff -u -r1.5.2.2 std_istream.h
--- include/std/std_istream.h 23 Apr 2003 16:44:15 -0000 1.5.2.2
+++ include/std/std_istream.h 30 Apr 2003 13:16:33 -0000
@@ -107,6 +107,12 @@
_M_gcount = streamsize(0);
}

+ basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc)
+ {
+ this->init(__sb, __lc);
+ _M_gcount = streamsize(0);
+ }
+
/**
* @brief Base destructor.
*
Index: include/std/std_ostream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_ostream.h,v
retrieving revision 1.5.2.1
diff -u -r1.5.2.1 std_ostream.h
--- include/std/std_ostream.h 9 Mar 2003 02:00:53 -0000 1.5.2.1
+++ include/std/std_ostream.h 30 Apr 2003 13:16:33 -0000
@@ -105,6 +105,9 @@
basic_ostream(__streambuf_type* __sb)
{ this->init(__sb); }

+ basic_ostream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc)
+ { this->init(__sb, __lc); }
+
/**
* @brief Base destructor.
*
Index: src/ios.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/src/ios.cc,v
retrieving revision 1.33.2.6
diff -u -r1.33.2.6 ios.cc
--- src/ios.cc 22 Apr 2003 15:40:34 -0000 1.33.2.6
+++ src/ios.cc 30 Apr 2003 13:16:34 -0000
@@ -182,14 +182,10 @@
new (&buf_cin) stdio_filebuf<char>(stdin, ios_base::in, __in_size);
new (&buf_cerr) stdio_filebuf<char>(stderr, ios_base::out, __out_size);

- new (&cout) ostream(&buf_cout);
- new (&cin) istream(&buf_cin);
- new (&cerr) ostream(&buf_cerr);
- new (&clog) ostream(&buf_cerr);
- cout.init(&buf_cout, &locale_cache_cout);
- cin.init(&buf_cin, &locale_cache_cin);
- cerr.init(&buf_cerr, &locale_cache_cerr);
- clog.init(&buf_cerr, &locale_cache_clog);
+ new (&cout) ostream(&buf_cout, &locale_cache_cout);
+ new (&cin) istream(&buf_cin, &locale_cache_cin);
+ new (&cerr) ostream(&buf_cerr, &locale_cache_cerr);
+ new (&clog) ostream(&buf_cerr, &locale_cache_clog);
cin.tie(&cout);
cerr.flags(ios_base::unitbuf);

@@ -197,14 +193,10 @@
new (&buf_wcout) stdio_filebuf<wchar_t>(stdout, ios_base::out, __out_size);
new (&buf_wcin) stdio_filebuf<wchar_t>(stdin, ios_base::in, __in_size);
new (&buf_wcerr) stdio_filebuf<wchar_t>(stderr, ios_base::out, __out_size);
- new (&wcout) wostream(&buf_wcout);
- new (&wcin) wistream(&buf_wcin);
- new (&wcerr) wostream(&buf_wcerr);
- new (&wclog) wostream(&buf_wcerr);
- wcout.init(&buf_wcout, &locale_cache_wcout);
- wcin.init(&buf_wcin, &locale_cache_wcin);
- wcerr.init(&buf_wcerr, &locale_cache_wcerr);
- wclog.init(&buf_wcerr, &locale_cache_wclog);
+ new (&wcout) wostream(&buf_wcout, &locale_cache_wcout);
+ new (&wcin) wistream(&buf_wcin, &locale_cache_wcin);
+ new (&wcerr) wostream(&buf_wcerr, &locale_cache_wcerr);
+ new (&wclog) wostream(&buf_wcerr, &locale_cache_wclog);
wcin.tie(&wcout);
wcerr.flags(ios_base::unitbuf);
#endif
@@ -339,7 +331,18 @@

void
ios_base::register_callback(event_callback __fn, int __index)
- { _M_callbacks = new _Callback_list(__fn, __index, _M_callbacks); }
+ {
+ // Deal with static standard streams. For these, we didn't want
+ // Callback_list objects to be created on the heap. Instead we
+ // use the buffer in the cache.
+ if (__index == 0 && iword(0) == 1)
+ {
+ char* __cb_array = _M_cache()._M_static_cblist;
+ _M_callbacks = new (__cb_array) _Callback_list(__fn, __index, _M_callbacks);
+ }
+ else
+ _M_callbacks = new _Callback_list(__fn, __index, _M_callbacks);
+ }

void
ios_base::_M_call_callbacks(event __e) throw()
@@ -362,7 +365,9 @@
while (__p && __p->_M_remove_reference() == 0)
{
_Callback_list* __next = __p->_M_next;
- delete __p;
+ // Don't delete _Callback_list that is part of a static cache.
+ if (!(__p->_M_index == 0 && iword(0) == 1))
+ delete __p;
__p = __next;
}
_M_callbacks = 0;
Jerry Quinn
2003-05-01 06:07:31 UTC
Permalink
Post by Jerry Quinn
I finally traced this to __locale_cache:_M_init. When
filling out the cache, there is a call to numpunct::truename(), which
is required to create a temporary string. In general, this can't be
bypassed, because it calls a virtual function that does an unknown
computation to generate this string. Only if there is no subclassing
can we avoid the creation of the string here.
To get around this, we either have to trash the cache (I REALLY hope
this doesn't happen), or restrict zero alloc to posix locale and hope
for the best.
At this point, I'm stymied. Help?
OK, I've gotten further. The following patch works with zero
allocation. But it has some other testsuite regressions. I'm going to
try to squish those tomorrow (too late now).

As before, it's not cleaned up, but comments are still welcome :-)

Jerry


Index: config/linker-map.gnu
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/config/linker-map.gnu,v
retrieving revision 1.25.2.7
diff -u -r1.25.2.7 linker-map.gnu
--- config/linker-map.gnu 7 Mar 2003 22:04:18 -0000 1.25.2.7
+++ config/linker-map.gnu 1 May 2003 05:47:38 -0000
@@ -374,6 +374,9 @@

_ZNKSt7num_putI[wc]St19ostreambuf_iteratorI[wc]St11char_traitsI[wc]EEE12_M_group_int*;

+ _ZNSt9basic_iosI[cw]St11char_traitsI[cw]EE4initEPSt15basic_streambufI[cw]S1_EPSt14__locale_cacheI[cw]E;
+
+
# vtable
_ZTVSt19__locale_cache_base;
_ZTVSt14__locale_cacheI[cw]E;
Index: include/bits/basic_ios.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.h,v
retrieving revision 1.14.2.4
diff -u -r1.14.2.4 basic_ios.h
--- include/bits/basic_ios.h 22 Apr 2003 15:40:34 -0000 1.14.2.4
+++ include/bits/basic_ios.h 1 May 2003 05:47:40 -0000
@@ -425,8 +425,11 @@
* memory.
*/
void
+ init(basic_streambuf<_CharT, _Traits>* __sb);
+
+ void
init(basic_streambuf<_CharT, _Traits>* __sb,
- __locale_cache<_CharT>* __cache=0);
+ __locale_cache<_CharT>* __cache);

bool
_M_check_facet(const locale::facet* __f) const
@@ -437,7 +440,7 @@
}

void
- _M_cache_locale(const locale& __loc,__locale_cache<_CharT>* __cache = 0);
+ _M_cache_locale(const locale& __loc,__locale_cache<_CharT>* __cache = 0, bool __init=false);

#if 1
// XXX GLIBCXX_ABI Deprecated, compatibility only.
Index: include/bits/basic_ios.tcc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/basic_ios.tcc,v
retrieving revision 1.17.4.3
diff -u -r1.17.4.3 basic_ios.tcc
--- include/bits/basic_ios.tcc 22 Apr 2003 15:40:34 -0000 1.17.4.3
+++ include/bits/basic_ios.tcc 1 May 2003 05:47:40 -0000
@@ -144,12 +144,17 @@

template<typename _CharT, typename _Traits>
void
+ basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb)
+ { init(__sb, 0); }
+
+ template<typename _CharT, typename _Traits>
+ void
basic_ios<_CharT, _Traits>::init(basic_streambuf<_CharT, _Traits>* __sb,
__locale_cache<_CharT>* __cache)
{
// NB: This may be called more than once on the same object.
ios_base::_M_init();
- _M_cache_locale(_M_ios_locale, __cache);
+ _M_cache_locale(_M_ios_locale, __cache, true);
_M_tie = 0;

// NB: The 27.4.4.1 Postconditions Table specifies requirements
@@ -175,7 +180,8 @@
template<typename _CharT, typename _Traits>
void
basic_ios<_CharT, _Traits>::_M_cache_locale(const locale& __loc,
- __locale_cache<_CharT>* __cache)
+ __locale_cache<_CharT>* __cache,
+ bool __init)
{
if (__builtin_expect(has_facet<__ctype_type>(__loc), true))
_M_fctype = &use_facet<__ctype_type>(__loc);
@@ -196,14 +202,14 @@
// be deleted.
if (__cache)
{
- pword(0) = auto_ptr<__cache_t>(new (__cache) __cache_t()).release();
+ pword(0) = auto_ptr<__cache_t>(new (__cache) __cache_t(true)).release();
iword(0) = 1; // so we don't try to clobber static cache
}
else
pword(0) = auto_ptr<__cache_t>(new __cache_t()).release();
register_callback(__cache_t::_S_callback, 0);
}
- static_cast<__cache_t&>(_M_cache())._M_init(__loc);
+ static_cast<__cache_t&>(_M_cache())._M_init(__loc, __init);
}

#if 1
Index: include/bits/ios_base.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/ios_base.h,v
retrieving revision 1.21.2.5
diff -u -r1.21.2.5 ios_base.h
--- include/bits/ios_base.h 5 Mar 2003 19:09:45 -0000 1.21.2.5
+++ include/bits/ios_base.h 1 May 2003 05:47:40 -0000
@@ -376,6 +376,8 @@
iostate _M_streambuf_state;
//@}

+ friend class __locale_cache_base;
+
// 27.4.2.6 Members for callbacks
// 27.4.2.6 ios_base callbacks
struct _Callback_list
Index: include/bits/locale_facets.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/locale_facets.h,v
retrieving revision 1.49.4.6
diff -u -r1.49.4.6 locale_facets.h
--- include/bits/locale_facets.h 5 Mar 2003 04:40:07 -0000 1.49.4.6
+++ include/bits/locale_facets.h 1 May 2003 05:47:40 -0000
@@ -579,6 +579,7 @@
_S_format_int(const ios_base& __io, char* __fptr, char __mod, char __modl);
};

+ template<typename _CharT> class __locale_cache;

template<typename _CharT>
class numpunct : public locale::facet
@@ -590,6 +591,8 @@

static locale::id id;

+ friend class __locale_cache<_CharT>;
+
private:
char_type _M_decimal_point;
char_type _M_thousands_sep;
@@ -845,6 +848,9 @@
do_get(iter_type, iter_type, ios_base&, ios_base::iostate&, bool&) const;
#endif

+ template <typename _CharT>
+ class __string;
+
template<typename _CharT, typename _OutIter>
class num_put : public locale::facet, public __num_base
{
@@ -902,7 +908,8 @@
char __mod, _ValueT __v) const;

void
- _M_group_float(const string& __grouping, char_type __sep,
+// _M_group_float(const string& __grouping, char_type __sep,
+ _M_group_float(const __string<char>& __grouping, char_type __sep,
const char_type* __p, char_type* __new, char_type* __cs,
int& __len) const;

@@ -912,7 +919,8 @@
_ValueT __v) const;

void
- _M_group_int(const string& __grouping, char_type __sep,
+// _M_group_int(const string& __grouping, char_type __sep,
+ _M_group_int(const __string<char>& __grouping, char_type __sep,
ios_base& __io, char_type* __new, char_type* __cs,
int& __len) const;

@@ -1964,11 +1972,77 @@
// calling the virtual functions in locale facets.
class __locale_cache_base
{
+ friend class ios_base;
+
+ // Used to provide space for a callback list entry for static caches.
+ char _M_static_cblist[sizeof(ios_base::_Callback_list)];
+
public:
virtual
~__locale_cache_base() {}
};

+ // Simple string wrapper. Assumes strings are null-terminated
+ // as necessary.
+ template <typename _CharT>
+ class __string
+ {
+ typedef char_traits<_CharT> traits_type;
+
+
+ _CharT _M_buf[8];
+ _CharT* _M_bufptr;
+ int _M_buflen;
+ int _M_len;
+ public:
+ __string() : _M_bufptr(_M_buf), _M_buflen(8), _M_len(0) { _M_buf[0] = 0; }
+ __string(const _CharT* __s)
+ : _M_bufptr(_M_buf), _M_buflen(8), _M_len(traits_type::length(__s))
+ { assign(__s, _M_len); }
+ __string(const _CharT* __s, int __len)
+ : _M_bufptr(_M_buf), _M_buflen(8), _M_len(__len)
+ { assign(__s, __len); }
+ __string(const __string& __s)
+ : _M_bufptr(_M_buf), _M_buflen(8), _M_len(__len)
+ { assign(__s._M_bufptr, __s._M_len); }
+ ~__string() { if (_M_bufptr != _M_buf) delete[] _M_bufptr; }
+
+ __string& operator=(const _CharT* __s)
+ {
+ assign(__s, traits_type::length(__s));
+ return *this;
+ }
+ __string& operator=(const __string& __s)
+ {
+ assign(__s._M_bufptr, __s._M_len);
+ return *this;
+ }
+ __string& operator=(const basic_string<_CharT>& __s)
+ {
+ assign(__s.c_str(), __s.length()+1);
+ return *this;
+ }
+
+ void assign(const _CharT* __s, int __len)
+ {
+ if (__len >= _M_buflen) {
+ if (_M_bufptr != _M_buf)
+ delete[] _M_bufptr;
+ _M_bufptr = new _CharT[__len+1];
+ _M_buflen = __len+1;
+ }
+ memcpy(_M_bufptr, __s, sizeof(_CharT)*(__len+1));
+ _M_len = __len;
+ }
+
+ int length() const { return _M_len; }
+ int size() const { return _M_len; }
+
+ const _CharT* c_str() const { return _M_bufptr; }
+ _CharT operator[](int __index) const { return _M_bufptr[__index]; }
+
+ };
+
template<typename _CharT>
class __locale_cache : public __locale_cache_base
{
@@ -1977,6 +2051,14 @@
typedef char_traits<_CharT> traits_type;
typedef basic_string<_CharT> string_type;

+ // Used to play ugly games for static standard streams
+ int _M_is_static;
+
+ _CharT _M_true_buf[8];
+ _CharT _M_false_buf[8];
+ _CharT _M_group_buf[8];
+
+ public:

public:
// Data Members:
@@ -2001,8 +2083,10 @@

// However the US's "false" and "true" are translated.
// From numpunct::truename() and numpunct::falsename(), respectively.
- string_type _M_truename;
- string_type _M_falsename;
+// string_type _M_truename;
+// string_type _M_falsename;
+ __string<_CharT> _M_truename;
+ __string<_CharT> _M_falsename;

// If we are checking groupings. This should be equivalent to
// numpunct::groupings().size() != 0
@@ -2010,18 +2094,26 @@

// If we are using numpunct's groupings, this is the current grouping
// string in effect (from numpunct::grouping()).
- string _M_grouping;
+ // string _M_grouping;
+ __string<char> _M_grouping;

- __locale_cache() : _M_use_grouping(false)
+ __locale_cache(bool __is_static=false) :
+// _M_is_static(__is_static), _M_truename(_M_true_buf), _M_falsename(_M_false_buf),
+// _M_use_grouping(false), _M_grouping(_M_group_buf)
+ _M_is_static(__is_static), _M_use_grouping(false)
{ };

+// ~__locale_cache()
+// {
+// }
+
__locale_cache&
operator=(const __locale_cache& __lc);


// Make sure the cache is built before the first use.
void
- _M_init(const locale&);
+ _M_init(const locale&, bool);

// ios_base::pword callbacks come here
static void
Index: include/bits/locale_facets.tcc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/locale_facets.tcc,v
retrieving revision 1.82.4.9
diff -u -r1.82.4.9 locale_facets.tcc
--- include/bits/locale_facets.tcc 22 Apr 2003 15:40:34 -0000 1.82.4.9
+++ include/bits/locale_facets.tcc 1 May 2003 05:47:41 -0000
@@ -730,7 +730,7 @@
template<typename _CharT, typename _OutIter>
void
num_put<_CharT, _OutIter>::
- _M_group_int(const string& __grouping, _CharT __sep, ios_base& __io,
+ _M_group_int(const __string<char>& __grouping, _CharT __sep, ios_base& __io,
_CharT* __new, _CharT* __cs, int& __len) const
{
// By itself __add_grouping cannot deal correctly with __ws when
@@ -816,7 +816,7 @@
template<typename _CharT, typename _OutIter>
void
num_put<_CharT, _OutIter>::
- _M_group_float(const string& __grouping, _CharT __sep, const _CharT* __p,
+ _M_group_float(const __string<char>& __grouping, _CharT __sep, const _CharT* __p,
_CharT* __new, _CharT* __cs, int& __len) const
{
#ifdef _GLIBCPP_RESOLVE_LIB_DEFECTS
@@ -977,15 +977,20 @@
{
typedef __locale_cache<_CharT> __cache_type;
__cache_type& __lc = static_cast<__cache_type&>(__io._M_cache());
- typedef basic_string<_CharT> __string_type;
- __string_type __name;
- if (__v)
- __name = __lc._M_truename;
- else
- __name = __lc._M_falsename;
+// typedef basic_string<_CharT> __string_type;
+// __string_type __name;
+// if (__v)
+// __name = __lc._M_truename;
+// else
+// __name = __lc._M_falsename;

+// const _CharT* __cs = __name.c_str();
+// int __len = __name.size();
+
+ __string<_CharT>& __name = __v ? __lc._M_truename : __lc._M_falsename;
const _CharT* __cs = __name.c_str();
int __len = __name.size();
+
_CharT* __cs3;
streamsize __w = __io.width();
if (__w > static_cast<streamsize>(__len))
@@ -2264,18 +2269,32 @@

template<typename _CharT>
void
- __locale_cache<_CharT>::_M_init(const locale& __loc)
+ __locale_cache<_CharT>::_M_init(const locale& __loc, bool __first_init)
{
if (__builtin_expect(has_facet<numpunct<_CharT> >(__loc), true))
{
const numpunct<_CharT>& __np = use_facet<numpunct<_CharT> >(__loc);
- _M_falsename = __np.falsename();
- _M_truename = __np.truename();
- _M_thousands_sep = __np.thousands_sep();
- _M_decimal_point = __np.decimal_point();
- _M_grouping = __np.grouping();
- _M_use_grouping = _M_grouping.size() != 0
- && _M_grouping.data()[0] != 0;
+ if (__builtin_expect(__first_init, false))
+ {
+ _M_truename = __np._M_truename;
+ _M_falsename = __np._M_falsename;
+ _M_thousands_sep = __np._M_thousands_sep;
+ _M_decimal_point = __np._M_decimal_point;
+ _M_grouping = __np._M_grouping;
+ _M_use_grouping = _M_grouping.size() != 0 && _M_grouping[0] != 0;
+ }
+ else
+ {
+ // The normal case
+ _M_falsename = __np.falsename();
+ _M_truename = __np.truename();
+ _M_thousands_sep = __np.thousands_sep();
+ _M_decimal_point = __np.decimal_point();
+ _M_grouping = __np.grouping();
+ _M_use_grouping = _M_grouping.size() != 0
+ && _M_grouping[0] != 0;
+ // && _M_grouping.data()[0] != 0;
+ }
}
if (__builtin_expect(has_facet<ctype<_CharT> >(__loc), true))
{
Index: include/std/std_istream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_istream.h,v
retrieving revision 1.5.2.2
diff -u -r1.5.2.2 std_istream.h
--- include/std/std_istream.h 23 Apr 2003 16:44:15 -0000 1.5.2.2
+++ include/std/std_istream.h 1 May 2003 05:47:41 -0000
@@ -107,6 +107,12 @@
_M_gcount = streamsize(0);
}

+ basic_istream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc)
+ {
+ this->init(__sb, __lc);
+ _M_gcount = streamsize(0);
+ }
+
/**
* @brief Base destructor.
*
Index: include/std/std_ostream.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/std/std_ostream.h,v
retrieving revision 1.5.2.1
diff -u -r1.5.2.1 std_ostream.h
--- include/std/std_ostream.h 9 Mar 2003 02:00:53 -0000 1.5.2.1
+++ include/std/std_ostream.h 1 May 2003 05:47:42 -0000
@@ -105,6 +105,9 @@
basic_ostream(__streambuf_type* __sb)
{ this->init(__sb); }

+ basic_ostream(__streambuf_type* __sb, __locale_cache<_CharT>* __lc)
+ { this->init(__sb, __lc); }
+
/**
* @brief Base destructor.
*
Index: src/ios.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/src/ios.cc,v
retrieving revision 1.33.2.6
diff -u -r1.33.2.6 ios.cc
--- src/ios.cc 22 Apr 2003 15:40:34 -0000 1.33.2.6
+++ src/ios.cc 1 May 2003 05:47:42 -0000
@@ -182,14 +182,10 @@
new (&buf_cin) stdio_filebuf<char>(stdin, ios_base::in, __in_size);
new (&buf_cerr) stdio_filebuf<char>(stderr, ios_base::out, __out_size);

- new (&cout) ostream(&buf_cout);
- new (&cin) istream(&buf_cin);
- new (&cerr) ostream(&buf_cerr);
- new (&clog) ostream(&buf_cerr);
- cout.init(&buf_cout, &locale_cache_cout);
- cin.init(&buf_cin, &locale_cache_cin);
- cerr.init(&buf_cerr, &locale_cache_cerr);
- clog.init(&buf_cerr, &locale_cache_clog);
+ new (&cout) ostream(&buf_cout, &locale_cache_cout);
+ new (&cin) istream(&buf_cin, &locale_cache_cin);
+ new (&cerr) ostream(&buf_cerr, &locale_cache_cerr);
+ new (&clog) ostream(&buf_cerr, &locale_cache_clog);
cin.tie(&cout);
cerr.flags(ios_base::unitbuf);

@@ -197,14 +193,10 @@
new (&buf_wcout) stdio_filebuf<wchar_t>(stdout, ios_base::out, __out_size);
new (&buf_wcin) stdio_filebuf<wchar_t>(stdin, ios_base::in, __in_size);
new (&buf_wcerr) stdio_filebuf<wchar_t>(stderr, ios_base::out, __out_size);
- new (&wcout) wostream(&buf_wcout);
- new (&wcin) wistream(&buf_wcin);
- new (&wcerr) wostream(&buf_wcerr);
- new (&wclog) wostream(&buf_wcerr);
- wcout.init(&buf_wcout, &locale_cache_wcout);
- wcin.init(&buf_wcin, &locale_cache_wcin);
- wcerr.init(&buf_wcerr, &locale_cache_wcerr);
- wclog.init(&buf_wcerr, &locale_cache_wclog);
+ new (&wcout) wostream(&buf_wcout, &locale_cache_wcout);
+ new (&wcin) wistream(&buf_wcin, &locale_cache_wcin);
+ new (&wcerr) wostream(&buf_wcerr, &locale_cache_wcerr);
+ new (&wclog) wostream(&buf_wcerr, &locale_cache_wclog);
wcin.tie(&wcout);
wcerr.flags(ios_base::unitbuf);
#endif
@@ -339,7 +331,18 @@

void
ios_base::register_callback(event_callback __fn, int __index)
- { _M_callbacks = new _Callback_list(__fn, __index, _M_callbacks); }
+ {
+ // Deal with static standard streams. For these, we didn't want
+ // Callback_list objects to be created on the heap. Instead we
+ // use the buffer in the cache.
+ if (__index == 0 && iword(0) == 1)
+ {
+ char* __cb_array = _M_cache()._M_static_cblist;
+ _M_callbacks = new (__cb_array) _Callback_list(__fn, __index, _M_callbacks);
+ }
+ else
+ _M_callbacks = new _Callback_list(__fn, __index, _M_callbacks);
+ }

void
ios_base::_M_call_callbacks(event __e) throw()
@@ -362,7 +365,9 @@
while (__p && __p->_M_remove_reference() == 0)
{
_Callback_list* __next = __p->_M_next;
- delete __p;
+ // Don't delete _Callback_list that is part of a static cache.
+ if (!(__p->_M_index == 0 && iword(0) == 1))
+ delete __p;
__p = __next;
}
_M_callbacks = 0;
Benjamin Kosnik
2003-05-01 22:24:08 UTC
Permalink
For some reason, my mail is not showing up on this list today.
I'm not quite sure what to do about this. I'm not super happy about
__string. No doubt, you are not pleased with this either. This seems
mighty hacky: the iword/pword stuff seems less so, since these are
reserved slots.
I'm trying to come up with an external allocator, so that you can just do
string<char, char_traits<char>, __extern_alloc> for this stuff.
I think that would be better.
I'll work on this today, and let you know where I am tonight. If this
cannot be resolved, I'm going to suggest that we temporarily back out
the locale cache from 3.3.0 until this has been satisfactorily resolved.
I'm sure this is a big bummer to you, and it is to me as well. The other
speed improvements are still in 3.3.0 though, so it won't be quite as
dire as 3.2.x. Thoughts?
I'm also having second thoughts about this approach to locale caching in
general. I've implemented __has_cache/__use_cache and while it's not as fast
as the __locale_cache code as present, it's still on par with 2.95 and
it seems like a better way to go.
Awful lot of "I's" in the above text. Your honest feedback is appreciated.
Also, perhaps this has been confused in the past: can you make sure that
3.3 check-ins pass "make check-abi" before checking in, or explicitly
note that on the patch? If this is unclear, please let me know how the
docs can be improved to better reflect reality. As I pointed out, there
is still work to be done in check-abi.
I just fixed the check-abi issues, so perhaps this will be clearer to
everybody now.

This patch is going to be reverted on the 3.3 branch. There will be time
to fix this properly for the next release.

-benjamin
Carlo Wood
2003-05-01 23:18:40 UTC
Permalink
Post by Benjamin Kosnik
This patch is going to be reverted on the 3.3 branch. There will be time
to fix this properly for the next release.
Till when do I have time to fix this?
When is the deadline for 3.3? I asked this before.

If _creating_ a basic_ios<> will use std::string in 3.3,
then that will break libcwd - I'd not be happy with that :(

Writing an allocator that uses preallocated stack space
of 8 bytes would be very easy to write. Please let me
know if I can help and what is the deadline for this.
--
Carlo Wood <***@alinoe.com>
Benjamin Kosnik
2003-05-01 23:27:05 UTC
Permalink
Post by Carlo Wood
Till when do I have time to fix this?
We don't have time to fix this. The release is tomorrow.

-benjamin
B. Kosnik
2003-05-01 19:51:55 UTC
Permalink
Jerry.

I'm not quite sure what to do about this. I'm not super happy about
__string. No doubt, you are not pleased with this either. This seems
mighty hacky: the iword/pword stuff seems less so, since these are
reserved slots.

I'm trying to come up with an external allocator, so that you can just do

string<char, char_traits<char>, __extern_alloc> for this stuff.

I think that would be better.

I'll work on this today, and let you know where I am tonight. If this
cannot be resolved, I'm going to suggest that we temporarily back out
the locale cache from 3.3.0 until this has been satisfactorily resolved.
I'm sure this is a big bummer to you, and it is to me as well. The other
speed improvements are still in 3.3.0 though, so it won't be quite as
dire as 3.2.x. Thoughts?

I'm also having second thoughts about this approach to locale caching in
general. I've implemented __has_cache/__use_cache and while it's not as fast
as the __locale_cache code as present, it's still on par with 2.95 and
it seems like a better way to go.

Awful lot of "I's" in the above text. Your honest feedback is appreciated.

Also, perhaps this has been confused in the past: can you make sure that
3.3 check-ins pass "make check-abi" before checking in, or explicitly
note that on the patch? If this is unclear, please let me know how the
docs can be improved to better reflect reality. As I pointed out, there
is still work to be done in check-abi.

-benjamin
Jerry Quinn
2003-05-02 03:56:40 UTC
Permalink
I See I am too late for 3.3.0 :-( Unfortunately, I haven't had the
bandwidth to sort this out fast enough for the release. So my
apologies for ending up on the critical path.

That said ...
Post by B. Kosnik
Jerry.
I'm not quite sure what to do about this. I'm not super happy about
__string. No doubt, you are not pleased with this either. This seems
mighty hacky: the iword/pword stuff seems less so, since these are
reserved slots.
You're right, I wasn't very happy with it. __string was just a
wrapper for the char buffer manipulations that were going to have to
happen anyway with this approach. I jut thought it was less ugly to
wrap it up.
Post by B. Kosnik
I'm trying to come up with an external allocator, so that you can just do
string<char, char_traits<char>, __extern_alloc> for this stuff.
I think that would be better.
I'll work on this today, and let you know where I am tonight. If this
cannot be resolved, I'm going to suggest that we temporarily back out
the locale cache from 3.3.0 until this has been satisfactorily resolved.
I'm sure this is a big bummer to you, and it is to me as well. The other
speed improvements are still in 3.3.0 though, so it won't be quite as
dire as 3.2.x. Thoughts?
Yeah, that's very disappointing. With v3 as I just checked it out,
3.3 is still more than 2X slower than 2.95 on writing ints to file.
But we're making forward progress.
Post by B. Kosnik
I'm also having second thoughts about this approach to locale caching in
general. I've implemented __has_cache/__use_cache and while it's not as fast
as the __locale_cache code as present, it's still on par with 2.95 and
it seems like a better way to go.
Awful lot of "I's" in the above text. Your honest feedback is appreciated.
I'd rather see it in there than not. For sure, the ABI compatibility
puts some limitations on what we can do. And I agree that the pword
method has some unpleasant aspects to it. Also, we need something
like the __has_cache approach to deal with the problem pointed out by
Petur in this thread:
http://gcc.gnu.org/ml/libstdc++/2003-04/msg00023.html

Will the __has_cache approach be ABI compatible and avoid the
allocation issues that are sinking the current approach?

I remember suggesting that the pword mechanism might be used to hold
this stuff if ABI concerns pop up again. Although this will again
require fiddling with the static streams.

I'd like to see the patch. I'm surprised you are getting speeds
equivalent to 2.95.
Post by B. Kosnik
Also, perhaps this has been confused in the past: can you make sure that
3.3 check-ins pass "make check-abi" before checking in, or explicitly
note that on the patch? If this is unclear, please let me know how the
docs can be improved to better reflect reality. As I pointed out, there
is still work to be done in check-abi.
I thought I mostly had (once I was aware of it), but I'll be more
explicit and careful about it in the future.

Jerry
Jerry Quinn
2003-05-02 04:49:16 UTC
Permalink
Post by Jerry Quinn
I'm also having second thoughts about this approach to locale caching in
general. I've implemented __has_cache/__use_cache and while it's not as fast
as the __locale_cache code as present, it's still on par with 2.95 and
it seems like a better way to go.
Awful lot of "I's" in the above text. Your honest feedback is appreciated.
I'd rather see it in there than not. For sure, the ABI compatibility
puts some limitations on what we can do. And I agree that the pword
method has some unpleasant aspects to it. Also, we need something
like the __has_cache approach to deal with the problem pointed out by
http://gcc.gnu.org/ml/libstdc++/2003-04/msg00023.html
I actually see that PR 9828 is still broken on 3.3, even with the
locale cache scrapped.

Ouch! I just tried to look at it in gdb, and I can no longer use gdb
within the library itself. I was using the approach of rebuilding
just the library with:

make clean
make CXXFLAGS=-g3 install

after which i could step into library code. After updating, I can't
seem to do it anymore.

Anyone else having similar difficulty now?

Jerry
B. Kosnik
2003-05-02 05:25:15 UTC
Permalink
Post by Jerry Quinn
I actually see that PR 9828 is still broken on 3.3, even with the
locale cache scrapped.
Huh. It works for me. (Even if it didn't, this would be fixed in 3.3.1...)
Post by Jerry Quinn
Ouch! I just tried to look at it in gdb, and I can no longer use gdb
within the library itself. I was using the approach of rebuilding
make clean
make CXXFLAGS=-g3 install
after which i could step into library code. After updating, I can't
seem to do it anymore.
Anyone else having similar difficulty now?
Usually I use '-g -O0' for CXXFLAGS. I'm using mainline and current
binutils and gdb, and managed to step through this.

-benjamin
Jerry Quinn
2003-05-02 05:50:44 UTC
Permalink
Post by B. Kosnik
Post by Jerry Quinn
I actually see that PR 9828 is still broken on 3.3, even with the
locale cache scrapped.
Huh. It works for me. (Even if it didn't, this would be fixed in 3.3.1...)
Post by Jerry Quinn
Ouch! I just tried to look at it in gdb, and I can no longer use gdb
within the library itself. I was using the approach of rebuilding
make clean
make CXXFLAGS=-g3 install
after which i could step into library code. After updating, I can't
seem to do it anymore.
Anyone else having similar difficulty now?
Usually I use '-g -O0' for CXXFLAGS. I'm using mainline and current
binutils and gdb, and managed to step through this.
Ugh! Ignore this nonsense (mine, that is). Debian installed a
libstdc++3.3 library and I didn't have LD_LIBRARY_PATH set right.

9828 works for me too. This is the problem with doing most of my gcc
after 11pm :-)

Jerry

B. Kosnik
2003-05-02 05:34:06 UTC
Permalink
Post by Jerry Quinn
I See I am too late for 3.3.0 :-( Unfortunately, I haven't had the
bandwidth to sort this out fast enough for the release. So my
apologies for ending up on the critical path.
No worries. Now we are free to work it out without the pressure of
looming deadlines.
Post by Jerry Quinn
Yeah, that's very disappointing. With v3 as I just checked it out,
3.3 is still more than 2X slower than 2.95 on writing ints to file.
But we're making forward progress.
Yep.
Post by Jerry Quinn
I'm also having second thoughts about this approach to locale caching in
general. I've implemented __has_cache/__use_cache and while it's not as fast
as the __locale_cache code as present, it's still on par with 2.95 and
it seems like a better way to go.
Awful lot of "I's" in the above text. Your honest feedback is appreciated.
I'd rather see it in there than not. For sure, the ABI compatibility
puts some limitations on what we can do. And I agree that the pword
method has some unpleasant aspects to it. Also, we need something
like the __has_cache approach to deal with the problem pointed out by
http://gcc.gnu.org/ml/libstdc++/2003-04/msg00023.html
Yeah.
Post by Jerry Quinn
Will the __has_cache approach be ABI compatible and avoid the
allocation issues that are sinking the current approach?
I remember suggesting that the pword mechanism might be used to hold
this stuff if ABI concerns pop up again. Although this will again
require fiddling with the static streams.
Uncertain. I liked the pword mechanism for fitting this stuff back into
3.2.x abi, actually. It seems to work pretty well.
Post by Jerry Quinn
I'd like to see the patch. I'm surprised you are getting speeds
equivalent to 2.95.
I've gotten better results with mainline than v2 for a while now. This
is on athlon/p4/p3.

I'll try to clean it up enough to post. Some critical bits remain
unresolved, and I just hacked through all the ctype issues instead of
dealing with them correctly. I was just looking at num_put.

RH9, T30 (P4/1.8G/512) 2.96
6.630u 0.420s 0:09.40 75.0% 0+0k 0+0io 137pf+0w

RH9, T30 (P4/1.8G/512) 20030429-gcc-sh.exe
5.880u 0.410s 0:09.83 63.9% 0+0k 0+0io 215pf+0w

RH9, T30 (P4/1.8G/512) 20030429-gcc-static.exe NO __locale_cache
12.610u 0.390s 0:18.70 69.5% 0+0k 0+0io 122pf+0w

RH9, T30 (P4/1.8G/512) 20030429-gcc-3_3-branch.exe __use_cache
7.000u 0.300s 0:07.67 95.1% 0+0k 0+0io 124pf+0w
Post by Jerry Quinn
I thought I mostly had (once I was aware of it), but I'll be more
explicit and careful about it in the future.
Yep. I fixed up 'check-abi' for both added symbols and missing symbols.

-benjamin
Benjamin Kosnik
2003-04-03 16:09:20 UTC
Permalink
Post by Jerry Quinn
OK, the reason we're getting memory leaks is that the locale cache was never
getting deleted. The callback got registered within the first call to
basic_ios::init, but the callback list was blown away by the second call,
hence no deletion.
Thanks for looking at this.
Post by Jerry Quinn
This doesn't address the zero allocation issues so we still leak the bytes for
the standard stream caches.
Yes. I'm currently knee-deep in dejagnu surgery and can't look at this
at the moment. I'm anticipating a fix for 3.3/3.4 in a bit.
Post by Jerry Quinn
* src/ios.cc (ios_base::_M_init): Remove _M_callbacks initialization.
please put this in 3.3, and 3.4.

thanks!

-benjamin
Loading...