Thread: ECPG - Specifying connections, TSD, sqlca.
Shridhar, Once the patches I've put forward are applied there's still a further change I've got planned which will remove the mutex locking in the common case - a NULL/DEFAULT connection parameter (I'll post a patch soon). This leaves the threaded case with comparable performance to the non-threaded case (both have a function call to get the connection, plus a getspecific call in the threaded case). As such is there much benefit in adding support for the connection being supplied by a struct pointer? You'd also have to add in something like "EXEC SQL GET DESCRIPTION xxx" to get the pointer too. How would it improve things over how they are in the test_thread_implicit test program? I still think it's worthwhile investigating the use of GCC's __thread storage class specifier to remove the use of pthread_*specific in this case. This would also be a help to the WIN32 port since this specifier maps well to similar constructs in Microsoft's and Borland's compilers (see "thread" item in the TODO at developer.postgresql.org). And I still can't see how you'll bind sqlca to the connection object, but best of luck! Regards, Lee K.
Lee Kindness wrote: > Shridhar, Once the patches I've put forward are applied there's still > a further change I've got planned which will remove the mutex locking > in the common case - a NULL/DEFAULT connection parameter (I'll post a > patch soon). This leaves the threaded case with comparable performance > to the non-threaded case (both have a function call to get the > connection, plus a getspecific call in the threaded case). As such is > there much benefit in adding support for the connection being supplied > by a struct pointer? You'd also have to add in something like "EXEC > SQL GET DESCRIPTION xxx" to get the pointer too. How would it improve > things over how they are in the test_thread_implicit test program? > > I still think it's worthwhile investigating the use of GCC's __thread > storage class specifier to remove the use of pthread_*specific in this > case. This would also be a help to the WIN32 port since this specifier > maps well to similar constructs in Microsoft's and Borland's compilers > (see "thread" item in the TODO at developer.postgresql.org). I would like to avoid compiler-specific thread stuff unless tests can show a performance benefit. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Lee Kindness wrote: > From: "Bruce Momjian" <pgman@candle.pha.pa.us> > > Lee Kindness wrote: > > > I still think it's worthwhile investigating the use of GCC's __thread > > > storage class specifier to remove the use of pthread_*specific in this > > > case. This would also be a help to the WIN32 port since this specifier > > > maps well to similar constructs in Microsoft's and Borland's compilers > > > (see "thread" item in the TODO at developer.postgresql.org). > > I would like to avoid compiler-specific thread stuff unless tests can > > show a performance benefit. > > I think concerns re performance are largely moot - with the thread specific > data performance is much the same as without. However native compiler > support for thread specific data is much more straightforward and > understandable - you simply end up with a variable that can be used like any > other except there is a copy of it for each thread. > > To make ECPG thread-safe for WIN32 an additional set of thread calls will > need to be added, and/or similar features to GCC's __thread storage > specifier. If we end up adding these for WIN32 then it may as well be done > for GCC too. I probably will experiment with it a bit (and get some real > performance figure, rather than my hunch!)... > > Perhaps a cleaner way is to use an existing thread package with encompasses > the various platform APIs - i.e. APR or ACE, or... But that's a big > discussion, and not one I'm keen to get into at the moment. A more > appropriate time is perhaps once the WIN32 port is completed? It would also > be straightforward to encompass this in an PostgreSQL specific API to wrap > around the various calls we use and, if available, make these no-ops when a > suitable storage class is supplied by the compiler? I'd be happy to write > this API if others saw it as a way forward. > > Perhaps someone would like to fwd this on to the hackers-win32 list (I'm not > subscribed) to see what their view is on thread safety in the client > libraries? And what approach they're planning? I guess my point was that if there isn't a big performance win, why do compiler specific and POSIX standard both in the same code. The compiler-specific might be clearer, but if we have to support non-gcc too, which we do, adding a cleaner solution to one that is already standard actually makes it look worse. I don't think MinGW support thread-specific right now (no TLS support), so we will need native Win32 in there anyway. Adding a third seems like more confusion. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
From: "Bruce Momjian" <pgman@candle.pha.pa.us> > Lee Kindness wrote: > > I still think it's worthwhile investigating the use of GCC's __thread > > storage class specifier to remove the use of pthread_*specific in this > > case. This would also be a help to the WIN32 port since this specifier > > maps well to similar constructs in Microsoft's and Borland's compilers > > (see "thread" item in the TODO at developer.postgresql.org). > I would like to avoid compiler-specific thread stuff unless tests can > show a performance benefit. I think concerns re performance are largely moot - with the thread specific data performance is much the same as without. However native compiler support for thread specific data is much more straightforward and understandable - you simply end up with a variable that can be used like any other except there is a copy of it for each thread. To make ECPG thread-safe for WIN32 an additional set of thread calls will need to be added, and/or similar features to GCC's __thread storage specifier. If we end up adding these for WIN32 then it may as well be done for GCC too. I probably will experiment with it a bit (and get some real performance figure, rather than my hunch!)... Perhaps a cleaner way is to use an existing thread package with encompasses the various platform APIs - i.e. APR or ACE, or... But that's a big discussion, and not one I'm keen to get into at the moment. A more appropriate time is perhaps once the WIN32 port is completed? It would also be straightforward to encompass this in an PostgreSQL specific API to wrap around the various calls we use and, if available, make these no-ops when a suitable storage class is supplied by the compiler? I'd be happy to write this API if others saw it as a way forward. Perhaps someone would like to fwd this on to the hackers-win32 list (I'm not subscribed) to see what their view is on thread safety in the client libraries? And what approach they're planning? L.
From: "Bruce Momjian" <pgman@candle.pha.pa.us> > Lee Kindness wrote: > > Perhaps a cleaner way is to use an existing thread package with encompasses > > the various platform APIs - i.e. APR or ACE, or... But that's a big > > discussion, and not one I'm keen to get into at the moment. A more > > appropriate time is perhaps once the WIN32 port is completed? It would also > > be straightforward to encompass this in an PostgreSQL specific API to wrap > > around the various calls we use and, if available, make these no-ops when a > > suitable storage class is supplied by the compiler? I'd be happy to write > > this API if others saw it as a way forward. > > > I don't think MinGW support thread-specific right now (no TLS support), > so we will need native Win32 in there anyway. Adding a third seems like > more confusion. Ah, ok - i've not been following the win32 stuff so wasn't even sure on compilers being used. I'd agree at this stage there's no point muddying the waters even further! I'll get back to you with the patch to move common-case connection retrieval outwith the mutex once the earlier patches are applied. Thanks, L.