MDEV-5675 - Performance: my_hash_sort_bin is called too often

Reduced number of my_hash_sort_bin() calls from 4 to 1 per query.
Reduced number of memory accesses done by my_hash_sort_bin().

Details:
- let MDL subsystem use pre-calculated hash value for hash
  inserts and deletes
- let table cache use pre-calculated MDL hash value
- MDL namespace is excluded from hash value calculation, so that
  hash value can be used by table cache as is
- hash value for MDL is calculated as resulting hash value + MDL
  namespace
- extended hash implementation to accept user defined hash function

client/mysqltest.cc

@@ -8945,7 +8945,7 @@ int main(int argc, char **argv)
   my_init_dynamic_array(&q_lines, sizeof(struct st_command*), 1024, 1024, MYF(0));
 
   if (my_hash_init2(&var_hash, 64, charset_info,
-                 128, 0, 0, get_var_key, var_free, MYF(0)))
+                 128, 0, 0, get_var_key, 0, var_free, MYF(0)))
     die("Variable hash initialization failed");
 
   var_set_string("MYSQL_SERVER_VERSION", MYSQL_SERVER_VERSION);

include/hash.h

@@ -44,6 +44,8 @@ extern "C" {
 
 typedef uint my_hash_value_type;
 typedef uchar *(*my_hash_get_key)(const uchar *,size_t*,my_bool);
+typedef my_hash_value_type (*my_hash_function)(const CHARSET_INFO *,
+                                               const uchar *, size_t);
 typedef void (*my_hash_free_key)(void *);
 typedef my_bool (*my_hash_walk_action)(void *,void *);
 
@@ -54,6 +56,7 @@ typedef struct st_hash {
   uint flags;
   DYNAMIC_ARRAY array;				/* Place for hash_keys */
   my_hash_get_key get_key;
+  my_hash_function hash_function;
   void (*free)(void *);
   CHARSET_INFO *charset;
 } HASH;
@@ -61,10 +64,11 @@ typedef struct st_hash {
 /* A search iterator state */
 typedef uint HASH_SEARCH_STATE;
 
-#define my_hash_init(A,B,C,D,E,F,G,H) my_hash_init2(A,0,B,C,D,E,F,G,H)
+#define my_hash_init(A,B,C,D,E,F,G,H) my_hash_init2(A,0,B,C,D,E,F,0,G,H)
 my_bool my_hash_init2(HASH *hash, uint growth_size, CHARSET_INFO *charset,
                       ulong default_array_elements, size_t key_offset,
                       size_t key_length, my_hash_get_key get_key,
+                      my_hash_function hash_function,
                       void (*free_element)(void*),
                       uint flags);
 void my_hash_free(HASH *tree);
@@ -74,8 +78,9 @@ uchar *my_hash_search(const HASH *info, const uchar *key, size_t length);
 uchar *my_hash_search_using_hash_value(const HASH *info,
                                        my_hash_value_type hash_value,
                                        const uchar *key, size_t length);
-my_hash_value_type my_calc_hash(const HASH *info,
+my_hash_value_type my_hash_sort(const CHARSET_INFO *cs,
                                 const uchar *key, size_t length);
+#define my_calc_hash(A, B, C) my_hash_sort((A)->charset, B, C)
 uchar *my_hash_first(const HASH *info, const uchar *key, size_t length,
                      HASH_SEARCH_STATE *state);
 uchar *my_hash_first_from_hash_value(const HASH *info,

mysql-test/r/create_or_replace.result

@@ -333,29 +333,29 @@ lock table t1 write, t2 read;
 select * from information_schema.metadata_lock_info;
 THREAD_ID	LOCK_MODE	LOCK_DURATION	LOCK_TYPE	TABLE_SCHEMA	TABLE_NAME
 #	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Global read lock		
-#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_NO_READ_WRITE	MDL_EXPLICIT	Table metadata lock	test	t1
+#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_READ	MDL_EXPLICIT	Table metadata lock	test	t2
 create or replace table t1 (i int);
 select * from information_schema.metadata_lock_info;
 THREAD_ID	LOCK_MODE	LOCK_DURATION	LOCK_TYPE	TABLE_SCHEMA	TABLE_NAME
 #	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Global read lock		
-#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_NO_READ_WRITE	MDL_EXPLICIT	Table metadata lock	test	t1
+#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_READ	MDL_EXPLICIT	Table metadata lock	test	t2
 create or replace table t1 like t2;
 select * from information_schema.metadata_lock_info;
 THREAD_ID	LOCK_MODE	LOCK_DURATION	LOCK_TYPE	TABLE_SCHEMA	TABLE_NAME
 #	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Global read lock		
-#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_NO_READ_WRITE	MDL_EXPLICIT	Table metadata lock	test	t1
+#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_READ	MDL_EXPLICIT	Table metadata lock	test	t2
 create or replace table t1 select 1 as f1;
 select * from information_schema.metadata_lock_info;
 THREAD_ID	LOCK_MODE	LOCK_DURATION	LOCK_TYPE	TABLE_SCHEMA	TABLE_NAME
 #	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Global read lock		
-#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_NO_READ_WRITE	MDL_EXPLICIT	Table metadata lock	test	t1
+#	MDL_INTENTION_EXCLUSIVE	MDL_EXPLICIT	Schema metadata lock	test	
 #	MDL_SHARED_READ	MDL_EXPLICIT	Table metadata lock	test	t2
 drop table t1;
 unlock tables;

mysys/hash.c

@@ -40,12 +40,12 @@ static void movelink(HASH_LINK *array,uint pos,uint next_link,uint newlink);
 static int hashcmp(const HASH *hash, HASH_LINK *pos, const uchar *key,
                    size_t length);
 
-static my_hash_value_type calc_hash(const HASH *hash,
-                                    const uchar *key, size_t length)
+my_hash_value_type my_hash_sort(const CHARSET_INFO *cs, const uchar *key,
+                                size_t length)
 {
-  ulong nr1=1, nr2=4;
-  hash->charset->coll->hash_sort(hash->charset,(uchar*) key,length,&nr1,&nr2);
-  return (my_hash_value_type)nr1;
+  ulong nr1= 1, nr2= 4;
+  cs->coll->hash_sort(cs, (uchar*) key, length, &nr1, &nr2);
+  return (my_hash_value_type) nr1;
 }
 
 /**
@@ -78,6 +78,7 @@ my_bool
 my_hash_init2(HASH *hash, uint growth_size, CHARSET_INFO *charset,
               ulong size, size_t key_offset, size_t key_length,
               my_hash_get_key get_key,
+              my_hash_function hash_function,
               void (*free_element)(void*), uint flags)
 {
   my_bool res;
@@ -89,6 +90,7 @@ my_hash_init2(HASH *hash, uint growth_size, CHARSET_INFO *charset,
   hash->key_length=key_length;
   hash->blength=1;
   hash->get_key=get_key;
+  hash->hash_function= hash_function ? hash_function : my_hash_sort;
   hash->free=free_element;
   hash->flags=flags;
   hash->charset=charset;
@@ -200,7 +202,8 @@ static uint my_hash_rec_mask(const HASH *hash, HASH_LINK *pos,
 {
   size_t length;
   uchar *key= (uchar*) my_hash_key(hash, pos->data, &length, 0);
-  return my_hash_mask(calc_hash(hash, key, length), buffmax, maxlength);
+  return my_hash_mask(hash->hash_function(hash->charset, key, length), buffmax,
+                      maxlength);
 }
 
 
@@ -214,7 +217,7 @@ my_hash_value_type rec_hashnr(HASH *hash,const uchar *record)
 {
   size_t length;
   uchar *key= (uchar*) my_hash_key(hash, record, &length, 0);
-  return calc_hash(hash,key,length);
+  return hash->hash_function(hash->charset, key, length);
 }
 
 
@@ -234,12 +237,6 @@ uchar* my_hash_search_using_hash_value(const HASH *hash,
                                        key, length, &state);
 }
 
-my_hash_value_type my_calc_hash(const HASH *hash,
-                                const uchar *key, size_t length)
-{
-  return calc_hash(hash, key, length ? length : hash->key_length);
-}
-
 
 /*
   Search after a record based on a key
@@ -254,7 +251,8 @@ uchar* my_hash_first(const HASH *hash, const uchar *key, size_t length,
   uchar *res;
   if (my_hash_inited(hash))
     res= my_hash_first_from_hash_value(hash,
-                   calc_hash(hash, key, length ? length : hash->key_length),
+                   hash->hash_function(hash->charset, key,
+                                       length ? length : hash->key_length),
                    key, length, current_record);
   else
     res= 0;
@@ -644,9 +642,9 @@ my_bool my_hash_update(HASH *hash, uchar *record, uchar *old_key,
 
   /* Search after record with key */
 
-  idx= my_hash_mask(calc_hash(hash, old_key, (old_key_length ?
-                                              old_key_length :
-                                              hash->key_length)),
+  idx= my_hash_mask(hash->hash_function(hash->charset, old_key,
+                                        (old_key_length ? old_key_length :
+                                                          hash->key_length)),
                     blength, records);
   new_index= my_hash_mask(rec_hashnr(hash, record), blength, records);
   if (idx == new_index)

mysys/thr_mutex.c

@@ -174,12 +174,12 @@ static int safe_mutex_lazy_init_deadlock_detection(safe_mutex_t *mp)
              128,
              offsetof(safe_mutex_deadlock_t, id),
              sizeof(mp->id),
-             0, 0, HASH_UNIQUE);
+             0, 0, 0, HASH_UNIQUE);
   my_hash_init2(mp->used_mutex, 64, &my_charset_bin,
              128,
              offsetof(safe_mutex_t, id),
              sizeof(mp->id),
-             0, 0, HASH_UNIQUE);
+             0, 0, 0, HASH_UNIQUE);
   return 0;
 }
 

sql/mdl.cc

@@ -124,15 +124,9 @@ class MDL_map_partition
 public:
   MDL_map_partition();
   ~MDL_map_partition();
-  inline MDL_lock *find_or_insert(const MDL_key *mdl_key,
-                                  my_hash_value_type hash_value);
-  unsigned long get_lock_owner(const MDL_key *key,
-                               my_hash_value_type hash_value);
+  inline MDL_lock *find_or_insert(const MDL_key *mdl_key);
+  unsigned long get_lock_owner(const MDL_key *key);
   inline void remove(MDL_lock *lock);
-  my_hash_value_type get_key_hash(const MDL_key *mdl_key) const
-  {
-    return my_calc_hash(&m_locks, mdl_key->ptr(), mdl_key->length());
-  }
 private:
   bool move_from_hash_to_lock_mutex(MDL_lock *lock);
   /** A partition of all acquired locks in the server. */
@@ -766,13 +760,21 @@ void MDL_map::init()
 }
 
 
