maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Sergei,

Ok to push. I'm sorry for the delay with this.

On Wed, May 20, 2015 at 08:09:36AM +0000, svoj@xxxxxxxxxxx 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@xxxxxxxxxxx
> 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