Daniel Eischen wrote:
On Tue, 30 Oct 2007, David Xu wrote:
Kris Kennaway wrote:
kris 2007-10-29 21:01:47 UTC
FreeBSD src repository
Modified files:
lib/libthr/thread thr_mutex.c lib/libkse/thread
thr_mutex.c include pthread.h Log:
Add a new "non-portable" mutex type, PTHREAD_MUTEX_ADAPTIVE_NP. This
is also implemented in glibc and is used by a number of existing
applications (mysql, firefox, etc).
This mutex type is a default mutex with the additional property that
it spins briefly when attempting to acquire a contested lock, doing
trylock operations in userland before entering the kernel to block if
eventually unsuccessful.
The expectation is that applications requesting this mutex type know
that the mutex is likely to be only held for very brief periods, so it
is faster to spin in userland and probably succeed in acquiring the
mutex, than to enter the kernel and sleep, only to be woken up almost
immediately. This can help significantly in certain cases when
pthread mutexes are heavily contended and held for brief durations
(such as mysql).
Spin up to 200 times before entering the kernel, which represents
only
a few us on modern CPUs. No performance degradation was observed with
this value and it is sufficient to avoid a large performance drop in
mysql performance in the heavily contended pthread mutex case.
The libkse implementation is a NOP.
Reviewed by: jeff
MFC after: 3 days
Revision Changes Path
1.41 +2 -0 src/include/pthread.h
1.54 +3 -0 src/lib/libkse/thread/thr_mutex.c
1.55 +29 -0 src/lib/libthr/thread/thr_mutex.c
I am not sure PTHREAD_MUTEX_ADAPTIVE_NP is a correct solution, in fact
I think this is Linux crap, shouldn't PTHREAD_PRIO_PROTECT and
PTHREAD_PRIO_INHERIT mutex be adaptivly spinned ?
Yes, but only if we can do it in a way that does not reduce performance
in other cases. As you know, and as I mentioned already to Dan, this is
architecturally hard.
also this commit does not change mutex_self_lock() to handle the
PTHREAD_MUTEX_ADAPTIVE_NP, what is the PTHREAD_MUTEX_ADAPTIVE_NP
definition when the mutex is already locked by the currect thread ?
deadlock or return error code ?
As I mentioned in the commit, it is defined to be the same as the
default "errorcheck" type in all other aspects than the adaptive
spinning. However I see I missed a case:
Index: thread/thr_mutex.c
===================================================================
RCS file: /home/ncvs/src/lib/libthr/thread/thr_mutex.c,v
retrieving revision 1.55
diff -u -r1.55 thr_mutex.c
--- thread/thr_mutex.c 29 Oct 2007 21:01:47 -0000 1.55
+++ thread/thr_mutex.c 30 Oct 2007 08:16:18 -0000
@@ -558,6 +558,7 @@
switch (m->m_type) {
case PTHREAD_MUTEX_ERRORCHECK:
+ case PTHREAD_MUTEX_ADAPTIVE_NP:
if (abstime) {
clock_gettime(CLOCK_REALTIME, &ts1);
TIMESPEC_SUB(&ts2, abstime, &ts1);
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.
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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
