cvs commit: src/lib/libthr/thread thr_mutex.c src/lib/libkse/thread thr_mutex.c src/include pthread.h

[David Xu]

Ken Smith wrote:
> On Tue, 2007-10-30 at 21:16 +0800, David Xu wrote:
>> Kris Kennaway wrote:
>>> PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP is just in <pthread.h>.
>>>
>>> /*
>>>  * Static initialization values.
>>>  */
>>> #define PTHREAD_MUTEX_INITIALIZER       NULL
>>> #define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP   NULL
>>> #define PTHREAD_COND_INITIALIZER        NULL
>>> #define PTHREAD_RWLOCK_INITIALIZER      NULL
>>>
>>>>>> I remembered mysql uses this macro to initialize spin mutex, and you
>>>>>> indead needs a patch to let it work
>>>>>
>>>>> No, with the code I committed mysql detects and uses it out of the 
>>>>> box, without requiring any patches.  It is easy to measure the 
>>>>> resulting 30% performance improvement at high loads ;-)
>>>>>
>>>> see above, I didn't see any code set PTHREAD_MUTEX_ADAPTIVE_NP type.
>>> The code is already in mysql for use with glibc.  It basically does
>>>
>>> #ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
>>> /*
>>>  * Use PTHREAD_MUTEX_ADAPTIVE_NP for the mutexes we know will benefit
>>>  * from it
>>>  */
>>> ...
>>> #endif
>>>
>>> so it just works.
>>>
>> I can not find code in libthr setting mutex's member field m_type to
>> PTHREAD_MUTEX_ADAPTIVE_NP by your change, so how can it work as
>> expected ?
>>
> 
> Wouldn't you expect that it's mysql that's setting the mutex's member
> field?  Something like this chunk of code from mysql perhaps?
> 
> #ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
>   /*
>     Set mutex type to "fast" a.k.a "adaptive"
> 
>     In this case the thread may steal the mutex from some other thread
>     that is waiting for the same mutex.  This will save us some
>     context switches but may cause a thread to 'starve forever' while
>     waiting for the mutex (not likely if the code within the mutex is
>     short).
>   */
>   pthread_mutexattr_init(&my_fast_mutexattr);
>   pthread_mutexattr_settype(&my_fast_mutexattr,
>                             PTHREAD_MUTEX_ADAPTIVE_NP);
> #endif
> 

Yes, I saw the code before. but PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
also can be used as:

pthread_mutex_t mymutex = PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP;

The above line initializes a mutex as adaptive mutex, but our
PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP is defined as NULL, so obviously
it creates a confusion from begin, it is wrong.

Regards,
David Xu