Hi, Sergey! On Mar 25, Sergey Vojtovich wrote:

revision-id: 48af6851da2 (mariadb-10.3.12-95-g48af6851da2) parent(s): a83d6ee812b author: Sergey Vojtovich <svoj@mariadb.org> committer: Sergey Vojtovich <svoj@mariadb.org> timestamp: 2019-03-20 01:58:42 +0400 message:

Make connect speed great again

Rather than parsing session_track_system_variables when thread starts, do it when first trackable event occurs.

Benchmarked on a 2socket/20core/40threads Broadwell system using sysbench connect brencmark @40 threads (with select 1 disabled): 101379.77 -> 143016.68 CPS, whereas 10.2 is currently at 137766.31 CPS.

Part of MDEV-14984 - regression in connect performance

diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 5d98e58046d..c493c790596 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -378,16 +378,9 @@ bool Session_sysvars_tracker::configure()

bool Session_sysvars_tracker::enable(THD *thd) { - LEX_STRING tmp= { session_track_system_variables, - safe_strlen(session_track_system_variables) }; orig_list.reinit(); - if (orig_list.parse_var_list(thd, tmp, true, thd->charset()) == true) - { - orig_list.reinit(); - m_enabled= false; - return true; - } - m_enabled= true; + m_parsed= false; + m_enabled= session_track_system_variables && *session_track_system_variables;

wouldn't it be simpler not to allow both NULL and "" as valid values for session_track_system_variables ? Either always store the empty string as NULL or as "", why would you want both?

return false; }

@@ -530,6 +524,19 @@ void Session_sysvars_tracker::mark_as_changed(THD *thd, { sysvar_node_st *node; sys_var *svar= (sys_var *)var; + + if (!m_parsed) + { + LEX_STRING tmp= { session_track_system_variables, + safe_strlen(session_track_system_variables) }; + if (orig_list.parse_var_list(thd, tmp, true, thd->charset())) + { + orig_list.reinit(); + return; + } + m_parsed= true;

This is of somewhat dubious value. The default value for the @@session_track_system_variables is DEFAULT("autocommit,character_set_client,character_set_connection," "character_set_results,time_zone"), and these are variables that are typically set by connectors automatically on every new connection. Only a pure C/C will benefit from it, that's what sysbench shows. If you'd tested with java, you, probably, wouldn't get any speedup.

+ } + /* Check if the specified system variable is being tracked, if so mark it as changed and also set the class's m_changed flag. diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 7194bbd03ef..53203d15f0b 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -7320,8 +7320,13 @@ void THD::set_last_commit_gtid(rpl_gtid >id) #ifndef EMBEDDED_LIBRARY if (changed_gtid && session_tracker.session_sysvars_tracker.is_enabled()) { + THD *orig_thd= current_thd; + if (orig_thd != this)

When can it be true?

+ set_current_thd(this); session_tracker.session_sysvars_tracker. mark_as_changed(this, (LEX_CSTRING*)Sys_last_gtid_ptr); + if (orig_thd != this) + set_current_thd(orig_thd); } #endif }

Regards, Sergei Chief Architect MariaDB and security@mariadb.org