Re: [PECL-CVS] cvs: pecl /apc apc_cache.c apc_lock.h apc_main.c apc_sma.c apc_sma.h config.m4 php_apc.c
[email protected] (Brian Shire) Sat, 28 Oct 2006 04:20:01 -0700
| Newsgroups | php.apc.dev |
|---|---|
| Message-ID | <[email protected]> |
What are your thoughts on including a posix_mutex_* implementation
for locks? For a linux system running NPTL the underlying locks are
futex's and the speed gain appears to be comparable with using the
futex directly. A larger range of systems could probably take
advantage of this as well in their own way, rather than just linux.
I wouldn't mind correcting/tuning the futex locks more, but this
seems like it might be a more stable solution for the general user.
-shire
On Sep 28, 2006, at 3:01 PM, Rasmus Lerdorf wrote:
> What always stopped me from implementing futex support was the lack
> of owner-death cleanup which will lead to deadlocks. Ingo and
> company came up with a way to handle this not too long ago by
> keeping a list of held futex locks in userspace, but it always
> seemed rather flaky to me and you have to coordinate the lock
> release with the list of locks and avoid a race condition where the
> process that just got the lock dies before it has a chance to
> update the list of held locks. If the two get out of sync we are
> back in a deadlock. This is something you get for free (well at
> the cost of some performance) with the other locking mechanisms and
> looking through your patch I don't see anything that addresses this.
>
> -Rasmus
>
> Brian Shire wrote:
>> shire Thu Sep 28 21:22:09 2006 UTC
>> Modified files: /pecl/apc apc_cache.c
>> apc_lock.h apc_main.c apc_sma.c apc_sma.h config.m4
>> php_apc.c Log:
>> Added linux futex lock support (--enable-apc-futex)
>> Added arch directory with architecture specific code.
>> Moved lock values to shared memory segments.
>> Removed the Safety Net code added by rasmus but ifdef'd out
>> (incl. apc_sma_lock function as it caused difficulty with the lock
>> move).
>>
>> ---------------------------------------------------------------------
>> ---
>> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_cache.c?
>> r1=3.124&r2=3.125&diff_format=u
>> Index: pecl/apc/apc_cache.c
>> diff -u pecl/apc/apc_cache.c:3.124 pecl/apc/apc_cache.c:3.125
>> --- pecl/apc/apc_cache.c:3.124 Sat Sep 23 22:38:06 2006
>> +++ pecl/apc/apc_cache.c Thu Sep 28 21:22:09 2006
>> @@ -28,7 +28,7 @@
>> */
>> -/* $Id: apc_cache.c,v 3.124 2006/09/23 22:38:06 iliaa Exp $ */
>> +/* $Id: apc_cache.c,v 3.125 2006/09/28 21:22:09 shire Exp $ */
>> #include "apc_cache.h"
>> #include "apc_lock.h"
>> @@ -43,10 +43,10 @@
>> /* {{{ locking macros */
>> #define CREATE_LOCK apc_lck_create(NULL, 0, 1)
>> -#define DESTROY_LOCK(c) apc_lck_destroy(c->lock)
>> -#define LOCK(c) { HANDLE_BLOCK_INTERRUPTIONS();
>> apc_lck_lock(c->lock); }
>> -#define RDLOCK(c) { HANDLE_BLOCK_INTERRUPTIONS();
>> apc_lck_rdlock(c->lock); }
>> -#define UNLOCK(c) { apc_lck_unlock(c->lock);
>> HANDLE_UNBLOCK_INTERRUPTIONS(); }
>> +#define DESTROY_LOCK(c) apc_lck_destroy(c->header->lock)
>> +#define LOCK(c) { HANDLE_BLOCK_INTERRUPTIONS();
>> apc_lck_lock(c->header->lock); }
>> +#define RDLOCK(c) { HANDLE_BLOCK_INTERRUPTIONS();
>> apc_lck_rdlock(c->header->lock); }
>> +#define UNLOCK(c) { apc_lck_unlock(c->header->lock);
>> HANDLE_UNBLOCK_INTERRUPTIONS(); }
>> /* }}} */
>> /* {{{ struct definition: slot_t */
>> @@ -66,6 +66,8 @@
>> Any values that must be shared among processes should go in
>> here. */
>> typedef struct header_t header_t;
>> struct header_t {
>> + int lock; /* read/write lock (exclusive
>> blocking cache lock) */
>> + int wrlock; /* write lock (non-blocking used
>> to prevent cache slams) */
>> int num_hits; /* total successful hits in cache */
>> int num_misses; /* total unsuccessful hits in
>> cache */
>> int num_inserts; /* total successful inserts in
>> cache */
>> @@ -86,8 +88,6 @@
>> int num_slots; /* number of slots in cache */
>> int gc_ttl; /* maximum time on GC list for a
>> slot */
>> int ttl; /* if slot is needed and entry's
>> access time is older than this ttl, remove it */
>> - int lock; /* read/write lock (exclusive
>> blocking cache lock) */
>> - int wrlock; /* write lock (non-blocking used
>> to prevent cache slams) */
>> };
>> /* }}} */
>> @@ -298,9 +298,9 @@
>> cache->num_slots = num_slots;
>> cache->gc_ttl = gc_ttl;
>> cache->ttl = ttl;
>> - cache->lock = CREATE_LOCK;
>> + cache->header->lock = CREATE_LOCK;
>> #if NONBLOCKING_LOCK_AVAILABLE
>> - cache->wrlock = CREATE_LOCK;
>> + cache->header->wrlock = CREATE_LOCK;
>> #endif
>> for (i = 0; i < num_slots; i++) {
>> cache->slots[i] = NULL;
>> @@ -1068,14 +1068,14 @@
>> /* {{{ apc_cache_write_lock */
>> zend_bool apc_cache_write_lock(apc_cache_t* cache)
>> {
>> - return apc_lck_nb_lock(cache->wrlock);
>> + return apc_lck_nb_lock(cache->header->wrlock);
>> }
>> /* }}} */
>> /* {{{ apc_cache_write_unlock */
>> void apc_cache_write_unlock(apc_cache_t* cache)
>> {
>> - apc_lck_unlock(cache->wrlock);
>> + apc_lck_unlock(cache->header->wrlock);
>> }
>> /* }}} */
>> #endif
>> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_lock.h?
>> r1=3.14&r2=3.15&diff_format=u
>> Index: pecl/apc/apc_lock.h
>> diff -u pecl/apc/apc_lock.h:3.14 pecl/apc/apc_lock.h:3.15
>> --- pecl/apc/apc_lock.h:3.14 Sun Sep 24 17:27:32 2006
>> +++ pecl/apc/apc_lock.h Thu Sep 28 21:22:09 2006
>> @@ -26,7 +26,7 @@
>> */
>> -/* $Id: apc_lock.h,v 3.14 2006/09/24 17:27:32 iliaa Exp $ */
>> +/* $Id: apc_lock.h,v 3.15 2006/09/28 21:22:09 shire Exp $ */
>> #ifndef APC_LOCK
>> #define APC_LOCK
>> @@ -54,6 +54,14 @@
>> #define apc_lck_lock(a) apc_sem_lock(a)
>> #define apc_lck_rdlock(a) apc_sem_lock(a)
>> #define apc_lck_unlock(a) apc_sem_unlock(a)
>> +#elif defined(APC_FUTEX_LOCKS)
>> +#define NONBLOCKING_LOCK_AVAILABLE 1 +#define apc_lck_create
>> (a,b,c) apc_futex_create()
>> +#define apc_lck_destroy(a) apc_futex_destroy(&a)
>> +#define apc_lck_lock(a) apc_futex_lock(&a)
>> +#define apc_lck_nb_lock(a) apc_futex_nonblocking_lock(&a)
>> +#define apc_lck_rdlock(a) apc_futex_lock(&a)
>> +#define apc_lck_unlock(a) apc_futex_unlock(&a)
>> #else
>> #define RDLOCK_AVAILABLE 1
>> #ifdef PHP_WIN32
>> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_main.c?
>> r1=3.83&r2=3.84&diff_format=u
>> Index: pecl/apc/apc_main.c
>> diff -u pecl/apc/apc_main.c:3.83 pecl/apc/apc_main.c:3.84
>> --- pecl/apc/apc_main.c:3.83 Mon Sep 4 23:56:39 2006
>> +++ pecl/apc/apc_main.c Thu Sep 28 21:22:09 2006
>> @@ -28,7 +28,7 @@
>> */
>> -/* $Id: apc_main.c,v 3.83 2006/09/04 23:56:39 rasmus Exp $ */
>> +/* $Id: apc_main.c,v 3.84 2006/09/28 21:22:09 shire Exp $ */
>> #include "apc_php.h"
>> #include "apc_main.h"
>> @@ -584,11 +584,6 @@
>> (apc_cache_entry_t*) apc_stack_pop(APCG(cache_stack));
>> apc_cache_release(apc_cache, cache_entry);
>> }
>> - /* Safety net */
>> -#if 0
>> - apc_sma_unlock();
>> - apc_cache_unlock(apc_cache);
>> -#endif
>> }
>> /* }}} */
>> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_sma.c?
>> r1=1.53&r2=1.54&diff_format=u
>> Index: pecl/apc/apc_sma.c
>> diff -u pecl/apc/apc_sma.c:1.53 pecl/apc/apc_sma.c:1.54
>> --- pecl/apc/apc_sma.c:1.53 Fri Jun 9 23:41:22 2006
>> +++ pecl/apc/apc_sma.c Thu Sep 28 21:22:09 2006
>> @@ -26,7 +26,7 @@
>> */
>> -/* $Id: apc_sma.c,v 1.53 2006/06/09 23:41:22 rasmus Exp $ */
>> +/* $Id: apc_sma.c,v 1.54 2006/09/28 21:22:09 shire Exp $ */
>> #include "apc_sma.h"
>> #include "apc.h"
>> @@ -53,10 +53,10 @@
>> static int* sma_segments; /* array of shm segment ids */
>> static void** sma_shmaddrs; /* array of shm segment
>> addresses */
>> static int sma_lastseg = 0; /* index of MRU segment */
>> -static int sma_lock; /* sempahore to serialize
>> access */
>> typedef struct header_t header_t;
>> struct header_t {
>> + int sma_lock; /* segment lock, MUST BE ALIGNED for
>> futex locks */
>> size_t segsize; /* size of entire segment */
>> size_t avail; /* bytes available (not necessarily
>> contiguous) */
>> size_t nfoffset; /* start next fit search from this
>> offset */
>> @@ -286,8 +286,6 @@
>> sma_segments = (int*) apc_emalloc(sma_numseg*sizeof(int));
>> sma_shmaddrs = (void**) apc_emalloc(sma_numseg*sizeof(void*));
>> - sma_lock = apc_lck_create(NULL, 0, 1);
>> -
>> for (i = 0; i < sma_numseg; i++) {
>> header_t* header;
>> block_t* block;
>> @@ -304,6 +302,7 @@
>> shmaddr = sma_shmaddrs[i];
>> header = (header_t*) shmaddr;
>> + header->sma_lock = apc_lck_create(NULL, 0, 1);
>> header->segsize = sma_segsize;
>> header->avail = sma_segsize - sizeof(header_t) - sizeof
>> (block_t) -
>> alignword(sizeof(int));
>> @@ -332,13 +331,13 @@
>> assert(sma_initialized);
>> for (i = 0; i < sma_numseg; i++) {
>> + apc_lck_destroy(((header_t*)sma_shmaddrs[i])->sma_lock);
>> #if APC_MMAP
>> apc_unmap(sma_shmaddrs[i], sma_segments[i]);
>> #else
>> apc_shm_detach(sma_shmaddrs[i]);
>> #endif
>> }
>> - apc_lck_destroy(sma_lock);
>> sma_initialized = 0;
>> apc_efree(sma_segments);
>> apc_efree(sma_shmaddrs);
>> @@ -353,17 +352,19 @@
>> TSRMLS_FETCH();
>> assert(sma_initialized);
>> - LOCK(sma_lock);
>> + LOCK(((header_t*)sma_shmaddrs[sma_lastseg])->sma_lock);
>> off = sma_allocate(sma_shmaddrs[sma_lastseg], n);
>> if (off != -1) {
>> void* p = (void *)(((char *)(sma_shmaddrs[sma_lastseg]))
>> + off);
>> if (APCG(mem_size_ptr) != NULL) { *(APCG(mem_size_ptr))
>> += n; }
>> - UNLOCK(sma_lock);
>> + UNLOCK(((header_t*)sma_shmaddrs[sma_lastseg])->sma_lock);
>> return p;
>> }
>> + UNLOCK(((header_t*)sma_shmaddrs[sma_lastseg])->sma_lock);
>> for (i = 0; i < sma_numseg; i++) {
>> + LOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> if (i == sma_lastseg) {
>> continue;
>> }
>> @@ -371,13 +372,13 @@
>> if (off != -1) {
>> void* p = (void *)(((char *)(sma_shmaddrs[i])) + off);
>> if (APCG(mem_size_ptr) != NULL) { *(APCG
>> (mem_size_ptr)) += n; }
>> - UNLOCK(sma_lock);
>> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> sma_lastseg = i;
>> return p;
>> }
>> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> }
>> - UNLOCK(sma_lock);
>> return NULL;
>> }
>> /* }}} */
>> @@ -417,20 +418,20 @@
>> }
>> assert(sma_initialized);
>> - LOCK(sma_lock);
>> for (i = 0; i < sma_numseg; i++) {
>> + LOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> size_t d_size = (size_t)((char *)p - (char *)(sma_shmaddrs
>> [i]));
>> if (p >= sma_shmaddrs[i] && d_size < sma_segsize) {
>> sma_deallocate(sma_shmaddrs[i], d_size);
>> if (APCG(mem_size_ptr) != NULL) { *(APCG
>> (mem_size_ptr)) -= d_size; }
>> - UNLOCK(sma_lock);
>> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> return;
>> }
>> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> }
>> apc_eprint("apc_sma_free: could not locate address %p", p);
>> - UNLOCK(sma_lock);
>> }
>> /* }}} */
>> @@ -454,10 +455,9 @@
>> info->list[i] = NULL;
>> }
>> - RDLOCK(sma_lock);
>> -
>> /* For each segment */
>> for (i = 0; i < sma_numseg; i++) {
>> + RDLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> char* shmaddr = sma_shmaddrs[i];
>> block_t* prv = BLOCKAT(sizeof(header_t));
>> @@ -475,9 +475,9 @@
>> prv = cur;
>> }
>> + UNLOCK(((header_t*)sma_shmaddrs[i])->sma_lock);
>> }
>> - UNLOCK(sma_lock);
>> return info;
>> }
>> /* }}} */
>> @@ -520,12 +520,6 @@
>> return header->adist; }
>> #endif
>> -/* {{{ apc_sma_unlock */
>> -void apc_sma_unlock()
>> -{
>> - UNLOCK(sma_lock);
>> -}
>> -/* }}} */
>> #if 0
>> /* {{{ apc_sma_check_integrity */
>> http://cvs.php.net/viewvc.cgi/pecl/apc/apc_sma.h?
>> r1=1.14&r2=1.15&diff_format=u
>> Index: pecl/apc/apc_sma.h
>> diff -u pecl/apc/apc_sma.h:1.14 pecl/apc/apc_sma.h:1.15
>> --- pecl/apc/apc_sma.h:1.14 Sun Mar 12 00:31:45 2006
>> +++ pecl/apc/apc_sma.h Thu Sep 28 21:22:09 2006
>> @@ -25,7 +25,7 @@
>> */
>> -/* $Id: apc_sma.h,v 1.14 2006/03/12 00:31:45 rasmus Exp $ */
>> +/* $Id: apc_sma.h,v 1.15 2006/09/28 21:22:09 shire Exp $ */
>> #ifndef APC_SMA_H
>> #define APC_SMA_H
>> @@ -42,7 +42,6 @@
>> extern void* apc_sma_realloc(void* p, size_t size);
>> extern char* apc_sma_strdup(const char *s);
>> extern void apc_sma_free(void* p);
>> -extern void apc_sma_unlock();
>> #if ALLOC_DISTRIBUTION extern size_t
>> *apc_sma_get_alloc_distribution();
>> #endif
>> http://cvs.php.net/viewvc.cgi/pecl/apc/config.m4?
>> r1=3.15&r2=3.16&diff_format=u
>> Index: pecl/apc/config.m4
>> diff -u pecl/apc/config.m4:3.15 pecl/apc/config.m4:3.16
>> --- pecl/apc/config.m4:3.15 Sat Sep 9 23:51:15 2006
>> +++ pecl/apc/config.m4 Thu Sep 28 21:22:09 2006
>> @@ -1,5 +1,5 @@
>> dnl
>> -dnl $Id: config.m4,v 3.15 2006/09/09 23:51:15 rasmus Exp $
>> +dnl $Id: config.m4,v 3.16 2006/09/28 21:22:09 shire Exp $
>> dnl
>> AC_MSG_CHECKING(whether apc needs to get compiler flags from apxs)
>> @@ -67,9 +67,27 @@
>> AC_MSG_RESULT(no)
>> ])
>> +AC_MSG_CHECKING(Checking whether we should use futex locking)
>> +AC_ARG_ENABLE(apc-futex,
>> +[ --enable-apc-futex
>> + Enable linux futex based locks ],
>> +[
>> + PHP_APC_FUTEX=$enableval
>> + AC_MSG_RESULT($enableval)
>> +],
>> +[
>> + PHP_APC_FUTEX=no
>> + AC_MSG_RESULT(no)
>> +])
>> +
>> +if test "$PHP_APC_FUTEX" != "no"; then
>> + AC_CHECK_HEADER(linux/futex.h, , [ AC_MSG_ERROR([futex.h not
>> found. Please verify you that are running a 2.5 or older linux
>> kernel and that futex support is enabled.]); ] )
>> +fi
>> +
>> if test "$PHP_APC" != "no"; then
>> test "$PHP_APC_MMAP" != "no" && AC_DEFINE(APC_MMAP, 1, [ ])
>> test "$PHP_APC_SEM" != "no" && AC_DEFINE(APC_SEM_LOCKS, 1, [ ])
>> + test "$PHP_APC_FUTEX" != "no" && AC_DEFINE(APC_FUTEX_LOCKS, 1,
>> [ ])
>> AC_CACHE_CHECK(for union semun, php_cv_semun,
>> [
>> @@ -100,6 +118,7 @@
>> apc_pair.c \
>> apc_sem.c \
>> apc_shm.c \
>> + apc_futex.c \
>> apc_sma.c \
>> apc_stack.c \
>> apc_zend.c \
>> http://cvs.php.net/viewvc.cgi/pecl/apc/php_apc.c?
>> r1=3.113&r2=3.114&diff_format=u
>> Index: pecl/apc/php_apc.c
>> diff -u pecl/apc/php_apc.c:3.113 pecl/apc/php_apc.c:3.114
>> --- pecl/apc/php_apc.c:3.113 Thu Sep 21 11:52:16 2006
>> +++ pecl/apc/php_apc.c Thu Sep 28 21:22:09 2006
>> @@ -26,7 +26,7 @@
>> */
>> -/* $Id: php_apc.c,v 3.113 2006/09/21 11:52:16 gopalv Exp $ */
>> +/* $Id: php_apc.c,v 3.114 2006/09/28 21:22:09 shire Exp $ */
>> #include "apc_zend.h"
>> #include "apc_cache.h"
>> @@ -161,10 +161,12 @@
>> #endif
>> #if APC_SEM
>> php_info_print_table_row(2, "Locking type", "IPC Semaphore");
>> +#elif APC_FUTEX_LOCKS
>> + php_info_print_table_row(2, "Locking type", "Linux Futex
>> Locks");
>> #else
>> php_info_print_table_row(2, "Locking type", "File Locks");
>> #endif
>> - php_info_print_table_row(2, "Revision", "$Revision: 3.113 $");
>> + php_info_print_table_row(2, "Revision", "$Revision: 3.114 $");
>> php_info_print_table_row(2, "Build Date", __DATE__ " "
>> __TIME__);
>> php_info_print_table_end();
>> DISPLAY_INI_ENTRIES();
>> @@ -289,6 +291,8 @@
>> #endif
>> #if APC_SEM_LOCKS
>> add_assoc_stringl(return_value, "locking_type", "IPC
>> semaphore", sizeof("IPC semaphore"), 1);
>> +#elif APC_FUTEX_LOCKS
>> + add_assoc_stringl(return_value, "locking_type", "Linux
>> Futex", sizeof("Linux Futex"), 1);
>> #else
>> add_assoc_stringl(return_value, "locking_type", "file", sizeof
>> ("file"), 1);
>> #endif
>