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

[Kris Kennaway]

David Xu wrote:

>>> As I said in previous email, I would rather see our default
>>> mutex implementations improved instead of adding new interfaces.
>>> If it's really necessary in the short term, perhaps an
>>> environment variable that can be set to force all mutexes
>>> to be adaptive (and when kern.smp.cpus > 1 perhaps?).
>>
>>
>> There might be a case for adding that for people who want to 
>> experiment, but it's not appropriate as a replacement since it's 
>> highly application specific, and the application already knows whether 
>> it wants this property or not.  It is also often not appropriate to 
>> use this behaviour on every mutex used by an application.
>>
> 
> I do think many application writters do not know what should be done for 
> his mutexes, generic spinning may be OK, but can be turned off.

That is a fairly radical change you are proposing.  One would expect 
that this adaptive spinning algorithm *reduces* performance in cases 
where the mutex is usually held for longer periods of time.  If you 
think this theoretical expectation is wrong, you need to provide some 
careful measurements supporting that somewhat surprising hypothesis :)

>> When arguing about this commit, keep in mind that with this simple 
>> change mysql performs 30% better out of the box at high loads (without 
>> requiring any patches).  That is not something that should be lightly 
>> dismissed until you have a better replacement ready.
>>
>> Kris
>>
> 
> I object adding PTHREAD_MUTEX_ADAPTIVE_NP, because there is no
> corresponding PTHREAD_ADPATIVE_MUTEX_INITIALIZER, but normal
> pthread mutex has macro PTHREAD_MUTEX_INITIALIZER, so it is
> inconsistent, maybe adding pthread_mutexattr_setspin() etcs are better
> than this hack for our implementation, it is not portable though.

There is an PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP, which is again the 
name that existing applications expect for it.  The fact that this 
interface *already exists* in other operating systems and *is already 
used* by real applications overrides objections about one name choice vs 
another.  The best you can argue for is to use a different name with a 
compatibility #define, but I dont see the point of this.

> 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 ;-)

 > in fact spin mutex in libthr is
> arguable, normally you can use pthread_mutex_trylock() in application to 
> simulate spinning, adding such mutex type it in libthr just simplified
> it, but it is not portable.

That is what the "_NP" indicates (although remember that this interface 
is compatible with glibc).

Kris