+my_hash_value_type mdl_hash_function(const CHARSET_INFO *cs,
+                                     const uchar *key, size_t length)
+{
+  MDL_key *mdl_key= (MDL_key*) (key - offsetof(MDL_key, m_ptr));
+  return mdl_key->hash_value();
+}
+
+
 /** Initialize the partition in the container with all MDL locks. */
 
 MDL_map_partition::MDL_map_partition()
 {
   mysql_mutex_init(key_MDL_map_mutex, &m_mutex, NULL);
-  my_hash_init(&m_locks, &my_charset_bin, 16 /* FIXME */, 0, 0,
-               mdl_locks_key, 0, 0);
+  my_hash_init2(&m_locks, 0, &my_charset_bin, 16 /* FIXME */, 0, 0,
+                mdl_locks_key, mdl_hash_function, 0, 0);
 };
 
 
@@ -846,11 +848,10 @@ MDL_lock* MDL_map::find_or_insert(const MDL_key *mdl_key)
     return lock;
   }
 
-  my_hash_value_type hash_value= m_partitions.at(0)->get_key_hash(mdl_key);
-  uint part_id= hash_value % mdl_locks_hash_partitions;
+  uint part_id= mdl_key->hash_value() % mdl_locks_hash_partitions;
   MDL_map_partition *part= m_partitions.at(part_id);
 
-  return part->find_or_insert(mdl_key, hash_value);
+  return part->find_or_insert(mdl_key);
 }
 
 
@@ -863,15 +864,14 @@ MDL_lock* MDL_map::find_or_insert(const MDL_key *mdl_key)
   @retval NULL     - Failure (OOM).
 */
 
-MDL_lock* MDL_map_partition::find_or_insert(const MDL_key *mdl_key,
-                                            my_hash_value_type hash_value)
+MDL_lock* MDL_map_partition::find_or_insert(const MDL_key *mdl_key)
 {
   MDL_lock *lock;
 
 retry:
   mysql_mutex_lock(&m_mutex);
   if (!(lock= (MDL_lock*) my_hash_search_using_hash_value(&m_locks,
-                                                          hash_value,
+                                                          mdl_key->hash_value(),
                                                           mdl_key->ptr(),
                                                           mdl_key->length())))
   {
@@ -1023,10 +1023,9 @@ MDL_map::get_lock_owner(const MDL_key *mdl_key)
   }
   else
   {
-    my_hash_value_type hash_value= m_partitions.at(0)->get_key_hash(mdl_key);
-    uint part_id= hash_value % mdl_locks_hash_partitions;
+    uint part_id= mdl_key->hash_value() % mdl_locks_hash_partitions;
     MDL_map_partition *part= m_partitions.at(part_id);
-    res= part->get_lock_owner(mdl_key, hash_value);
+    res= part->get_lock_owner(mdl_key);
   }
   return res;
 }
@@ -1034,15 +1033,14 @@ MDL_map::get_lock_owner(const MDL_key *mdl_key)
 
 
 unsigned long
-MDL_map_partition::get_lock_owner(const MDL_key *mdl_key,
-                                  my_hash_value_type hash_value)
+MDL_map_partition::get_lock_owner(const MDL_key *mdl_key)
 {
   MDL_lock *lock;
   unsigned long res= 0;
 
   mysql_mutex_lock(&m_mutex);
   lock= (MDL_lock*) my_hash_search_using_hash_value(&m_locks,
-                                                  hash_value,
+                                                  mdl_key->hash_value(),
                                                   mdl_key->ptr(),
                                                   mdl_key->length());
   if (lock)

sql/mdl.h

@@ -28,6 +28,7 @@
 #include <my_sys.h>
 #include <m_string.h>
 #include <mysql_com.h>
+#include <hash.h>
 
 #include <algorithm>
 
@@ -347,12 +348,15 @@ public:
                                           m_ptr - 1);
     m_length= static_cast<uint16>(strmake(m_ptr + m_db_name_length + 2, name,
                                           NAME_LEN) - m_ptr + 1);
+    m_hash_value= my_hash_sort(&my_charset_bin, (uchar*) m_ptr + 1,
+                               m_length - 1);
   }
   void mdl_key_init(const MDL_key *rhs)
   {
     memcpy(m_ptr, rhs->m_ptr, rhs->m_length);
     m_length= rhs->m_length;
     m_db_name_length= rhs->m_db_name_length;
+    m_hash_value= rhs->m_hash_value;
   }
   bool is_equal(const MDL_key *rhs) const
   {
@@ -392,15 +396,26 @@ public:
   {
     return & m_namespace_to_wait_state_name[(int)mdl_namespace()];
   }
+  my_hash_value_type hash_value() const
+  {
+    return m_hash_value + mdl_namespace();
+  }
+  my_hash_value_type tc_hash_value() const
+  {
+    return m_hash_value;
+  }
 
 private:
   uint16 m_length;
   uint16 m_db_name_length;
+  my_hash_value_type m_hash_value;
   char m_ptr[MAX_MDLKEY_LENGTH];
   static PSI_stage_info m_namespace_to_wait_state_name[NAMESPACE_END];
 private:
   MDL_key(const MDL_key &);                     /* not implemented */
   MDL_key &operator=(const MDL_key &);          /* not implemented */
+  friend my_hash_value_type mdl_hash_function(const CHARSET_INFO *,
+                                              const uchar *, size_t);
 };
 
 

sql/sql_acl.cc

@@ -1085,7 +1085,7 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables)
   table->use_all_columns();
   (void) my_init_dynamic_array(&acl_users,sizeof(ACL_USER), 50, 100, MYF(0));
   (void) my_hash_init2(&acl_roles,50, &my_charset_utf8_bin,
-                       0,0,0, (my_hash_get_key) acl_role_get_key,
+                       0, 0, 0, (my_hash_get_key) acl_role_get_key, 0,
                        (void (*)(void *))free_acl_role, 0);
 
   username_char_length= MY_MIN(table->field[1]->char_length(),
@@ -1427,8 +1427,8 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables)
     table->use_all_columns();
     /* account for every role mapping */
 
-    (void) my_hash_init2(&acl_roles_mappings, 50, system_charset_info,
-                         0,0,0, (my_hash_get_key) acl_role_map_get_key, 0,0);
+    (void) my_hash_init2(&acl_roles_mappings, 50, system_charset_info, 0, 0, 0,
+                         (my_hash_get_key) acl_role_map_get_key, 0, 0, 0);
     MEM_ROOT temp_root;
     init_alloc_root(&temp_root, ACL_ALLOC_BLOCK_SIZE, 0, MYF(0));
     while (!(read_record_info.read_record(&read_record_info)))
@@ -3734,8 +3734,8 @@ public:
   bool ok() { return privs != 0 || cols != 0; }
   void init_hash()
   {
-    my_hash_init2(&hash_columns, 4, system_charset_info,
-                  0, 0, 0, (my_hash_get_key) get_key_column, 0, 0);
+    my_hash_init2(&hash_columns, 4, system_charset_info, 0, 0, 0,
+                  (my_hash_get_key) get_key_column, 0, 0, 0);
   }
 };
 

sql/sql_base.cc

@@ -2296,7 +2296,9 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
 retry_share:
 
   share= tdc_acquire_share(thd, table_list->db, table_list->table_name,
-                           key, key_length, gts_flags, &table);
+                           key, key_length,
+                           table_list->mdl_request.key.tc_hash_value(),
+                           gts_flags, &table);
 
   if (!share)
   {

sql/table_cache.cc

@@ -664,16 +664,14 @@ void tdc_unlock_share(TABLE_SHARE *share)
 */
 
 TABLE_SHARE *tdc_acquire_share(THD *thd, const char *db, const char *table_name,
-                               const char *key, uint key_length, uint flags,
+                               const char *key, uint key_length,
+                               my_hash_value_type hash_value, uint flags,
                                TABLE **out_table)
 {
   TABLE_SHARE *share;
   bool was_unused;
-  my_hash_value_type hash_value;
   DBUG_ENTER("tdc_acquire_share");
 
-  hash_value= my_calc_hash(&tdc_hash, (uchar*) key, key_length);
-
   mysql_rwlock_rdlock(&LOCK_tdc);
   share= (TABLE_SHARE*) my_hash_search_using_hash_value(&tdc_hash, hash_value,
                                                         (uchar*) key,

sql/table_cache.h

@@ -40,6 +40,7 @@ extern void tdc_unlock_share(TABLE_SHARE *share);
 extern TABLE_SHARE *tdc_acquire_share(THD *thd, const char *db,
                                       const char *table_name,
                                       const char *key, uint key_length,
+                                      my_hash_value_type hash_value,
                                       uint flags, TABLE **out_table);
 extern void tdc_release_share(TABLE_SHARE *share);
 extern bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
@@ -88,7 +89,9 @@ static inline TABLE_SHARE *tdc_acquire_share(THD *thd, const char *db,
                                              const char *key,
                                              uint key_length, uint flags)
 {
-  return tdc_acquire_share(thd, db, table_name, key, key_length, flags, 0);
+  return tdc_acquire_share(thd, db, table_name, key, key_length,
+                           my_hash_sort(&my_charset_bin, (uchar*) key,
+                                        key_length), flags, 0);
 }
 
 
@@ -120,7 +123,8 @@ static inline TABLE_SHARE *tdc_acquire_share_shortlived(THD *thd, TABLE_LIST *tl
 {
   const char *key;
   uint        key_length= get_table_def_key(tl, &key);
-  return tdc_acquire_share(thd, tl->db, tl->table_name, key, key_length, flags);
+  return tdc_acquire_share(thd, tl->db, tl->table_name, key, key_length,
+                           tl->mdl_request.key.tc_hash_value(), flags, 0);
 }
 
 

storage/maria/maria_read_log.c

@@ -301,7 +301,7 @@ get_one_option(int optid __attribute__((unused)),
     if (!my_hash_inited(&tables_to_redo))
     {
       my_hash_init2(&tables_to_redo, 16, &my_charset_bin,
-                    16, 0, 0, my_hash_get_string, 0, HASH_UNIQUE);
+                    16, 0, 0, my_hash_get_string, 0, 0, HASH_UNIQUE);
     }
     do
     {

strings/ctype-bin.c

@@ -276,7 +276,9 @@ void my_hash_sort_8bit_bin(CHARSET_INFO *cs __attribute__((unused)),
                            const uchar *key, size_t len,
                            ulong *nr1, ulong *nr2)
 {
-  
+  ulong tmp1= *nr1;
+  ulong tmp2= *nr2;
+
   /*
      Remove trailing spaces. We have to do this to be able to compare
     'A ' and 'A' as identical
@@ -285,10 +287,13 @@ void my_hash_sort_8bit_bin(CHARSET_INFO *cs __attribute__((unused)),
 
   for (; key < end ; key++)
   {
-    nr1[0]^=(ulong) ((((uint) nr1[0] & 63)+nr2[0]) * 
-	     ((uint)*key)) + (nr1[0] << 8);
-    nr2[0]+=3;
+    tmp1^= (ulong) ((((uint) tmp1 & 63) + tmp2) *
+                    ((uint) *key)) + (tmp1 << 8);
+    tmp2+= 3;
   }
+
+  *nr1= tmp1;
+  *nr2= tmp2;
 }
 
 
@@ -296,13 +301,18 @@ void my_hash_sort_bin(CHARSET_INFO *cs __attribute__((unused)),
 		      const uchar *key, size_t len,ulong *nr1, ulong *nr2)
 {
   const uchar *end = key + len;
-  
+  ulong tmp1= *nr1;
+  ulong tmp2= *nr2;
+
   for (; key < end ; key++)
   {
-    nr1[0]^=(ulong) ((((uint) nr1[0] & 63)+nr2[0]) * 
-	     ((uint)*key)) + (nr1[0] << 8);
-    nr2[0]+=3;
+    tmp1^= (ulong) ((((uint) tmp1 & 63) + tmp2) *
+                    ((uint) *key)) + (tmp1 << 8);
+    tmp2+= 3;
   }
+
+  *nr1= tmp1;
+  *nr2= tmp2;
 }