From edd38b50f665cb419507aa09f816cddcd895dfd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 24 Apr 2020 16:01:10 +0300 Subject: [PATCH] MDEV-7962 wsrep_on() takes 0.14% in OLTP RO The reason why we have wsrep_on() at all is that the macro WSREP(thd) depends on the definition of THD, and that is intentionally an opaque data type for InnoDB. So, we cannot avoid invoking wsrep_on(), but we can evaluate the less expensive conditions thd && WSREP_ON before calling the function. Global_read_lock: Use WSREP_NNULL(thd) instead of wsrep_on(thd) because we not only know the definition of THD but also that the pointer is not null. wsrep_open(): Use WSREP(thd) instead of wsrep_on(thd). InnoDB: Replace thd && wsrep_on(thd) with wsrep_on(thd), now that the condition has been merged to the definition of the macro wsrep_on(). --- include/mysql/service_wsrep.h | 2 +- sql/lock.cc | 13 +++++++++---- sql/wsrep_trans_observer.h | 2 +- storage/innobase/dict/dict0stats_bg.cc | 3 +-- storage/innobase/handler/ha_innodb.cc | 4 ++-- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/mysql/service_wsrep.h b/include/mysql/service_wsrep.h index 61a829f3043..522eb56fc4e 100644 --- a/include/mysql/service_wsrep.h +++ b/include/mysql/service_wsrep.h @@ -97,7 +97,7 @@ extern struct wsrep_service_st { #define wsrep_is_wsrep_xid(X) wsrep_service->wsrep_is_wsrep_xid_func(X) #define wsrep_xid_seqno(X) wsrep_service->wsrep_xid_seqno_func(X) #define wsrep_xid_uuid(X) wsrep_service->wsrep_xid_uuid_func(X) -#define wsrep_on(X) WSREP_ON && wsrep_service->wsrep_on_func(X) +#define wsrep_on(thd) (thd) && WSREP_ON && wsrep_service->wsrep_on_func(thd) #define wsrep_prepare_key_for_innodb(A,B,C,D,E,F,G) wsrep_service->wsrep_prepare_key_for_innodb_func(A,B,C,D,E,F,G) #define wsrep_thd_LOCK(T) wsrep_service->wsrep_thd_LOCK_func(T) #define wsrep_thd_UNLOCK(T) wsrep_service->wsrep_thd_UNLOCK_func(T) diff --git a/sql/lock.cc b/sql/lock.cc index 94e0d2733c7..8b8dd6fbed1 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -1,5 +1,6 @@ /* Copyright (c) 2000, 2011, Oracle and/or its affiliates. + Copyright (c) 2020, MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -1102,13 +1103,15 @@ void Global_read_lock::unlock_global_read_lock(THD *thd) { Wsrep_server_state& server_state= Wsrep_server_state::instance(); if (server_state.state() == Wsrep_server_state::s_donor || - (wsrep_on(thd) && server_state.state() != Wsrep_server_state::s_synced)) + (WSREP_NNULL(thd) && + server_state.state() != Wsrep_server_state::s_synced)) { /* TODO: maybe redundant here?: */ wsrep_locked_seqno= WSREP_SEQNO_UNDEFINED; server_state.resume(); } - else if (wsrep_on(thd) && server_state.state() == Wsrep_server_state::s_synced) + else if (WSREP_NNULL(thd) && + server_state.state() == Wsrep_server_state::s_synced) { server_state.resume_and_resync(); } @@ -1164,11 +1167,13 @@ bool Global_read_lock::make_global_read_lock_block_commit(THD *thd) Wsrep_server_state& server_state= Wsrep_server_state::instance(); wsrep::seqno paused_seqno; if (server_state.state() == Wsrep_server_state::s_donor || - (wsrep_on(thd) && server_state.state() != Wsrep_server_state::s_synced)) + (WSREP_NNULL(thd) && + server_state.state() != Wsrep_server_state::s_synced)) { paused_seqno= server_state.pause(); } - else if (wsrep_on(thd) && server_state.state() == Wsrep_server_state::s_synced) + else if (WSREP_NNULL(thd) && + server_state.state() == Wsrep_server_state::s_synced) { paused_seqno= server_state.desync_and_pause(); } diff --git a/sql/wsrep_trans_observer.h b/sql/wsrep_trans_observer.h index 98fc63cf783..1044cab76ad 100644 --- a/sql/wsrep_trans_observer.h +++ b/sql/wsrep_trans_observer.h @@ -405,7 +405,7 @@ static inline void wsrep_after_apply(THD* thd) static inline void wsrep_open(THD* thd) { DBUG_ENTER("wsrep_open"); - if (wsrep_on(thd)) + if (WSREP(thd)) { thd->wsrep_cs().open(wsrep::client_id(thd->thread_id)); thd->wsrep_cs().debug_log_level(wsrep_debug); diff --git a/storage/innobase/dict/dict0stats_bg.cc b/storage/innobase/dict/dict0stats_bg.cc index df877c36aae..e90b5da2411 100644 --- a/storage/innobase/dict/dict0stats_bg.cc +++ b/storage/innobase/dict/dict0stats_bg.cc @@ -179,8 +179,7 @@ void dict_stats_update_if_needed_func(dict_table_t* table) generated row locks and allow BF thread lock waits to be enqueued at head of waiting queue. */ - if (thd - && wsrep_on(thd) + if (wsrep_on(thd) && !wsrep_thd_is_applying(thd) && wsrep_thd_is_BF(thd, 0)) { WSREP_DEBUG("Avoiding background statistics" diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index fd4ff0aeffa..9dd1ec926b9 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4366,7 +4366,7 @@ innobase_commit_low( #ifdef WITH_WSREP THD* thd = (THD*)trx->mysql_thd; const char* tmp = 0; - if (thd && wsrep_on(thd)) { + if (wsrep_on(thd)) { #ifdef WSREP_PROC_INFO char info[64]; info[sizeof(info) - 1] = '\0'; @@ -4386,7 +4386,7 @@ innobase_commit_low( } trx->will_lock = 0; #ifdef WITH_WSREP - if (thd && wsrep_on(thd)) { thd_proc_info(thd, tmp); } + if (wsrep_on(thd)) { thd_proc_info(thd, tmp); } #endif /* WITH_WSREP */ }