Hi, Sergey! Did you benchmark the improvement you get with this patch? On Dec 05, Sergey Vojtovich wrote:
At lp:maria/10.0
------------------------------------------------------------ revno: 3915 revision-id: svoj@mariadb.org-20131205084430-7rt735tbanw0i6uq parent: svoj@mariadb.org-20130827121233-b9mguuxctnjlq5wd committer: Sergey Vojtovich <svoj@mariadb.org> branch nick: 10.0 timestamp: Thu 2013-12-05 12:44:30 +0400 message: MDEV-5388 - Reduce usage of LOCK_open: unused_tables
Removed unused_tables, find LRU object by timestamp instead.
=== modified file 'sql/table_cache.cc' --- a/sql/table_cache.cc 2013-08-27 12:12:33 +0000 +++ b/sql/table_cache.cc 2013-12-05 08:44:30 +0000 @@ -324,20 +248,44 @@ void tc_add_table(THD *thd, TABLE *table DBUG_ASSERT(table->in_use == thd); mysql_mutex_lock(&LOCK_open); table->s->tdc.all_tables.push_front(table); - tc_count++; /* If we have too many TABLE instances around, try to get rid of them */ - if (tc_count > tc_size && unused_tables) + if (tc_count == tc_size)
I'd rather put (tc_count >= tc_size), looks safer.
{ - TABLE *purge_table= unused_tables; - tc_remove_table(purge_table); - mysql_rwlock_rdlock(&LOCK_flush); + TDC_iterator tdc_it; mysql_mutex_unlock(&LOCK_open); - intern_close_table(purge_table); - mysql_rwlock_unlock(&LOCK_flush); - check_unused(thd); + + tdc_it.init(); + mysql_mutex_lock(&LOCK_open); + if (tc_count == tc_size)
and here
+ { + TABLE *purge_table= 0; + TABLE_SHARE *share;
I think a status variable for this would be nice. So that one could make an informed decision about the desired size of the cache.
+ while ((share= tdc_it.next())) + { + TABLE_SHARE::TABLE_list::Iterator it(share->tdc.free_tables); + TABLE *entry; + while ((entry= it++)) + if (!purge_table || entry->tc_time < purge_table->tc_time) + purge_table= entry; + } + if (purge_table) + { + tdc_it.deinit(); + purge_table->s->tdc.free_tables.remove(purge_table); + purge_table->s->tdc.all_tables.remove(purge_table); + mysql_rwlock_rdlock(&LOCK_flush); + mysql_mutex_unlock(&LOCK_open); + intern_close_table(purge_table); + mysql_rwlock_unlock(&LOCK_flush); + check_unused(thd); + return; + } + } + tdc_it.deinit();
Okay, so you only purge one table at time? Since purge is kind of an expensive operation, it's better to amortize the cost of it by purging many tables at once.
} - else - mysql_mutex_unlock(&LOCK_open); + /* Nothing to evict, increment tc_count. */ + tc_count++; + mysql_mutex_unlock(&LOCK_open); }
2 Regards, Sergei