Re: [Maria-developers] Rev 3965: MDEV-5403 - Reduce usage of LOCK_open: tc_count

Hi, Sergey! Please, see below. On Jan 30, Sergey Vojtovich wrote:

revno: 3965 revision-id: svoj@mariadb.org-20140130123642-wsysj31e1wwlqz0o parent: psergey@askmonty.org-20140120123657-g1z4g11xmsdiw2dc committer: Sergey Vojtovich branch nick: 10.0 timestamp: Thu 2014-01-30 16:36:42 +0400 message: MDEV-5403 - Reduce usage of LOCK_open: tc_count

Lock-free tc_count. === modified file 'sql/table_cache.cc' --- a/sql/table_cache.cc 2013-12-12 17:49:14 +0000 +++ b/sql/table_cache.cc 2014-01-30 12:36:42 +0000 @@ -173,7 +175,9 @@ void tc_purge(void) { share->tdc.all_tables.remove(table); purge_tables.push_front(table); - tc_count--; + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); + my_atomic_add32(&tc_count, -1); + my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics);

Okay, so in this new code tc_count is always equal or greater than the actual number of tables in the cache, right? (because you first remove the table from the cache, then decrement the counter)

} } tdc_it.deinit(); @@ -245,47 +249,52 @@ static void check_unused(THD *thd)

void tc_add_table(THD *thd, TABLE *table) { + bool need_purge; DBUG_ASSERT(table->in_use == thd); mysql_mutex_lock(&LOCK_open); table->s->tdc.all_tables.push_front(table); + mysql_mutex_unlock(&LOCK_open); + /* If we have too many TABLE instances around, try to get rid of them */ - if (tc_count == tc_size) + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); + need_purge= my_atomic_add32(&tc_count, 1) >= (int32) tc_size;

Hmm, but here you increment after adding a table. I'd suggest to keep tc_count consistently either always no smaller than the number of tables in the cache (that is, decrement after removing, increment before adding), or always no larger than that (derement before removing, increment after adding).

+ my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics); + + if (need_purge) { + TABLE *purge_table= 0; + TABLE_SHARE *share; TDC_iterator tdc_it; - mysql_mutex_unlock(&LOCK_open);

tdc_it.init(); mysql_mutex_lock(&LOCK_open); - if (tc_count == tc_size) - { - TABLE *purge_table= 0; - TABLE_SHARE *share; 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; } + tdc_it.deinit(); + 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); + + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); + my_atomic_add32(&tc_count, -1); + my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics); check_unused(thd); - return; } - } - tdc_it.deinit(); - } - /* Nothing to evict, increment tc_count. */ - tc_count++; + else mysql_mutex_unlock(&LOCK_open); + } }

@@ -357,25 +366,35 @@ bool tc_release_table(TABLE *table) DBUG_ASSERT(table->in_use); DBUG_ASSERT(table->file);

+ if (table->needs_reopen() || tc_records() > tc_size) + { + mysql_mutex_lock(&LOCK_open); + table->in_use= 0; + goto purge; + }

Why did you move this up?

table->tc_time= my_interval_timer();

mysql_mutex_lock(&LOCK_open); table->in_use= 0; - if (table->s->has_old_version() || table->needs_reopen() || tc_count > tc_size) - { - tc_count--; + if (table->s->has_old_version()) + goto purge; + /* 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); + check_unused(thd); + return false; + +purge: + my_atomic_rwlock_wrlock(&LOCK_tdc_atomics); + my_atomic_add32(&tc_count, -1); + my_atomic_rwlock_wrunlock(&LOCK_tdc_atomics); table->s->tdc.all_tables.remove(table);

and now you decrement before removal

mysql_rwlock_rdlock(&LOCK_flush); mysql_mutex_unlock(&LOCK_open); intern_close_table(table); mysql_rwlock_unlock(&LOCK_flush); return true; - } - /* 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); - check_unused(thd); - return false; }

Regards, Sergei