Author: kotkov
Date: Thu Feb 5 16:46:55 2015
New Revision: 1657623
URL: http://svn.apache.org/r1657623
Log:
Tune the conditional compilation related to us using two different types
of locks (apr_thread_mutex_t and apr_thread_rwlock_t) in the membuffer cache.
Instead of having nested #if's for APR_HAS_THREADS and USE_SIMPLE_MUTEX,
make every possible code path explicit and ensure that they don't intersect.
We cannot really get rid of the conditional compilation, because using an
apr_thread_mutex_t under Windows is a workaround for poor APR read-write lock
performance and possible race condition (see [1,2]).
However, what we can do here is avoid merging the conditionally compiled
statements and ensure that only required fields of the struct svn_membuffer_t
are compiled in the corresponding #if statements. This patch gets rid of the
'allow_blocking_writes' field initialization when we do not have a read-write
lock, and also prevents a preprocessor from outputting code with multiple
return statements, e.g.:
static svn_error_t *
read_lock_cache(svn_membuffer_t * cache)
{
return svn_mutex__lock(cache->lock);
return SVN_NO_ERROR;
}
[1] http://svn.haxx.se/dev/archive-2014-12/0112.shtml
(Message-ID:
<CABw-3Yfs4ydsEZj6Y7yL=HK_zas6-9=xpZ=EwS+JaC=7hul...@mail.gmail.com>)
[2] https://issues.apache.org/bugzilla/show_bug.cgi?id=45455
* subversion/libsvn_subr/cache-membuffer.c
(struct svn_membuffer_t): Replace the nested #if's with two explicit
conditions. Compile out the 'allow_blocking_writes' field when we favor
apr_thread_mutex_t over apr_thread_rwlock_t, as it is not being used on
the corresponding code paths.
(read_lock_cache, write_lock_cache, force_write_lock_cache, unlock_cache):
Replace the nested #if's with explicit #if-elif-else conditions.
(svn_cache__membuffer_cache_create): Same here, but without an #else, as
with !APR_HAS_THREADS we don't have locks to initialize.
Modified:
subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1657623&r1=1657622&r2=1657623&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Thu Feb 5
16:46:55 2015
@@ -587,16 +587,15 @@ struct svn_membuffer_t
*/
apr_uint64_t total_hits;
-#if APR_HAS_THREADS
+#if (APR_HAS_THREADS && USE_SIMPLE_MUTEX)
/* A lock for intra-process synchronization to the cache, or NULL if
* the cache's creator doesn't feel the cache needs to be
* thread-safe.
*/
-# if USE_SIMPLE_MUTEX
svn_mutex__t *lock;
-# else
+#elif (APR_HAS_THREADS && !USE_SIMPLE_MUTEX)
+ /* Same for read-write lock. */
apr_thread_rwlock_t *lock;
-# endif
/* If set, write access will wait until they get exclusive access.
* Otherwise, they will become no-ops if the segment is currently
@@ -619,19 +618,20 @@ struct svn_membuffer_t
static svn_error_t *
read_lock_cache(svn_membuffer_t *cache)
{
-#if APR_HAS_THREADS
-# if USE_SIMPLE_MUTEX
+#if (APR_HAS_THREADS && USE_SIMPLE_MUTEX)
return svn_mutex__lock(cache->lock);
-# else
+#elif (APR_HAS_THREADS && !USE_SIMPLE_MUTEX)
if (cache->lock)
{
apr_status_t status = apr_thread_rwlock_rdlock(cache->lock);
if (status)
return svn_error_wrap_apr(status, _("Can't lock cache mutex"));
}
-# endif
-#endif
+
return SVN_NO_ERROR;
+#else
+ return SVN_NO_ERROR;
+#endif
}
/* If locking is supported for CACHE, acquire a write lock for it.
@@ -639,13 +639,9 @@ read_lock_cache(svn_membuffer_t *cache)
static svn_error_t *
write_lock_cache(svn_membuffer_t *cache, svn_boolean_t *success)
{
-#if APR_HAS_THREADS
-# if USE_SIMPLE_MUTEX
-
+#if (APR_HAS_THREADS && USE_SIMPLE_MUTEX)
return svn_mutex__lock(cache->lock);
-
-# else
-
+#elif (APR_HAS_THREADS && !USE_SIMPLE_MUTEX)
if (cache->lock)
{
apr_status_t status;
@@ -668,9 +664,10 @@ write_lock_cache(svn_membuffer_t *cache,
_("Can't write-lock cache mutex"));
}
-# endif
-#endif
return SVN_NO_ERROR;
+#else
+ return SVN_NO_ERROR;
+#endif
}
/* If locking is supported for CACHE, acquire an unconditional write lock
@@ -679,21 +676,18 @@ write_lock_cache(svn_membuffer_t *cache,
static svn_error_t *
force_write_lock_cache(svn_membuffer_t *cache)
{
-#if APR_HAS_THREADS
-# if USE_SIMPLE_MUTEX
-
+#if (APR_HAS_THREADS && USE_SIMPLE_MUTEX)
return svn_mutex__lock(cache->lock);
-
-# else
-
+#elif (APR_HAS_THREADS && !USE_SIMPLE_MUTEX)
apr_status_t status = apr_thread_rwlock_wrlock(cache->lock);
if (status)
return svn_error_wrap_apr(status,
_("Can't write-lock cache mutex"));
-# endif
-#endif
return SVN_NO_ERROR;
+#else
+ return SVN_NO_ERROR;
+#endif
}
/* If locking is supported for CACHE, release the current lock
@@ -702,13 +696,9 @@ force_write_lock_cache(svn_membuffer_t *
static svn_error_t *
unlock_cache(svn_membuffer_t *cache, svn_error_t *err)
{
-#if APR_HAS_THREADS
-# if USE_SIMPLE_MUTEX
-
+#if (APR_HAS_THREADS && USE_SIMPLE_MUTEX)
return svn_mutex__unlock(cache->lock, err);
-
-# else
-
+#elif (APR_HAS_THREADS && !USE_SIMPLE_MUTEX)
if (cache->lock)
{
apr_status_t status = apr_thread_rwlock_unlock(cache->lock);
@@ -719,9 +709,10 @@ unlock_cache(svn_membuffer_t *cache, svn
return svn_error_wrap_apr(status, _("Can't unlock cache mutex"));
}
-# endif
-#endif
return err;
+#else
+ return err;
+#endif
}
/* If supported, guard the execution of EXPR with a read lock to cache.
@@ -1797,17 +1788,14 @@ svn_cache__membuffer_cache_create(svn_me
return svn_error_wrap_apr(APR_ENOMEM, "OOM");
}
-#if APR_HAS_THREADS
+#if (APR_HAS_THREADS && USE_SIMPLE_MUTEX)
/* A lock for intra-process synchronization to the cache, or NULL if
* the cache's creator doesn't feel the cache needs to be
* thread-safe.
*/
-# if USE_SIMPLE_MUTEX
-
SVN_ERR(svn_mutex__init(&c[seg].lock, thread_safe, pool));
-
-# else
-
+#elif (APR_HAS_THREADS && !USE_SIMPLE_MUTEX)
+ /* Same for read-write lock. */
c[seg].lock = NULL;
if (thread_safe)
{
@@ -1817,8 +1805,6 @@ svn_cache__membuffer_cache_create(svn_me
return svn_error_wrap_apr(status, _("Can't create cache mutex"));
}
-# endif
-
/* Select the behavior of write operations.
*/
c[seg].allow_blocking_writes = allow_blocking_writes;
