SEO
vBulletin Search Engine Optimization
| |||||||
| Register | FAQ | Members List | Calendar | Search | Today's Posts | Mark Forums Read |
04-19-2008, 06:28 AM
| ||||
| ||||
 Re: libpq Win32 Mutex performance patch Andrew Chernow wrote: > I noticed several months ago, and came across it again today, that > libpq's pthread-win32.c implementation is using CreateMutex rather > than CRITICAL_SECTION. CreateMutex is like a semaphore in that it is > designed to be accessible via name system-wide. Even when you don't > give it a name, thus bound to process that created it, it still > carries significant overhead compared to using win32 > CRITICAL_SECTIONs. > > The attached patch replaces the win32 mutex calls with critical > section calls. The change will not affect the behavior of the > windows pthread_xxx functions. First of all, I like this in general :-) But a couple of comments. It changes the behavior when the pointer passed in is invalid from crash to silent working, right? This shouldn't actually matter, since these functions are only ever supposed to run from callers *inside libpq*, so it probalby doesn't matter... Which brings up the second point - is there any actual reason for adding the pthread_mutex_destroy call? Since libpq never calls it, and it's never used from outside libpq (it's not exported outside the library even), isn't it just going to end up as dead code? //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches      |
|
04-19-2008, 06:28 AM
| |||
| |||
| Re: libpq Win32 Mutex performance patch Magnus Hagander wrote: >It changes the behavior when the pointer passed in is invalid from >crash to silent working, right? Correct, it a Habit. I sub-consciously write code that checks pointers. We can remove the pointer checks and let the thing dump core if people prefer. > Which brings up the second point - is there any actual reason for > adding the pthread_mutex_destroy call? Since libpq never calls it, and > it's never used from outside libpq (it's not exported outside the > library even), isn't it just going to end up as dead code? > > //Magnus > The destroy call is within a comment. I only put it there in case it is ever needeed. BTW, I just noticed the commented destroy call forgot to free(*mp) ... ooppssseee. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches |
|
04-19-2008, 06:28 AM
| |||
| |||
| Re: libpq Win32 Mutex performance patch On Fri, Apr 11, 2008 at 2:49 PM, Magnus Hagander <magnus@hagander.net> wrote: > Andrew Chernow wrote: > > I noticed several months ago, and came across it again today, that > > libpq's pthread-win32.c implementation is using CreateMutex rather > > than CRITICAL_SECTION. CreateMutex is like a semaphore in that it is > > designed to be accessible via name system-wide. Even when you don't > > give it a name, thus bound to process that created it, it still > > carries significant overhead compared to using win32 > > CRITICAL_SECTIONs. > > > > The attached patch replaces the win32 mutex calls with critical > > section calls. The change will not affect the behavior of the > > windows pthread_xxx functions. > > First of all, I like this in general :-) But a couple of comments. > > It changes the behavior when the pointer passed in is invalid from > crash to silent working, right? This shouldn't actually matter, > since these functions are only ever supposed to run from callers > *inside libpq*, so it probalby doesn't matter... I noticed you conjured up a ecpg threading patch sometime around early 2007. You used a mutex there deliberately because that's what libpq did. Maybe that patch should be adjusted? merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches |
|
04-19-2008, 06:28 AM
| |||
| |||
| Re: libpq Win32 Mutex performance patch Andrew Chernow wrote: > Magnus Hagander wrote: > > >It changes the behavior when the pointer passed in is invalid from > >crash to silent working, right? > > Correct, it a Habit. I sub-consciously write code that checks > pointers. We can remove the pointer checks and let the thing dump > core if people prefer. Actually, if we can avoid it being a pointer at all, that'd be even better :-) Because if we don't send a pointer we have to allocate, then the critical section functions have the same properties as the pthread ones, namely they cannot fail. Any chance for that way instead? //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches |
|
04-19-2008, 06:28 AM
| ||||
| ||||
| Re: libpq Win32 Mutex performance patch Merlin Moncure wrote: > On Fri, Apr 11, 2008 at 2:49 PM, Magnus Hagander > <magnus@hagander.net> wrote: > > Andrew Chernow wrote: > > > I noticed several months ago, and came across it again today, > > > that libpq's pthread-win32.c implementation is using CreateMutex > > > rather than CRITICAL_SECTION. CreateMutex is like a semaphore > > > in that it is designed to be accessible via name system-wide. > > > Even when you don't give it a name, thus bound to process that > > > created it, it still carries significant overhead compared to > > > using win32 CRITICAL_SECTIONs. > > > > > > The attached patch replaces the win32 mutex calls with critical > > > section calls. The change will not affect the behavior of the > > > windows pthread_xxx functions. > > > > First of all, I like this in general :-) But a couple of comments. > > > > It changes the behavior when the pointer passed in is invalid from > > crash to silent working, right? This shouldn't actually matter, > > since these functions are only ever supposed to run from callers > > *inside libpq*, so it probalby doesn't matter... > > I noticed you conjured up a ecpg threading patch sometime around early > 2007. You used a mutex there deliberately because that's what libpq > did. Maybe that patch should be adjusted? Yes, I think it should. //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches |
 |
« Re: libpq patch for pqtypes hook api and PGresult creation
|
Re: [HACKERS] Terminating a backend »
| Thread Tools | |
| Display Modes | |
Linear Mode
| |
All times are GMT. The time now is 04:16 PM.
