Hi Sergei, thanks for your comments, answers inline.
On Tue, Dec 10, 2013 at 10:14:32PM +0100, Sergei Golubchik wrote: > Hi, Sergey! > > Did you benchmark the improvement you get with this patch? Yes, my results against recent 10.0 are in jira task description. Also please find Axel's results (MDEV-4956 + MDEV-5388) attached. >From my results please note that mutex wait time dropped almost 2x. >From Axel's results please note performance regression between 10.0.4 and 10.0.5. With 10.0.4 we've seen much better performance improvement. I suppose that 10.0.5 regression doesn't permit to go over certain threshold. I can benchmark 10.0.4 + MDEV-4956 + MDEV-5388 again if you like. > On Dec 05, Sergey Vojtovich wrote: > > At lp:maria/10.0 > > > > ------------------------------------------------------------ > > revno: 3915 > > revision-id: [email protected] > > parent: [email protected] > > committer: Sergey Vojtovich <[email protected]> > > 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. (tc_count > tc_size) means all instances are used and there is nothing to evict. tc_release_table() guards this assertion. Anyway I changed it to ">=" in the next patch (re tc_count). This assertion is not valid there anymore. > > > { > > - 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. Agree, added to my todo. > > + 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. With current algorithm it is impossible to get multiple tables for purge here, explanation above. Thanks, Sergey
sysbench45c.ods
Description: application/vnd.oasis.opendocument.spreadsheet
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
