maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sergey!

On Apr 22, svoj@xxxxxxxxxxx wrote:
> revision-id: 8a0f3310275f7b4fa445f907140f677910e18999
> parent(s): 7feee74dd30c96bd50d1c90e4ce3b06a656b17a5
> committer: Sergey Vojtovich
> branch nick: mariadb
> timestamp: 2015-04-22 14:18:51 +0400
> message:
> 
> MDEV-7943 - pthread_getspecific() takes 0.76% in OLTP RO
> 
> Avoid calling current_thd from thd_kill_level(). This reduces number of
> pthread_getspecific() calls from 776 to 354.
> 
> Also thd_kill_level(NULL) is not permitted anymore: this saves one condition.
> 
> ---
>  plugin/semisync/semisync_master.cc |  4 ++--
>  sql/sql_class.cc                   | 20 +++++++++-----------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/plugin/semisync/semisync_master.cc b/plugin/semisync/semisync_master.cc
> index c88c162..b1f7fbd 100644
> --- a/plugin/semisync/semisync_master.cc
> +++ b/plugin/semisync/semisync_master.cc
> @@ -635,7 +635,7 @@ int ReplSemiSyncMaster::commitTrx(const char* trx_wait_binlog_name,
>                              (int)is_on());
>      }
>  
> -    while (is_on() && !thd_killed(NULL))
> +    while (is_on() && !thd_killed(current_thd))

thd_killed() is a function of the kill_statement service
(see include/mysql/service_kill_statement.h). It is created for plugins
to use. But current_thd is not. Plugins generally have no access to it.

>      {
>        if (reply_file_name_inited_)
>        {
> diff --git a/sql/sql_class.cc b/sql/sql_class.cc
> index f273974..d32b14d 100644
> --- a/sql/sql_class.cc
> +++ b/sql/sql_class.cc
> @@ -4208,20 +4208,18 @@ void THD::restore_backup_open_tables_state(Open_tables_backup *backup)
>  */
>  extern "C" enum thd_kill_levels thd_kill_level(const MYSQL_THD thd)
>  {
> -  THD* current= current_thd;
> -
> -  if (!thd)
> -    thd= current;
> -
> -  if (thd == current)

I wonder if there's a cheaper way to check "if THD is current_thd".

E.g. THD stores the "end of stack" value. If it'll also store "beginning
of stack", one can easily check whether a local variable is within these
limits, which would mean the THD is current.

> -  {
> -    Apc_target *apc_target= (Apc_target*)&thd->apc_target;
> -    if (apc_target->have_apc_requests())
> -        apc_target->process_apc_requests(); 
> -  }
> +  DBUG_ASSERT(thd);
>  
>    if (likely(thd->killed == NOT_KILLED))
> +  {
> +    Apc_target *apc_target= (Apc_target*) &thd->apc_target;
> +    if (unlikely(apc_target->have_apc_requests()))
> +    {
> +      if (thd == current_thd)
> +        apc_target->process_apc_requests();
> +    }
>      return THD_IS_NOT_KILLED;
> +  }

Why did you put process_apc_requests() under thd->killed == NOT_KILLED ?

>    return thd->killed & KILL_HARD_BIT ? THD_ABORT_ASAP : THD_ABORT_SOFTLY;
>  }

Regards,
Sergei