maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sergey!

Looks pretty much ok. See few comments/questions below.

On Oct 24, Sergey Vojtovich wrote:
> ------------------------------------------------------------
> revno: 3807
> revision-id: svoj@xxxxxxxxxxx-20130827121233-xh1uyhgfwbhedqyf
> parent: jplindst@xxxxxxxxxxx-20130823060357-pww92qxla7o8iir7
> committer: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> branch nick: 10.0
> timestamp: Tue 2013-08-27 16:12:33 +0400
> message:
>   MDEV-4956 - Reduce usage of LOCK_open: TABLE_SHARE::tdc.used_tables
>   
>   - tc_acquire_table and tc_release_table do not access
>     TABLE_SHARE::tdc.used_tables anymore

ok

>   - in tc_acquire_table(): release LOCK_tdc after we relase LOCK_open
>     (saves a few CPU cycles in critical section)

This is a bit counter-intuitive. Your change is to hold the lock more
than necessary. Does it give a measurable performance improvement?

>   - in tc_release_table(): if we reached table cache threshold, evict
>     to-be-released table without moving it to unused_tables. unused_tables
>     must be empty at this point (added assertion).

Why unused_tables must be empty at this point?

> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc	2013-08-14 08:48:50 +0000
> +++ b/sql/sql_base.cc	2013-08-27 12:12:33 +0000
> @@ -370,7 +372,7 @@ void free_io_cache(TABLE *table)
>  
>  void kill_delayed_threads_for_table(TABLE_SHARE *share)
>  {
> -  TABLE_SHARE::TABLE_list::Iterator it(share->tdc.used_tables);
> +  TABLE_SHARE::All_share_tables_list::Iterator it(share->tdc.all_tables);

In this function, I'd suggest to add some flag to be able to skip the
mutex and the loop altogether - as in the absolute majority of cases
there will be no delayed insert thread to kill.

Either use a global flag to know if there's a delayed thread running,
or create a per-share flag.

>    TABLE *tab;
>  
Regards,
Sergei