Adieu find_sys_var_ex()

Only take LOCK_plugin for plugin system variables.

Reverted optimisation that was originally done for session tracker: it
makes much less sense now. Specifically only if connections would want to
track plugin session variables changes and these changes would actually
happen frequently. If this ever becomes an issue, there're much better
ways to optimise this workload.

Part of MDEV-14984 - regression in connect performance

5 files changed, 17 insertions, 67 deletions

diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc
index db82b7dffe9..63a6770f7d1 100644
--- a/sql/session_tracker.cc
+++ b/sql/session_tracker.cc
@@ -135,13 +135,6 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd,
   token= var_list.str;
 
   track_all= false;
-  /*
-    If Lock to the plugin mutex is not acquired here itself, it results
-    in having to acquire it multiple times in find_sys_var_ex for each
-    token value. Hence the mutex is handled here to avoid a performance
-    overhead.
-  */
-  mysql_mutex_lock(&LOCK_plugin);
   for (;;)
   {
     sys_var *svar;
@@ -165,11 +158,10 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd,
     {
       track_all= true;
     }
-    else if ((svar=
-              find_sys_var_ex(thd, var.str, var.length, throw_error, true)))
+    else if ((svar= find_sys_var(thd, var.str, var.length, throw_error)))
     {
       if (insert(svar) == TRUE)
-        goto error;
+        return true;
     }
     else if (throw_error && thd)
     {
@@ -179,20 +171,14 @@ bool Session_sysvars_tracker::vars_list::parse_var_list(THD *thd,
                           "be ignored.", (int)var.length, token);
     }
     else
-      goto error;
+      return true;
 
     if (lasts)
       token= lasts + 1;
     else
       break;
   }
-  mysql_mutex_unlock(&LOCK_plugin);
-
   return false;
-
-error:
-  mysql_mutex_unlock(&LOCK_plugin);
-  return true;
 }
 
 
@@ -211,14 +197,6 @@ bool sysvartrack_validate_value(THD *thd, const char *str, size_t len)
 
   token= var_list.str;
 
-  /*
-    If Lock to the plugin mutex is not acquired here itself, it results
-    in having to acquire it multiple times in find_sys_var_ex for each
-    token value. Hence the mutex is handled here to avoid a performance
-    overhead.
-  */
-  if (!thd)
-    mysql_mutex_lock(&LOCK_plugin);
   for (;;)
   {
     LEX_CSTRING var;
@@ -237,22 +215,14 @@ bool sysvartrack_validate_value(THD *thd, const char *str, size_t len)
     /* Remove leading/trailing whitespace. */
     trim_whitespace(system_charset_info, &var);
 
-    if(!strcmp(var.str, "*") &&
-       !find_sys_var_ex(thd, var.str, var.length, false, true))
-    {
-      if (!thd)
-        mysql_mutex_unlock(&LOCK_plugin);
+    if (!strcmp(var.str, "*") && !find_sys_var(thd, var.str, var.length))
       return true;
-    }
 
     if (lasts)
       token= lasts + 1;
     else
       break;
   }
-  if (!thd)
-    mysql_mutex_unlock(&LOCK_plugin);
-
   return false;
 }
 
diff --git a/sql/set_var.h b/sql/set_var.h
index 6097b28e76f..6e673cffefb 100644
--- a/sql/set_var.h
+++ b/sql/set_var.h
@@ -398,7 +398,8 @@ extern SHOW_COMP_OPTION have_openssl;
 SHOW_VAR* enumerate_sys_vars(THD *thd, bool sorted, enum enum_var_type type);
 int fill_sysvars(THD *thd, TABLE_LIST *tables, COND *cond);
 
-sys_var *find_sys_var(THD *thd, const char *str, size_t length=0);
+sys_var *find_sys_var(THD *thd, const char *str, size_t length= 0,
+                      bool throw_error= false);
 int sql_set_variables(THD *thd, List<set_var_base> *var_list, bool free);
 
 #define SYSVAR_AUTOSIZE(VAR,VAL)                        \
diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
index b5ff060ecc6..6ce778d03cf 100644
--- a/sql/sql_lex.cc
+++ b/sql/sql_lex.cc
@@ -7254,8 +7254,7 @@ bool LEX::set_system_variable(THD *thd, enum_var_type var_type,
 {
   sys_var *tmp;
   if (unlikely(check_reserved_words(name1)) ||
-      unlikely(!(tmp= find_sys_var_ex(thd, name2->str, name2->length, true,
-                                      false))))
+      unlikely(!(tmp= find_sys_var(thd, name2->str, name2->length, true))))
   {
     my_error(ER_UNKNOWN_STRUCTURED_VARIABLE, MYF(0),
              (int) name1->length, name1->str);
@@ -7649,7 +7648,7 @@ int set_statement_var_if_exists(THD *thd, const char *var_name,
     my_error(ER_SP_BADSTATEMENT, MYF(0), "[NO]WAIT");
     return 1;
   }
-  if ((sysvar= find_sys_var_ex(thd, var_name, var_name_length, true, false)))
+  if ((sysvar= find_sys_var(thd, var_name, var_name_length, true)))
   {
     Item *item= new (thd->mem_root) Item_uint(thd, value);
     set_var *var= new (thd->mem_root) set_var(thd, OPT_SESSION, sysvar,
diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc
index d0682ecb151..b8aff064aca 100644
--- a/sql/sql_plugin.cc
+++ b/sql/sql_plugin.cc
@@ -2822,37 +2822,25 @@ static void update_func_double(THD *thd, struct st_mysql_sys_var *var,
   System Variables support
 ****************************************************************************/
 
-sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length,
-                         bool throw_error, bool locked)
+sys_var *find_sys_var(THD *thd, const char *str, size_t length,
+                      bool throw_error)
 {
   sys_var *var;
-  sys_var_pluginvar *pi= NULL;
-  plugin_ref plugin;
-  DBUG_ENTER("find_sys_var_ex");
+  sys_var_pluginvar *pi;
+  DBUG_ENTER("find_sys_var");
   DBUG_PRINT("enter", ("var '%.*s'", (int)length, str));
 
-  if (!locked)
-    mysql_mutex_lock(&LOCK_plugin);
   mysql_prlock_rdlock(&LOCK_system_variables_hash);
   if ((var= intern_find_sys_var(str, length)) &&
       (pi= var->cast_pluginvar()))
   {
-    mysql_prlock_unlock(&LOCK_system_variables_hash);
-    LEX *lex= thd ? thd->lex : 0;
-    if (!(plugin= intern_plugin_lock(lex, plugin_int_to_ref(pi->plugin))))
+    mysql_mutex_lock(&LOCK_plugin);
+    if (!intern_plugin_lock(thd ? thd->lex : 0, plugin_int_to_ref(pi->plugin),
+                            PLUGIN_IS_READY))
       var= NULL; /* failed to lock it, it must be uninstalling */
-    else
-    if (!(plugin_state(plugin) & PLUGIN_IS_READY))
-    {
-      /* initialization not completed */
-      var= NULL;
-      intern_plugin_unlock(lex, plugin);
-    }
-  }
-  else
-    mysql_prlock_unlock(&LOCK_system_variables_hash);
-  if (!locked)
     mysql_mutex_unlock(&LOCK_plugin);
+  }
+  mysql_prlock_unlock(&LOCK_system_variables_hash);
 
   if (unlikely(!throw_error && !var))
     my_error(ER_UNKNOWN_SYSTEM_VARIABLE, MYF(0),
@@ -2861,11 +2849,6 @@ sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length,
 }
 
 
-sys_var *find_sys_var(THD *thd, const char *str, size_t length)
-{
-  return find_sys_var_ex(thd, str, length, false, false);
-}
-
 /*
   called by register_var, construct_options and test_plugin_options.
   Returns the 'bookmark' for the named variable.
diff --git a/sql/sql_plugin.h b/sql/sql_plugin.h
index 4e899e18f9b..01ec0563050 100644
--- a/sql/sql_plugin.h
+++ b/sql/sql_plugin.h
@@ -196,9 +196,6 @@ extern void sync_dynamic_session_variables(THD* thd, bool global_lock);
 extern bool plugin_dl_foreach(THD *thd, const LEX_CSTRING *dl,
                               plugin_foreach_func *func, void *arg);
 
-sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length,
-                         bool throw_error, bool locked);
-
 extern void sync_dynamic_session_variables(THD* thd, bool global_lock);
 #endif