maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi!

The second patch should be on it's way to commits. Also I wrote a blog about it:
http://svoj-db.blogspot.ru/2014/02/mariadb-100-performance-myhashsortbin.html

Regards,
Sergey

On Fri, Feb 14, 2014 at 04:30:29PM +0400, Sergey Vojtovich wrote:
> Hi!
> 
> JFYI: we discussed this during the Barcelona meeting. I will submit table cache
> part separately (not to overcomplicate this patch). Reviewers are very welcome.
> 
> Regards,
> Sergey
> 
> 
> ----- Forwarded message from Sergey Vojtovich <svoj@xxxxxxxxxxx> -----
> 
> Date: Fri, 14 Feb 2014 12:25:50 +0000 (UTC)
> From: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> To: commits@xxxxxxxxxxx
> Subject: [Commits] Rev 4006: MDEV-5674 - Performance: my_hash_sort_bin is called too often in lp:maria/10.0
> 
> At lp:maria/10.0
> 
> ------------------------------------------------------------
> revno: 4006
> revision-id: svoj@xxxxxxxxxxx-20140214122541-tenvcllk760deai8
> parent: svoj@xxxxxxxxxxx-20140213071355-psk0jpsarx2u7myc
> committer: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> branch nick: 10.0
> timestamp: Fri 2014-02-14 16:25:41 +0400
> message:
>   MDEV-5674 - Performance: my_hash_sort_bin is called too often
>   
>   - reduced number of my_hash_sort_bin() calls from 4 to 2 per query.
>     Let MDL subsystem use pre-calculated hash value for hash
>     inserts and deletes.
>   - reduced number of memory accesses done by my_hash_sort_bin()
> === modified file 'include/hash.h'
> --- a/include/hash.h	2013-09-14 01:09:36 +0000
> +++ b/include/hash.h	2014-02-14 12:25:41 +0000
> @@ -86,7 +86,13 @@ uchar *my_hash_first_from_hash_value(con
>  uchar *my_hash_next(const HASH *info, const uchar *key, size_t length,
>                      HASH_SEARCH_STATE *state);
>  my_bool my_hash_insert(HASH *info, const uchar *data);
> +my_bool my_hash_insert_using_hash_value(HASH *info,
> +                                        my_hash_value_type hash_value,
> +                                        const uchar *record);
>  my_bool my_hash_delete(HASH *hash, uchar *record);
> +my_bool my_hash_delete_using_hash_value(HASH *hash,
> +                                        my_hash_value_type hash_value,
> +                                        uchar *record);
>  my_bool my_hash_update(HASH *hash, uchar *record, uchar *old_key,
>                         size_t old_key_length);
>  void my_hash_replace(HASH *hash, HASH_SEARCH_STATE *state, uchar *new_row);
> 
> === modified file 'mysys/hash.c'
> --- a/mysys/hash.c	2013-09-14 01:09:36 +0000
> +++ b/mysys/hash.c	2014-02-14 12:25:41 +0000
> @@ -380,6 +380,15 @@ static int hashcmp(const HASH *hash, HAS
>  
>  my_bool my_hash_insert(HASH *info, const uchar *record)
>  {
> +  return my_hash_insert_using_hash_value(info, rec_hashnr(info, record),
> +                                         record);
> +}
> +
> +
> +my_bool my_hash_insert_using_hash_value(HASH *info,
> +                                        my_hash_value_type hash_value,
> +                                        const uchar *record)
> +{
>    int flag;
>    size_t idx,halfbuff,first_index;
>    my_hash_value_type hash_nr;
> @@ -480,7 +489,7 @@ my_bool my_hash_insert(HASH *info, const
>    }
>    /* Check if we are at the empty position */
>  
> -  idx= my_hash_mask(rec_hashnr(info, record), info->blength, info->records + 1);
> +  idx= my_hash_mask(hash_value, info->blength, info->records + 1);
>    pos=data+idx;
>    if (pos == empty)
>    {
> @@ -528,6 +537,15 @@ my_bool my_hash_insert(HASH *info, const
>  
>  my_bool my_hash_delete(HASH *hash, uchar *record)
>  {
> +  return my_hash_delete_using_hash_value(hash, rec_hashnr(hash, record),
> +                                         record);
> +}
> +
> +
> +my_bool my_hash_delete_using_hash_value(HASH *hash,
> +                                        my_hash_value_type hash_value,
> +                                        uchar *record)
> +{
>    uint pos2,idx,empty_index;
>    my_hash_value_type pos_hashnr, lastpos_hashnr;
>    size_t blength;
> @@ -539,7 +557,7 @@ my_bool my_hash_delete(HASH *hash, uchar
>    blength=hash->blength;
>    data=dynamic_element(&hash->array,0,HASH_LINK*);
>    /* Search after record with key */
> -  pos= data + my_hash_mask(rec_hashnr(hash, record), blength, hash->records);
> +  pos= data + my_hash_mask(hash_value, blength, hash->records);
>    gpos = 0;
>  
>    while (pos->data != record)
> 
> === modified file 'sql/mdl.cc'
> --- a/sql/mdl.cc	2013-12-10 15:31:04 +0000
> +++ b/sql/mdl.cc	2014-02-14 12:25:41 +0000
> @@ -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. */
> @@ -846,11 +840,10 @@ MDL_lock* MDL_map::find_or_insert(const
>      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 +856,14 @@ MDL_lock* MDL_map::find_or_insert(const
>    @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())))
>    {
> @@ -902,7 +894,9 @@ MDL_lock* MDL_map_partition::find_or_ins
>        lock= MDL_lock::create(mdl_key, this);
>      }
>  
> -    if (!lock || my_hash_insert(&m_locks, (uchar*)lock))
> +    if (!lock || my_hash_insert_using_hash_value(&m_locks,
> +                                                 mdl_key->hash_value(),
> +                                                 (uchar*) lock))
>      {
>        if (unused_lock)
>        {
> @@ -1023,10 +1017,9 @@ MDL_map::get_lock_owner(const MDL_key *m
>    }
>    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 +1027,14 @@ MDL_map::get_lock_owner(const MDL_key *m
>  
>  
>  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)
> @@ -1085,7 +1077,8 @@ void MDL_map::remove(MDL_lock *lock)
>  void MDL_map_partition::remove(MDL_lock *lock)
>  {
>    mysql_mutex_lock(&m_mutex);
> -  my_hash_delete(&m_locks, (uchar*) lock);
> +  my_hash_delete_using_hash_value(&m_locks, lock->key.hash_value(),
> +                                  (uchar*) lock);
>    /*
>      To let threads holding references to the MDL_lock object know that it was
>      moved to the list of unused objects or destroyed, we increment the version
> 
> === modified file 'sql/mdl.h'
> --- a/sql/mdl.h	2013-12-16 08:26:20 +0000
> +++ b/sql/mdl.h	2014-02-14 12:25:41 +0000
> @@ -347,12 +347,17 @@ class MDL_key
>                                            m_ptr - 1);
>      m_length= static_cast<uint16>(strmake(m_ptr + m_db_name_length + 2, name,
>                                            NAME_LEN) - m_ptr + 1);
> +    ulong nr2= 4;
> +    m_hash_value= 1;
> +    my_charset_bin.coll->hash_sort(&my_charset_bin, (uchar*) m_ptr, m_length,
> +                                   &m_hash_value, &nr2);
>    }
>    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,10 +397,15 @@ class MDL_key
>    {
>      return & m_namespace_to_wait_state_name[(int)mdl_namespace()];
>    }
> +  ulong hash_value() const
> +  {
> +    return m_hash_value;
> +  }
>  
>  private:
>    uint16 m_length;
>    uint16 m_db_name_length;
> +  ulong m_hash_value;
>    char m_ptr[MAX_MDLKEY_LENGTH];
>    static PSI_stage_info m_namespace_to_wait_state_name[NAMESPACE_END];
>  private:
> 
> === modified file 'strings/ctype-bin.c'
> --- a/strings/ctype-bin.c	2013-11-20 11:05:39 +0000
> +++ b/strings/ctype-bin.c	2014-02-14 12:25:41 +0000
> @@ -277,7 +277,9 @@ void my_hash_sort_8bit_bin(CHARSET_INFO
>                             ulong *nr1, ulong *nr2)
>  {
>    const uchar *pos = key;
> -  
> +  ulong tmp1= *nr1;
> +  ulong tmp2= *nr2;
> +
>    /*
>       Remove trailing spaces. We have to do this to be able to compare
>      'A ' and 'A' as identical
> @@ -286,10 +288,13 @@ void my_hash_sort_8bit_bin(CHARSET_INFO
>  
>    for (; pos < (uchar*) key ; pos++)
>    {
> -    nr1[0]^=(ulong) ((((uint) nr1[0] & 63)+nr2[0]) * 
> -	     ((uint)*pos)) + (nr1[0] << 8);
> -    nr2[0]+=3;
> +    tmp1^= (ulong) ((((uint) tmp1 & 63) + tmp2) *
> +                    ((uint) *pos)) + (tmp1 << 8);
> +    tmp2+= 3;
>    }
> +
> +  *nr1= tmp1;
> +  *nr2= tmp2;
>  }
>  
>  
> @@ -297,15 +302,20 @@ void my_hash_sort_bin(CHARSET_INFO *cs _
>  		      const uchar *key, size_t len,ulong *nr1, ulong *nr2)
>  {
>    const uchar *pos = key;
> -  
> +  ulong tmp1= *nr1;
> +  ulong tmp2= *nr2;
> +
>    key+= len;
> -  
> +
>    for (; pos < (uchar*) key ; pos++)
>    {
> -    nr1[0]^=(ulong) ((((uint) nr1[0] & 63)+nr2[0]) * 
> -	     ((uint)*pos)) + (nr1[0] << 8);
> -    nr2[0]+=3;
> +    tmp1^= (ulong) ((((uint) tmp1 & 63) + tmp2) *
> +                    ((uint) *pos)) + (tmp1 << 8);
> +    tmp2+= 3;
>    }
> +
> +  *nr1= tmp1;
> +  *nr2= tmp2;
>  }
>  
>  
> 
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
> 
> ----- End forwarded message -----
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp