Hi, Sergey! On Mar 15, Sergey Vojtovich wrote:
At lp:maria/10.0
------------------------------------------------------------ revno: 4053 revision-id: svoj@mariadb.org-20140315185301-ce4r6br023co80dm parent: sanja@montyprogram.com-20140314073116-nklb0jkjd4t1w40n committer: Sergey Vojtovich <svoj@mariadb.org> branch nick: 10.0 timestamp: Sat 2014-03-15 22:53:01 +0400 message: MDEV-5864 - Reduce usage of LOCK_open: TABLE_SHARE::tdc.free_tables
Let TABLE_SHARE::tdc.free_tables, TABLE_SHARE::tdc.all_tables, TABLE_SHARE::tdc.flushed and corresponding invariants be protected by per-share TABLE_SHARE::tdc.LOCK_table_share instead of global LOCK_open.
This looks pretty good to me! A couple of comments, see below.
=== modified file 'sql/sql_plist.h' --- a/sql/sql_plist.h 2013-11-20 11:05:39 +0000 +++ b/sql/sql_plist.h 2014-03-15 18:53:01 +0000 @@ -142,6 +142,14 @@ class I_P_List : public C, public I
return result; } + inline T *back() + { + T *result= m_first; + if (result) + while (*B::next_ptr(result)) + result= *B::next_ptr(result); + return result; + }
I think this is wrong. The correct way would be to use I_P_List<> with the I_P_List_fast_push_back policy, then you could use have get_last() method to get the last element in the list. May be as inline T *back() { return *I::get_last(); }
void swap(I_P_List<T, B, C> &rhs) { swap_variables(T *, m_first, rhs.m_first);
=== modified file 'sql/table_cache.cc' --- a/sql/table_cache.cc 2014-03-06 12:19:12 +0000 +++ b/sql/table_cache.cc 2014-03-15 18:53:01 +0000 @@ -70,13 +70,6 @@ static int32 tc_count; /**< Number of TA
/** - Protects TABLE_SHARE::tdc.free_tables, TABLE_SHARE::tdc.all_tables. -*/ - -mysql_mutex_t LOCK_open;
So, you've now removed LOCK_open completely? Cool!
- - -/** Protects unused shares list.
TABLE_SHARE::tdc.prev @@ -360,12 +378,15 @@ bool tc_release_table(TABLE *table) table->in_use= 0; /* Add table to the list of unused TABLE objects for this share. */ table->s->tdc.free_tables.push_front(table); - mysql_mutex_unlock(&LOCK_open); + mysql_mutex_unlock(&table->s->tdc.LOCK_table_share); return false;
purge: + /* Wait for MDL deadlock detector */ + while (table->s->tdc.all_tables_refs) + mysql_cond_wait(&table->s->tdc.COND_release, &table->s->tdc.LOCK_table_share);
you're using it many times, I'd put it in a small static inline function, like static wait_for_mdl_deadlock_detector(TABLE *table) { mysql_mutex_assert_owner(&table->s->tdc.LOCK_table_share); while (table->s->tdc.all_tables_refs) mysql_cond_wait(&table->s->tdc.COND_release, &table->s->tdc.LOCK_table_share); }
tc_remove_table(table); - mysql_mutex_unlock(&LOCK_open); + mysql_mutex_unlock(&table->s->tdc.LOCK_table_share); table->in_use= 0; intern_close_table(table); return true;
Regards, Sergei