Allocate Session_sysvars_tracker statically

One less new/delete per connection.

Removed m_mem_flag since most allocs are thread specific. The only
exception are allocs performed during initialization.

Removed State_tracker and Session_tracker constructors as they don't make
sense anymore.

No reason to access session_sysvars_tracker via get_tracker(), so access
it directly instead.

Part of MDEV-14984 - regression in connect performance

sql/session_tracker.cc

@@ -16,8 +16,6 @@
 
 
 #include "sql_plugin.h"
-#include "session_tracker.h"
-
 #include "hash.h"
 #include "table.h"
 #include "rpl_gtid.h"
@@ -34,145 +32,6 @@ void State_tracker::mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name)
 }
 
 
-/**
-  Session_sysvars_tracker
-
-  This is a tracker class that enables & manages the tracking of session
-  system variables. It internally maintains a hash of user supplied variable
-  references and a boolean field to store if the variable was changed by the
-  last statement.
-*/
-
-class Session_sysvars_tracker : public State_tracker
-{
-  struct sysvar_node_st {
-    sys_var *m_svar;
-    bool *test_load;
-    bool m_changed;
-  };
-
-  class vars_list
-  {
-    /**
-      Registered system variables. (@@session_track_system_variables)
-      A hash to store the name of all the system variables specified by the
-      user.
-    */
-    HASH m_registered_sysvars;
-    /** Size of buffer for string representation */
-    size_t buffer_length;
-    myf m_mem_flag;
-    /**
-      If TRUE then we want to check all session variable.
-    */
-    bool track_all;
-    void init()
-    {
-      my_hash_init(&m_registered_sysvars,
-                   &my_charset_bin,
-		   4, 0, 0, (my_hash_get_key) sysvars_get_key,
-		   my_free, MYF(HASH_UNIQUE |
-                                ((m_mem_flag & MY_THREAD_SPECIFIC) ?
-                                 HASH_THREAD_SPECIFIC : 0)));
-    }
-    void free_hash()
-    {
-      if (my_hash_inited(&m_registered_sysvars))
-      {
-	my_hash_free(&m_registered_sysvars);
-      }
-    }
-
-    sysvar_node_st *search(const sys_var *svar)
-    {
-      return reinterpret_cast<sysvar_node_st*>(
-               my_hash_search(&m_registered_sysvars,
-                             reinterpret_cast<const uchar*>(&svar),
-                             sizeof(sys_var*)));
-    }
-
-    sysvar_node_st *at(ulong i)
-    {
-      DBUG_ASSERT(i < m_registered_sysvars.records);
-      return reinterpret_cast<sysvar_node_st*>(
-               my_hash_element(&m_registered_sysvars, i));
-    }
-  public:
-    vars_list(): buffer_length(0), track_all(false)
-    {
-      m_mem_flag= current_thd ? MY_THREAD_SPECIFIC : 0;
-      init();
-    }
-
-    size_t get_buffer_length()
-    {
-      DBUG_ASSERT(buffer_length != 0); // asked earlier then should
-      return buffer_length;
-    }
-    ~vars_list()
-    {
-      /* free the allocated hash. */
-      if (my_hash_inited(&m_registered_sysvars))
-      {
-	my_hash_free(&m_registered_sysvars);
-      }
-    }
-
-    sysvar_node_st *insert_or_search(const sys_var *svar)
-    {
-      sysvar_node_st *res= search(svar);
-      if (!res)
-      {
-	if (track_all)
-	{
-	  insert(svar);
-	  return search(svar);
-	}
-      }
-      return res;
-    }
-
-    bool insert(const sys_var *svar);
-    void reinit();
-    void reset();
-    inline bool is_enabled()
-    {
-      return track_all || m_registered_sysvars.records;
-    }
-    void copy(vars_list* from, THD *thd);
-    bool parse_var_list(THD *thd, LEX_STRING var_list, bool throw_error,
-                        CHARSET_INFO *char_set, bool take_mutex);
-    bool construct_var_list(char *buf, size_t buf_len);
-    bool store(THD *thd, String *buf);
-  };
-  /**
-    Two objects of vars_list type are maintained to manage
-    various operations.
-  */
-  vars_list orig_list;
-
-public:
-  size_t get_buffer_length()
-  {
-    return orig_list.get_buffer_length();
-  }
-  bool construct_var_list(char *buf, size_t buf_len)
-  {
-    return orig_list.construct_var_list(buf, buf_len);
-  }
-
-  bool enable(THD *thd);
-  bool update(THD *thd, set_var *var);
-  bool store(THD *thd, String *buf);
-  void mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name);
-  /* callback */
-  static uchar *sysvars_get_key(const char *entry, size_t *length,
-                                my_bool not_used __attribute__((unused)));
-
-  friend bool sysvartrack_global_update(THD *thd, char *str, size_t len);
-};
-
-
 /* To be used in expanding the buffer. */
 static const unsigned int EXTRA_ALLOC= 1024;
 
@@ -217,7 +76,9 @@ bool Session_sysvars_tracker::vars_list::insert(const sys_var *svar)
 {
   sysvar_node_st *node;
   if (!(node= (sysvar_node_st *) my_malloc(sizeof(sysvar_node_st),
-                                           MYF(MY_WME | m_mem_flag))))
+                                           MYF(MY_WME |
+                                               (mysqld_server_initialized ?
+                                                MY_THREAD_SPECIFIC : 0)))))
     return true;
 
   node->m_svar= (sys_var *)svar;
@@ -511,6 +372,7 @@ bool Session_sysvars_tracker::vars_list::construct_var_list(char *buf,
 
 bool Session_sysvars_tracker::enable(THD *thd)
 {
+  orig_list.reinit();
   mysql_mutex_lock(&LOCK_plugin);
   LEX_STRING tmp;
   tmp.str= global_system_variables.session_track_system_variables;
@@ -519,6 +381,7 @@ bool Session_sysvars_tracker::enable(THD *thd)
   {
     mysql_mutex_unlock(&LOCK_plugin);
     orig_list.reinit();
+    m_enabled= false;
     return true;
   }
   mysql_mutex_unlock(&LOCK_plugin);
@@ -1330,49 +1193,6 @@ bool Session_state_change_tracker::store(THD *thd, String *buf)
 
 ///////////////////////////////////////////////////////////////////////////////
 
-/**
-  @brief Initialize session tracker objects.
-*/
-
-Session_tracker::Session_tracker()
-{
-  /* track data ID fit into one byte in net coding */
-  compile_time_assert(SESSION_TRACK_always_at_the_end < 251);
-  /* one tracker could serv several tracking data */
-  compile_time_assert((uint)SESSION_TRACK_always_at_the_end >=
-                      (uint)SESSION_TRACKER_END);
-
-  m_trackers[SESSION_SYSVARS_TRACKER]= 0;
-  m_trackers[CURRENT_SCHEMA_TRACKER]= &current_schema;
-  m_trackers[SESSION_STATE_CHANGE_TRACKER]= &state_change;
-  m_trackers[TRANSACTION_INFO_TRACKER]= &transaction_info;
-}
-
-
-/**
-  @brief Enables the tracker objects.
-
-  @param thd [IN]    The thread handle.
-
-  @return            void
-*/
-
-void Session_tracker::enable(THD *thd)
-{
-  /*
-    Originally and correctly this allocation was in the constructor and
-    deallocation in the destructor, but in this case memory counting
-    system works incorrectly (for example in INSERT DELAYED thread)
-  */
-  deinit();
-  m_trackers[SESSION_SYSVARS_TRACKER]=
-    new (std::nothrow) Session_sysvars_tracker();
-
-  for (int i= 0; i < SESSION_TRACKER_END; i++)
-    m_trackers[i]->enable(thd);
-}
-
-
 /**
   @brief Store all change information in the specified buffer.
 
@@ -1385,6 +1205,12 @@ void Session_tracker::store(THD *thd, String *buf)
 {
   size_t start;
 
+  /* track data ID fit into one byte in net coding */
+  compile_time_assert(SESSION_TRACK_always_at_the_end < 251);
+  /* one tracker could serv several tracking data */
+  compile_time_assert((uint) SESSION_TRACK_always_at_the_end >=
+                      (uint) SESSION_TRACKER_END);
+
   /*
     Probably most track result will fit in 251 byte so lets made it at
     least efficient. We allocate 1 byte for length and then will move

sql/session_tracker.h

@@ -71,13 +71,7 @@ class State_tracker
   bool m_changed;
 
 public:
-  /** Constructor */
-  State_tracker() : m_enabled(false), m_changed(false)
-  {}
-
-  /** Destructor */
-  virtual ~State_tracker()
-  {}
+  virtual ~State_tracker() {}
 
   /** Getters */
   bool is_enabled() const
@@ -112,6 +106,130 @@ class State_tracker
 };
 
 
+/**
+  Session_sysvars_tracker
+
+  This is a tracker class that enables & manages the tracking of session
+  system variables. It internally maintains a hash of user supplied variable
+  references and a boolean field to store if the variable was changed by the
+  last statement.
+*/
+
+class Session_sysvars_tracker: public State_tracker
+{
+  struct sysvar_node_st {
+    sys_var *m_svar;
+    bool *test_load;
+    bool m_changed;
+  };
+
+  class vars_list
+  {
+    /**
+      Registered system variables. (@@session_track_system_variables)
+      A hash to store the name of all the system variables specified by the
+      user.
+    */
+    HASH m_registered_sysvars;
+    /** Size of buffer for string representation */
+    size_t buffer_length;
+    /**
+      If TRUE then we want to check all session variable.
+    */
+    bool track_all;
+    void init()
+    {
+      my_hash_init(&m_registered_sysvars, &my_charset_bin, 0, 0, 0,
+                   (my_hash_get_key) sysvars_get_key, my_free,
+                   HASH_UNIQUE | (mysqld_server_initialized ?
+                                  HASH_THREAD_SPECIFIC : 0));
+    }
+    void free_hash()
+    {
+      DBUG_ASSERT(my_hash_inited(&m_registered_sysvars));
+      my_hash_free(&m_registered_sysvars);
+    }
+
+    sysvar_node_st *search(const sys_var *svar)
+    {
+      return reinterpret_cast<sysvar_node_st*>(
+               my_hash_search(&m_registered_sysvars,
+                             reinterpret_cast<const uchar*>(&svar),
+                             sizeof(sys_var*)));
+    }
+
+    sysvar_node_st *at(ulong i)
+    {
+      DBUG_ASSERT(i < m_registered_sysvars.records);
+      return reinterpret_cast<sysvar_node_st*>(
+               my_hash_element(&m_registered_sysvars, i));
+    }
+  public:
+    vars_list(): buffer_length(0), track_all(false) { init(); }
+    void deinit() { free_hash(); }
+
+    size_t get_buffer_length()
+    {
+      DBUG_ASSERT(buffer_length != 0); // asked earlier then should
+      return buffer_length;
+    }
+
+    sysvar_node_st *insert_or_search(const sys_var *svar)
+    {
+      sysvar_node_st *res= search(svar);
+      if (!res)
+      {
+        if (track_all)
+        {
+          insert(svar);
+          return search(svar);
+        }
+      }
+      return res;
+    }
+
+    bool insert(const sys_var *svar);
+    void reinit();
+    void reset();
+    inline bool is_enabled()
+    {
+      return track_all || m_registered_sysvars.records;
+    }
+    void copy(vars_list* from, THD *thd);
+    bool parse_var_list(THD *thd, LEX_STRING var_list, bool throw_error,
+                        CHARSET_INFO *char_set, bool take_mutex);
+    bool construct_var_list(char *buf, size_t buf_len);
+    bool store(THD *thd, String *buf);
+  };
+  /**
+    Two objects of vars_list type are maintained to manage
+    various operations.
+  */
+  vars_list orig_list;
+
+public:
+  size_t get_buffer_length()
+  {
+    return orig_list.get_buffer_length();
+  }
+  bool construct_var_list(char *buf, size_t buf_len)
+  {
+    return orig_list.construct_var_list(buf, buf_len);
+  }
+
+  bool enable(THD *thd);
+  bool update(THD *thd, set_var *var);
+  bool store(THD *thd, String *buf);
+  void mark_as_changed(THD *thd, LEX_CSTRING *tracked_item_name);
+  void deinit() { orig_list.deinit(); }
+  /* callback */
+  static uchar *sysvars_get_key(const char *entry, size_t *length,
+                                my_bool not_used __attribute__((unused)));
+
+  friend bool sysvartrack_global_update(THD *thd, char *str, size_t len);
+};
+
+
 bool sysvartrack_validate_value(THD *thd, const char *str, size_t len);
 bool sysvartrack_global_update(THD *thd, char *str, size_t len);
 uchar *sysvartrack_session_value_ptr(THD *thd, const LEX_CSTRING *base);
@@ -310,18 +428,21 @@ class Session_tracker
   Current_schema_tracker current_schema;
   Session_state_change_tracker state_change;
   Transaction_state_tracker transaction_info;
+  Session_sysvars_tracker sysvars;
 
-  Session_tracker();
-  ~Session_tracker() { deinit(); }
-
-  /* trick to make happy memory accounting system */
-  void deinit()
+  Session_tracker()
   {
-    delete m_trackers[SESSION_SYSVARS_TRACKER];
-    m_trackers[SESSION_SYSVARS_TRACKER]= 0;
+    m_trackers[SESSION_SYSVARS_TRACKER]= &sysvars;
+    m_trackers[CURRENT_SCHEMA_TRACKER]= &current_schema;
+    m_trackers[SESSION_STATE_CHANGE_TRACKER]= &state_change;
+    m_trackers[TRANSACTION_INFO_TRACKER]= &transaction_info;
   }
 
-  void enable(THD *thd);
+  void enable(THD *thd)
+  {
+    for (int i= 0; i < SESSION_TRACKER_END; i++)
+      m_trackers[i]->enable(thd);
+  }
 
   /** Returns the pointer to the tracker object for the specified tracker. */
   inline State_tracker *get_tracker(enum_session_tracker tracker) const

sql/set_var.cc

@@ -1016,13 +1016,13 @@ int set_var_collation_client::update(THD *thd)
 
   /* Mark client collation variables as changed */
 #ifndef EMBEDDED_LIBRARY
-  if (thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->is_enabled())
+  if (thd->session_tracker.sysvars.is_enabled())
   {
-    thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->
+    thd->session_tracker.sysvars.
       mark_as_changed(thd, (LEX_CSTRING*)Sys_character_set_client_ptr);
-    thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->
+    thd->session_tracker.sysvars.
       mark_as_changed(thd, (LEX_CSTRING*)Sys_character_set_results_ptr);
-    thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->
+    thd->session_tracker.sysvars.
       mark_as_changed(thd, (LEX_CSTRING*)Sys_character_set_connection_ptr);
   }
   thd->session_tracker.mark_as_changed(thd, SESSION_STATE_CHANGE_TRACKER, NULL);

sql/sql_class.cc

@@ -1661,7 +1661,7 @@ THD::~THD()
 
   /* trick to make happy memory accounting system */
 #ifndef EMBEDDED_LIBRARY
-  session_tracker.deinit();
+  session_tracker.sysvars.deinit();
 #endif //EMBEDDED_LIBRARY
 
   if (status_var.local_memory_used != 0)
@@ -7302,12 +7302,11 @@ void THD::set_last_commit_gtid(rpl_gtid &gtid)
 #endif
   m_last_commit_gtid= gtid;
 #ifndef EMBEDDED_LIBRARY
-  if (changed_gtid &&
-      session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->is_enabled())
+  if (changed_gtid && session_tracker.sysvars.is_enabled())
   {
-    session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->
+    session_tracker.sysvars.
       mark_as_changed(this, (LEX_CSTRING*)Sys_last_gtid_ptr);
- }
+  }
 #endif
 }
 

sql/sys_vars.ic

@@ -623,10 +623,7 @@ public:
     return (new_val == 0 && var->save_result.string_value.str != 0);
   }
   bool session_update(THD *thd, set_var *var)
-  {
-    return thd->session_tracker.get_tracker(SESSION_SYSVARS_TRACKER)->
-      update(thd, var);
-  }
+  { return thd->session_tracker.sysvars.update(thd, var); }
   void session_save_default(THD *thd, set_var *var)
   {
      var->save_result.string_value.str= global_var(char*);