Hi Sergei, Ok to push. I'm sorry for the delay with this. On Wed, May 20, 2015 at 08:09:36AM +0000, svoj@mariadb.org wrote:

revision-id: d1f5dfe67703a22f017eb25dd48b1eb5246a9c54 parent(s): 80333ad847d9f1708fa02fb47a976e96f014bc50 committer: Sergey Vojtovich branch nick: mariadb timestamp: 2015-05-20 12:04:32 +0400 message:

MDEV-8030 - Apc_target::disable() locks mutex twice

Moved Apc_target::destroy(), Apc_target::enable() and Apc_targe::disable() definitions to my_apc.h so that they can be inlined.

Apc_targe::disable() now calls Apc_target::process_apc_requests() only if there're APC requests. This saves one pthread_mutex_lock() call.

Overhead change: Apc_target::disable 0.04% -> out of radar Apc_target::enable 0.03% -> out of radar Apc_target::process_apc_requests 0.02% -> out of radar pthread_mutex_lock 0.43% -> 0.42% pthread_mutex_unlock 0.26% -> 0.25%

--- sql/my_apc.cc | 39 --------------------------------------- sql/my_apc.h | 27 +++++++++++++++++++++++---- 2 files changed, 23 insertions(+), 43 deletions(-)

diff --git a/sql/my_apc.cc b/sql/my_apc.cc index 1766068..91f5cd3 100644 --- a/sql/my_apc.cc +++ b/sql/my_apc.cc @@ -41,45 +41,6 @@ void Apc_target::init(mysql_mutex_t *target_mutex) }

-/* - Destroy the target. The target must be disabled when this call is made. -*/ -void Apc_target::destroy() -{ - DBUG_ASSERT(!enabled); -} - - -/* - Enter ther state where the target is available for serving APC requests -*/ -void Apc_target::enable() -{ - /* Ok to do without getting/releasing the mutex: */ - enabled++; -} - - -/* - Make the target unavailable for serving APC requests. - - @note - This call will serve all requests that were already enqueued -*/ - -void Apc_target::disable() -{ - bool process= FALSE; - DBUG_ASSERT(enabled); - mysql_mutex_lock(LOCK_thd_data_ptr); - if (!(--enabled)) - process= TRUE; - mysql_mutex_unlock(LOCK_thd_data_ptr); - if (process) - process_apc_requests(); -} - - /* [internal] Put request qe into the request list */

void Apc_target::enqueue_request(Call_request *qe) diff --git a/sql/my_apc.h b/sql/my_apc.h index dfeef5e..20b1ee4 100644 --- a/sql/my_apc.h +++ b/sql/my_apc.h @@ -50,10 +50,29 @@ class Apc_target ~Apc_target() { DBUG_ASSERT(!enabled && !apc_calls);}

void init(mysql_mutex_t *target_mutex); - void destroy(); - void enable(); - void disable(); - + + /* Destroy the target. The target must be disabled when this call is made. */ + void destroy() { DBUG_ASSERT(!enabled); } + + /* Enter ther state where the target is available for serving APC requests */ + void enable() { enabled++; } + + /* + Make the target unavailable for serving APC requests. + + @note + This call will serve all requests that were already enqueued + */ + void disable() + { + DBUG_ASSERT(enabled); + mysql_mutex_lock(LOCK_thd_data_ptr); + bool process= !--enabled && have_apc_requests(); + mysql_mutex_unlock(LOCK_thd_data_ptr); + if (unlikely(process)) + process_apc_requests(); + } + void process_apc_requests(); /* A lightweight function, intended to be used in frequent checks like this: _______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

-- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog