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 <s...@mariadb.org> -----
>
> Date: Fri, 14 Feb 2014 12:25:50 +0000 (UTC)
> From: Sergey Vojtovich <s...@mariadb.org>
> To: comm...@mariadb.org
> 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: s...@mariadb.org-20140214122541-tenvcllk760deai8
> parent: s...@mariadb.org-20140213071355-psk0jpsarx2u7myc
> committer: Sergey Vojtovich <s...@mariadb.org>
> 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
> comm...@mariadb.org
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
>
> ----- End forwarded message -----
>
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to : maria-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~maria-developers
> More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
