From: <dan...@ya...> - 2001-11-28 10:04:36
|
Thomas I'm having second thoughts on your patch to mingw runtime thread support lib and corresponding patch to libgcc.a I modified your testcase to print out mem stats (using GlobalMemoryStatus) after every 1000 calls to _beginthreadex/CloseHandle. What I show below is sum of MEMORYSTATUS.dwAvailPhys + MEMORYSTATUS.dwAvailPageFile. This is just a prelim look, I have repeated this several times with similar results but have not done proper statistical tests. Your patch does indeed fix a memory leak with 2.95.3. However, there does not appear to be any leak (using your testcase) on 3.1, with or without your patch. I'm guessing that EH code cleans up after itself better in GCC 3.1. Is patch necessary or beneficial for other reasons? Can you comment on performance costs of your patch versus Mumit's version? Without your patches (mingw runtime 1.1 release and unpatched gthr-win32.h) --------------------------------------------- 2.95.3 3.1 (CVS) --------------- --------------- Calls Avail Delta Avail Delta mem(kb) mem(kb) ----- -------------- --------------- 0 115220 0 116132 0 1000 115200 -20 116136 4 2000 115072 -148 116136 4 3000 115056 -164 116128 -4 4000 114932 -288 116128 -4 5000 114916 -304 116152 20 6000 114784 -436 116152 20 7000 114776 -444 116136 4 8000 114660 -560 116136 4 9000 114660 -560 116200 68 10000 114528 -692 116200 68 11000 114588 -632 116200 68 12000 114436 -784 116216 84 13000 114436 -784 116212 80 14000 114308 -912 116180 48 15000 114324 -896 116188 56 16000 114196 -1024 116180 48 17000 114196 -1024 116172 40 18000 114068 -1152 116176 44 19000 113964 -1256 116176 44 20000 113964 -1256 116216 84 21000 113836 -1384 116212 80 22000 113836 -1384 116212 80 23000 113708 -1512 116220 88 24000 113708 -1512 116220 88 25000 113580 -1640 116212 80 26000 113580 -1640 116200 68 27000 113452 -1768 116192 60 28000 113452 -1768 116192 60 29000 113324 -1896 116192 60 30000 113316 -1904 116164 32 31000 113188 -2032 116172 40 -------------------------------------------- With your patches to mingw runtime and gthr-win32.h --------------------------------------------- 2.95.3 3.1 (CVS) --------------- --------------- Calls Avail Delta Avail Delta mem(kb) mem(kb) ----- -------------- --------------- 0 115976 0 117044 0 1000 115964 -12 117032 -12 2000 116016 40 117032 -12 3000 116012 36 117032 -12 4000 116012 36 117032 -12 5000 116020 44 117032 -12 6000 116020 44 117032 -12 7000 116020 44 117064 20 8000 116008 32 117064 20 9000 116052 76 117064 20 10000 116052 76 117064 20 11000 116052 76 117024 -20 12000 116060 84 117024 -20 13000 116048 72 117024 -20 14000 116032 56 117024 -20 15000 116032 56 117100 56 16000 116040 64 117076 32 17000 116040 64 117076 32 18000 116040 64 117092 48 19000 116064 88 117092 48 20000 116064 88 117060 16 21000 116064 88 117060 16 22000 116064 88 117108 64 23000 116056 80 117108 64 24000 116056 80 117108 64 25000 116056 80 117100 56 26000 116056 80 117108 64 27000 116056 80 117108 64 28000 116056 80 117028 -16 29000 116056 80 117020 -24 30000 116064 88 117012 -32 31000 116056 80 116980 -64 32000 116056 80 116980 -64 33000 116056 80 117012 -32 34000 116056 80 117012 -32 --------------------------------------------- --- Thomas Pfaff <tp...@gm...> wrote: > Hi Danny, > > > On Wed, 21 Nov 2001, Danny Smith wrote: > > > Thomas > > > > Could you please have a look at this and perhap strengthen the > rationale > > and I will submit to GCC-patches. > > > > > > > > In earlier versions of the mingw runtime C++ multithread support for > > C++ exceptions,the key/dtor code was thread dependent. This can > > lead to memory leaks, since only the first dying thread will have its > > additional memory for exception handling freed. > > > > Thomas Pfaff has provided a patch to the mingw runtime that corrects > this > > problem. The following patch to the gthr-win32.h complements that > patch, > > by letting __gthread_key_dtor act like the posix version during sjlj > chain > > unwinding. > > > > The patch also cleans up some problems with prototypes when included > > from stl_threads.h and removes a duplicate include of _mingw.h > > > > Bootstrapped and tested on i586-pc-mingw32 configured as > > ../gcc/configure > --with-gcc-version-trigger=/develop/gcc/gcc/gcc/version.c > > --with-gnu-ld --with-gnu-as --host=i586-pc-mingw32 > > --target=i586--pc-mingw32 --prefix=/mingw --enable-sjlj-exceptions > > --enable-threads --disable-nls --enable-languages=c++,f77 > > --disable-win32-registry > > > > > > 2001-11-21 Thomas Pfaff <tp...@gm...> > > > > * gthr-win32.h (__gthread_key_dtor): Let it act like the posix > > version in gthr-posix.h > > (__ghthread_key_delete): Call __mingwthr_remove_key_dtor to > > remove the key from the key/dtor list. > > > > 2001-11-21 Mumit Khan <kh...@xr...> > > > > * gthr-win32.h: Update copyright statement. > > Wrap prototypes for external mingw runtime functions > > in extern "C" if __cplusplus. > > (_mingw.h): Remove second include. > > > > > > Index: gcc/gcc/gthr-win32.h > > =================================================================== > > RCS file: /cvs/gcc/gcc/gcc/gthr-win32.h,v > > retrieving revision 1.11 > > diff -u -p -r1.11 gthr-win32.h > > --- gthr-win32.h 2001/10/12 13:10:33 1.11 > > +++ gthr-win32.h 2001/11/19 20:49:38 > > @@ -1,6 +1,6 @@ > > /* Threads compatibility routines for libgcc2 and libobjc. */ > > /* Compile this one with gcc. */ > > -/* Copyright (C) 1999, 2000 Free Software Foundation, Inc. > > +/* Copyright (C) 1999, 2000, 2001 Free Software Foundation, Inc. > > Contributed by Mumit Khan <kh...@xr...>. > > > > This file is part of GCC. > > @@ -320,10 +322,6 @@ __gthread_objc_condition_signal(objc_con > > > > #else /* _LIBOBJC */ > > > > -#ifdef __MINGW32__ > > -#include <_mingw.h> > > -#endif > > - > > typedef DWORD __gthread_key_t; > > > > typedef struct { > > @@ -339,7 +337,14 @@ typedef HANDLE __gthread_mutex_t; > > #if __MINGW32_MAJOR_VERSION >= 1 || \ > > (__MINGW32_MAJOR_VERSION == 0 && __MINGW32_MINOR_VERSION > 2) > > #define MINGW32_SUPPORTS_MT_EH 1 > > -extern int __mingwthr_key_dtor PARAMS ((DWORD, void (*) (void *))); > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > +extern int __mingwthr_key_dtor (DWORD, void (*) (void *)); > > +extern int __mingwthr_remove_key_dtor (DWORD); > > +#ifdef __cplusplus > > +} > > +#endif > > /* Mingw runtime >= v0.3 provides a magic variable that is set to > non-zero > > if -mthreads option was specified, or 0 otherwise. This is to get > > around > > the lack of weak symbols in PE-COFF. */ > > @@ -410,16 +415,26 @@ __gthread_key_create (__gthread_key_t *k > > > > /* Currently, this routine is called only for Mingw runtime, and if > > -mthreads option is chosen to link in the thread support DLL. */ > > + > > static inline int > > -__gthread_key_dtor (__gthread_key_t key, void *ptr) > > -{ > > - /* Nothing needed. */ > > - return 0; > > -} > > +__gthread_setspecific (__gthread_key_t key, const void *ptr); > > > > static inline int > > +__gthread_key_dtor (__gthread_key_t key, void *ptr) > > + { > > + /* Just reset the key value to zero. */ > > + if (ptr) > > + return __gthread_setspecific(key, 0); > > + else > > + return 0; /* Nothing needed. */ > > +} > > + > > + static inline int > > __gthread_key_delete (__gthread_key_t key) > > { > > +#ifdef MINGW32_SUPPORTS_MT_EH > > + __mingwthr_remove_key_dtor(key); > > +#endif > > return (TlsFree (key) != 0) ? 0 : (int) GetLastError (); > > } > > > > > > > > > > > > http://shopping.yahoo.com.au - Yahoo! Shopping > > - Get organised for Christmas early this year! > > > http://shopping.yahoo.com.au - Yahoo! Shopping - Get organised for Christmas early this year! |
From: Thomas P. <tp...@gm...> - 2001-11-28 11:59:05
|
On Wed, 28 Nov 2001, Danny Smith wrote: > Thomas > > I'm having second thoughts on your patch to mingw runtime thread support > lib and corresponding patch to libgcc.a > > I modified your testcase to print out mem stats (using > GlobalMemoryStatus) after every 1000 calls to _beginthreadex/CloseHandle. > What I show below is sum of MEMORYSTATUS.dwAvailPhys + > MEMORYSTATUS.dwAvailPageFile. > > This is just a prelim look, I have repeated this several times with > similar results but have not done proper statistical tests. > > Your patch does indeed fix a memory leak with 2.95.3. However, there does > not appear to be any leak (using your testcase) on 3.1, with or without > your patch. I'm guessing that EH code cleans up after itself better in > GCC 3.1. If you can give me an ftp access to your gcc-3.1 snapshot i will take a look at it. > > Is patch necessary or beneficial for other reasons? Can you comment on > performance costs of your patch versus Mumit's version? > This patch was only meant to create TLS keys similar to pthread_key_create, where you can give a dtor function that will be called on thread termination. Look at Mumits comments in mthr.c Unfortunately his implementation was not correct and has lead to memory leaks. Therefore it does not matter which code is faster (i guess Mumits is because free() is only called once ;-) ). If my patch is not neccessary with gcc-3.1 you can forget Mumits version as well. You can easily try this if you link the program without libmingthrd.a. If there is no differende than the mthr code is simply not needed (at least whith thread exception cleanup). |
From: <dan...@ya...> - 2001-11-28 20:55:34
|
--- Thomas Pfaff <tp...@gm...> wrote: > > > On Wed, 28 Nov 2001, Danny Smith wrote: > > > Your patch does indeed fix a memory leak with 2.95.3. However, there > does > > not appear to be any leak (using your testcase) on 3.1, with or without > > your patch. I'm guessing that EH code cleans up after itself better > in > > GCC 3.1. > > If you can give me an ftp access to your gcc-3.1 snapshot i will take a > look at it. > Sorry gcc core and g++ src about 12MB gzipped. My connection is very slow. I won't be uploading 3.1 srcs and binaries anywhere until I'm sure I won't have to do it again soon. I can send gcc/gthr-win32.h and gcc/unwind-sjlj.c (where libgcc's generic exception code lives) or you can grab those from gcc CVS > > > > Is patch necessary or beneficial for other reasons? Can you comment on > > performance costs of your patch versus Mumit's version? > > > > This patch was only meant to create TLS keys similar to > pthread_key_create, where you can give a dtor function that will be > called > on thread termination. Look at Mumits comments in mthr.c > Unfortunately his implementation was not correct and has lead to memory > leaks. Therefore it does not matter which code is faster (i guess Mumits > is because free() is only called once ;-) ). > > If my patch is not neccessary with gcc-3.1 you can forget Mumits version > as well. You can easily try this if you link the program without > libmingthrd.a. If there is no differende than the mthr code is simply not > needed (at least whith thread exception cleanup). > I have tried 3.1 without -mthreads switch and these are results calls free delta mem 0 101912 0 1000 101872 -40 2000 101924 12 3000 101924 12 4000 101916 4 5000 101908 -4 6000 101936 24 7000 101928 16 8000 101928 16 9000 101928 16 10000 101936 24 11000 101936 24 12000 101928 16 13000 101928 16 14000 101876 -36 15000 101876 -36 16000 101848 -64 17000 101856 -56 18000 101856 -56 19000 101828 -84 20000 101828 -84 21000 101864 -48 22000 101864 -48 23000 101856 -56 24000 101876 -36 25000 101868 -44 26000 101868 -44 27000 101868 -44 28000 101884 -28 29000 101884 -28 No apparent leak I need to study unwind-sjlj.c some more to follow what is actually happening I won't push the patch to gthr-win32.h for gcc-3.1 foe awhile. Feel free to do so yourself if you can answer the questions that will come back from that list. Please understand that I am not criticising your efforts. I am just having trouble with what is for me a very steep learning curve. Danny > > _______________________________________________ > MinGW-dvlpr mailing list > Min...@li... > https://lists.sourceforge.net/lists/listinfo/mingw-dvlpr http://shopping.yahoo.com.au - Yahoo! Shopping - Get organised for Christmas early this year! |
From: Thomas P. <tp...@gm...> - 2001-11-29 13:08:53
|
On Thu, 29 Nov 2001, Danny Smith wrote: > --- Thomas Pfaff <tp...@gm...> wrote: > > > > > On Wed, 28 Nov 2001, Danny Smith wrote: > > > > > Your patch does indeed fix a memory leak with 2.95.3. However, there > > does > > > not appear to be any leak (using your testcase) on 3.1, with or without > > > your patch. I'm guessing that EH code cleans up after itself better > > in > > > GCC 3.1. > > > > If you can give me an ftp access to your gcc-3.1 snapshot i will take a > > look at it. > > > > Sorry gcc core and g++ src about 12MB gzipped. My connection is very slow. > I won't be uploading 3.1 srcs and binaries anywhere until I'm sure I won't > have to do it again soon. I can send gcc/gthr-win32.h and > gcc/unwind-sjlj.c (where libgcc's generic exception code lives) or you can > grab those from gcc CVS I just downloaded gcc-snapshot 20011126 (i can not access the CVS repositorie due to a firewall that blocks CVS). Maybe you can generate a diff with your changes as i do not expect to get mingw support out of the box (correct me if i am wrong and the snapshot is up to date). > > > > > > Is patch necessary or beneficial for other reasons? Can you comment on > > > performance costs of your patch versus Mumit's version? > > > > > > > This patch was only meant to create TLS keys similar to > > pthread_key_create, where you can give a dtor function that will be > > called > > on thread termination. Look at Mumits comments in mthr.c > > Unfortunately his implementation was not correct and has lead to memory > > leaks. Therefore it does not matter which code is faster (i guess Mumits > > is because free() is only called once ;-) ). > > > > If my patch is not neccessary with gcc-3.1 you can forget Mumits version > > as well. You can easily try this if you link the program without > > libmingthrd.a. If there is no differende than the mthr code is simply not > > needed (at least whith thread exception cleanup). > > > > I have tried 3.1 without -mthreads switch and these are results > > calls free delta > mem > 0 101912 0 > 28000 101884 -28 > 29000 101884 -28 > > No apparent leak > > I need to study unwind-sjlj.c some more to follow what is actually > happening > > I won't push the patch to gthr-win32.h for gcc-3.1 foe awhile. Feel free > to do so yourself if you can answer the questions that will come back from > that list. Please understand that I am not criticising your efforts. I am > just having trouble with what is for me a very steep learning curve. > I have spent many hours to find a bug in gthread_getspecific that has cleared the WIN32 LastError (by TlsGetValue). I guess my learning curve was the same as yours. I do not think that the patch for gthr-win32 is really neccessary but it would make it complete and i will submit the patch to gcc when you release the new mingw runtime. |
From: <dan...@ya...> - 2001-11-29 20:45:48
|
--- Thomas Pfaff <tp...@gm...> wrote: > > > On Thu, 29 Nov 2001, Danny Smith wrote: > > > --- Thomas Pfaff <tp...@gm...> wrote: > > > > > > > On Wed, 28 Nov 2001, Danny Smith wrote: > > > > > > > Your patch does indeed fix a memory leak with 2.95.3. However, > there > > > does > > > > not appear to be any leak (using your testcase) on 3.1, with or > without > > > > your patch. I'm guessing that EH code cleans up after itself > better > > > in > > > > GCC 3.1. > > > > > > If you can give me an ftp access to your gcc-3.1 snapshot i will take > a > > > look at it. > > > > > > > Sorry gcc core and g++ src about 12MB gzipped. My connection is very > slow. > > I won't be uploading 3.1 srcs and binaries anywhere until I'm sure I > won't > > have to do it again soon. I can send gcc/gthr-win32.h and > > gcc/unwind-sjlj.c (where libgcc's generic exception code lives) or you > can > > grab those from gcc CVS > > I just downloaded gcc-snapshot 20011126 (i can not access the CVS > repositorie due to a firewall that blocks CVS). Maybe you can generate a > diff with your changes as i do not expect to get mingw support out of the > box (correct me if i am wrong and the snapshot is up to date). > I wish you were wrong. I have many patches to go in (mainly contribs from other people like yourself) but not enough time. I'll generate a diff in a day or two ( a recent change to GCC configure has just broken mingw build, I've submitted patch but needs revision -- will generate diff once that is settled.) Is anyone else on the mingw-devlpr list interested in this diff? (Please say yes) > > > > > > > > Is patch necessary or beneficial for other reasons? Can you comment > on > > > > performance costs of your patch versus Mumit's version? > > > > > > > > > > This patch was only meant to create TLS keys similar to > > > pthread_key_create, where you can give a dtor function that will be > > > called > > > on thread termination. Look at Mumits comments in mthr.c > > > Unfortunately his implementation was not correct and has lead to > memory > > > leaks. Therefore it does not matter which code is faster (i guess > Mumits > > > is because free() is only called once ;-) ). > > > > > > If my patch is not neccessary with gcc-3.1 you can forget Mumits > version > > > as well. You can easily try this if you link the program without > > > libmingthrd.a. If there is no differende than the mthr code is simply > not > > > needed (at least whith thread exception cleanup). > > > > > > > I have tried 3.1 without -mthreads switch and these are results > > > > calls free delta > > mem > > 0 101912 0 > > 28000 101884 -28 > > 29000 101884 -28 > > > > No apparent leak > > > > I need to study unwind-sjlj.c some more to follow what is actually > > happening > > > > I won't push the patch to gthr-win32.h for gcc-3.1 foe awhile. Feel > free > > to do so yourself if you can answer the questions that will come back > from > > that list. Please understand that I am not criticising your efforts. > I am > > just having trouble with what is for me a very steep learning curve. > > > > I have spent many hours to find a bug in gthread_getspecific that has > cleared the WIN32 LastError (by TlsGetValue). I guess my learning curve > was the same as yours. I do not think that the patch for gthr-win32 is > really neccessary but it would make it complete and i will submit the > patch to gcc when you release the new mingw runtime. > Aha, I don't mind that kind of leverage :). Earnie, any idea on mingw-runtime-1.2 release? Danny http://shopping.yahoo.com.au - Yahoo! Shopping - Get organised for Christmas early this year! |
From: Earnie B. <ear...@ya...> - 2001-11-29 21:51:00
|
Danny Smith wrote: > > > > > I have spent many hours to find a bug in gthread_getspecific that has > > cleared the WIN32 LastError (by TlsGetValue). I guess my learning curve > > was the same as yours. I do not think that the patch for gthr-win32 is > > really neccessary but it would make it complete and i will submit the > > patch to gcc when you release the new mingw runtime. > > > > Aha, I don't mind that kind of leverage :). Earnie, any idea on > mingw-runtime-1.2 release? > It's on my todo list. I need to merge changes between Cygwin and MinGW. I plan to do that by Monday or Tuesday. Earnie. _________________________________________________________ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com |
From: <dan...@ya...> - 2001-12-05 09:45:23
Attachments:
20011205.diff.gz
|
--- Thomas Pfaff <tp...@gm...> wrote: > > > On Thu, 29 Nov 2001, Danny Smith wrote: > > > --- Thomas Pfaff <tp...@gm...> wrote: > > > > > > > On Wed, 28 Nov 2001, Danny Smith wrote: > > > > > I just downloaded gcc-snapshot 20011126 (i can not access the CVS > repositorie due to a firewall that blocks CVS). Maybe you can generate a > diff with your changes as i do not expect to get mingw support out of the > box (correct me if i am wrong and the snapshot is up to date). > > Sorry about the delay. Attached is diff of my sandbox against CVS. Looking at it I don't think any of the C code diffs are necessary for successful build of working compiler. Some of the makefile.in changes may be (depending on how clever your version of make is and whether or not you want to mimic current mingw tree structure on install). You may need to regenerate configure from configure.in. Here is my configure #!/bin/sh ../gcc/configure --with-gcc-version-trigger=/develop/gcc/gcc/gcc/version.c --with-gcc --with-gnu-ld --with-gnu-as --host=mingw32 --target=mingw32 --prefix=/mingw --enable-sjlj-exceptions --enable-threads --disable-nls --enable-languages=ada,f77,c++ --disable-win32-registry --enable-shared Take out the ada unless you have a working gnat and gnatbind. I do not think that the patch for gthr-win32 is > really neccessary but it would make it complete and i will submit the > patch to gcc when you release the new mingw runtime. > The patch http://gcc.gnu.org/ml/gcc-patches/2001-11/msg01455.html has been sitting unreviewed on gcc-patches since 22 Nov. Probably time for a ping. > _______________________________________________ > MinGW-dvlpr mailing list > Min...@li... > https://lists.sourceforge.net/lists/listinfo/mingw-dvlpr http://shopping.yahoo.com.au - Yahoo! Shopping - Get organised for Christmas early this year! |