maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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@xxxxxxxxxxx-20131205084430-7rt735tbanw0i6uq
> parent: svoj@xxxxxxxxxxx-20130827121233-b9mguuxctnjlq5wd
> committer: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> 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