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
- in tc_acquire_table(): release LOCK_tdc after we relase LOCK_open
  (saves a few CPU cycles in critical section)
- 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.

sql/sql_base.cc

@@ -309,8 +309,10 @@ OPEN_TABLE_LIST *list_open_tables(THD *thd, const char *db, const char *wild)
 	   share->table_name.str);
     (*start_list)->in_use= 0;
     mysql_mutex_lock(&LOCK_open);
-    TABLE_SHARE::TABLE_list::Iterator it(share->tdc.used_tables);
-    while (it++)
+    TABLE_SHARE::All_share_tables_list::Iterator it(share->tdc.all_tables);
+    TABLE *table;
+    while ((table= it++))
+      if (table->in_use)
         ++(*start_list)->in_use;
     mysql_mutex_unlock(&LOCK_open);
     (*start_list)->locked= 0;                   /* Obsolete. */
@@ -371,16 +373,19 @@ 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);
   TABLE *tab;
 
   mysql_mutex_assert_owner(&LOCK_open);
 
+  if (!delayed_insert_threads)
+    return;
+
   while ((tab= it++))
   {
     THD *in_use= tab->in_use;
 
-    if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) &&
+    if (in_use && (in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) &&
         ! in_use->killed)
     {
       in_use->killed= KILL_SYSTEM_THREAD;

sql/sql_test.cc

@@ -92,16 +92,17 @@ static void print_cached_tables(void)
   mysql_mutex_lock(&LOCK_open);
   while ((share= tdc_it.next()))
   {
-    TABLE_SHARE::TABLE_list::Iterator it(share->tdc.used_tables);
+    TABLE_SHARE::All_share_tables_list::Iterator it(share->tdc.all_tables);
     while ((entry= it++))
+    {
+      if (entry->in_use)
       {
         printf("%-14.14s %-32s%6ld%8ld%6d  %s\n",
                entry->s->db.str, entry->s->table_name.str, entry->s->version,
                entry->in_use->thread_id, entry->db_stat ? 1 : 0,
                lock_descriptions[(int)entry->reginfo.lock_type]);
       }
-    it.init(share->tdc.free_tables);
-    while ((entry= it++))
+      else
       {
         unused++;
         printf("%-14.14s %-32s%6ld%8ld%6d  %s\n",
@@ -109,6 +110,7 @@ static void print_cached_tables(void)
                0L, entry->db_stat ? 1 : 0, "Not in use");
       }
     }
+  }
   tdc_it.deinit();
   if ((start_link=lnk=unused_tables))
   {

sql/sys_vars.cc

@@ -2859,11 +2859,22 @@ static Sys_var_ulong Sys_table_def_size(
        VALID_RANGE(TABLE_DEF_CACHE_MIN, 512*1024),
        DEFAULT(TABLE_DEF_CACHE_DEFAULT), BLOCK_SIZE(1));
 
+
+static bool fix_table_open_cache(sys_var *, THD *, enum_var_type)
+{
+  mysql_mutex_unlock(&LOCK_global_system_variables);
+  tc_purge();
+  mysql_mutex_lock(&LOCK_global_system_variables);
+  return false;
+}
+
+
 static Sys_var_ulong Sys_table_cache_size(
        "table_open_cache", "The number of cached open tables",
        GLOBAL_VAR(tc_size), CMD_LINE(REQUIRED_ARG),
        VALID_RANGE(1, 512*1024), DEFAULT(TABLE_OPEN_CACHE_DEFAULT),
-       BLOCK_SIZE(1));
+       BLOCK_SIZE(1), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
+       ON_UPDATE(fix_table_open_cache));
 
 static Sys_var_ulong Sys_thread_cache_size(
        "thread_cache_size",

sql/table.cc

@@ -3817,7 +3817,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
   if (gvisitor->m_lock_open_count++ == 0)
     mysql_mutex_lock(&LOCK_open);
 
-  TABLE_list::Iterator tables_it(tdc.used_tables);
+  All_share_tables_list::Iterator tables_it(tdc.all_tables);
 
   /*
     In case of multiple searches running in parallel, avoid going
@@ -3835,7 +3835,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
 
   while ((table= tables_it++))
   {
-    if (gvisitor->inspect_edge(&table->in_use->mdl_context))
+    if (table->in_use && gvisitor->inspect_edge(&table->in_use->mdl_context))
     {
       goto end_leave_node;
     }
@@ -3844,7 +3844,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
   tables_it.rewind();
   while ((table= tables_it++))
   {
-    if (table->in_use->mdl_context.visit_subgraph(gvisitor))
+    if (table->in_use && table->in_use->mdl_context.visit_subgraph(gvisitor))
     {
       goto end_leave_node;
     }

sql/table.h

@@ -479,6 +479,7 @@ TABLE_CATEGORY get_table_category(const LEX_STRING *db,
 
 
 struct TABLE_share;
+struct All_share_tables;
 
 extern ulong tdc_refresh_version(void);
 
@@ -603,6 +604,7 @@ struct TABLE_SHARE
   mysql_mutex_t LOCK_share;             /* To protect TABLE_SHARE */
 
   typedef I_P_List <TABLE, TABLE_share> TABLE_list;
+  typedef I_P_List <TABLE, All_share_tables> All_share_tables_list;
   struct
   {
     /**
@@ -619,7 +621,7 @@ struct TABLE_SHARE
       Doubly-linked (back-linked) lists of used and unused TABLE objects
       for this share.
     */
-    TABLE_list used_tables;
+    All_share_tables_list all_tables;
     TABLE_list free_tables;
   } tdc;
 
@@ -1014,8 +1016,10 @@ struct TABLE
      One should use methods of I_P_List template instead.
   */
   TABLE *share_next, **share_prev;
+  TABLE *share_all_next, **share_all_prev;
 
   friend struct TABLE_share;
+  friend struct All_share_tables;
 
 public:
 
@@ -1379,6 +1383,19 @@ struct TABLE_share
 };
 
 
+struct All_share_tables
+{
+  static inline TABLE **next_ptr(TABLE *l)
+  {
+    return &l->share_all_next;
+  }
+  static inline TABLE ***prev_ptr(TABLE *l)
+  {
+    return &l->share_all_prev;
+  }
+};
+
+
 enum enum_schema_table_state
 { 
   NOT_PROCESSED= 0,

sql/table_cache.cc

@@ -43,11 +43,8 @@
   - free_table_share()
 
   Table cache invariants:
-  - TABLE_SHARE::used_tables shall not contain objects with TABLE::in_use == 0
   - TABLE_SHARE::free_tables shall not contain objects with TABLE::in_use != 0
   - unused_tables shall not contain objects with TABLE::in_use != 0
-  - cached TABLE object must be either in TABLE_SHARE::used_tables or in
-    TABLE_SHARE::free_tables
 */
 
 #include "my_global.h"
@@ -81,7 +78,7 @@ static uint tc_count; /**< Number of TABLE objects in table cache. */
   TABLE::next
   TABLE::prev
   TABLE_SHARE::tdc.free_tables
-  TABLE_SHARE::tdc.used_tables
+  TABLE_SHARE::tdc.all_tables
 */
 
 mysql_mutex_t LOCK_open;
@@ -159,7 +156,8 @@ uint tc_records(void)
   Free all unused TABLE objects.
 
   While locked:
-  - remove unused objects from TABLE_SHARE::tdc.free_tables lists
+  - remove unused objects from TABLE_SHARE::tdc.free_tables and
+    TABLE_SHARE::tdc.all_tables
   - reset unused_tables
   - decrement tc_count
 
@@ -180,6 +178,7 @@ void tc_purge(void)
     do
     {
       unused_tables->s->tdc.free_tables.remove(unused_tables);
+      unused_tables->s->tdc.all_tables.remove(unused_tables);
       tc_count--;
     } while ((unused_tables= unused_tables->next));
     mysql_rwlock_rdlock(&LOCK_flush);
@@ -249,13 +248,10 @@ static void check_unused(THD *thd)
       count--;
       open_files++;
     }
-    it.init(share->tdc.used_tables);
-    while ((entry= it++))
-    {
-      if (!entry->in_use)
+    TABLE_SHARE::All_share_tables_list::Iterator it2(share->tdc.all_tables);
+    while ((entry= it2++))
     {
-        DBUG_PRINT("error",("Unused table is in share's list of used tables")); /* purecov: inspected */
-      }
+      if (entry->in_use)
         open_files++;
     }
   }
@@ -278,7 +274,8 @@ static void check_unused(THD *thd)
   @pre LOCK_open is locked, table is not used.
 
   While locked:
-  - remove object from TABLE_SHARE::tdc.free_tables
+  - remove object from TABLE_SHARE::tdc.free_tables and
+    TABLE_SHARE::tdc.all_tables
   - remove object from unused_tables
 
   @note This is helper routine, supposed to be used by table cache
@@ -291,6 +288,7 @@ static void tc_remove_table(TABLE *table)
   DBUG_ASSERT(!table->in_use);
   /* Remove from per-share chain of unused TABLE objects. */
   table->s->tdc.free_tables.remove(table);
+  table->s->tdc.all_tables.remove(table);
 
   /* And global unused chain. */
   table->next->prev= table->prev;
@@ -313,7 +311,7 @@ static void tc_remove_table(TABLE *table)
   Added object cannot be evicted or acquired.
 
   While locked:
-  - add object to TABLE_SHARE::tdc.used_tables
+  - add object to TABLE_SHARE::tdc.all_tables
   - increment tc_count
   - evict LRU object from table cache if we reached threshold
 
@@ -325,7 +323,7 @@ void tc_add_table(THD *thd, TABLE *table)
 {
   DBUG_ASSERT(table->in_use == thd);
   mysql_mutex_lock(&LOCK_open);
-  table->s->tdc.used_tables.push_front(table);
+  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)
@@ -352,13 +350,9 @@ void tc_add_table(THD *thd, TABLE *table)
 
   While locked:
   - pop object from TABLE_SHARE::tdc.free_tables()
-  - remove share protection
   - remove object from unused_tables
-  - add object to TABLE_SHARE::tdc.used_tables()
   - mark object used by thd
 
-  @note share protection is kept if there are no unused objects.
-
   @return TABLE object, or NULL if no unused objects.
 */
 
@@ -372,7 +366,6 @@ static TABLE *tc_acquire_table(THD *thd, TABLE_SHARE *share)
     mysql_mutex_unlock(&LOCK_open);
     return 0;
   }
-  mysql_rwlock_unlock(&LOCK_tdc);
   DBUG_ASSERT(!table->in_use);
 
   /* Unlink table from global unused tables list. */
@@ -385,15 +378,12 @@ static TABLE *tc_acquire_table(THD *thd, TABLE_SHARE *share)
   table->prev->next=table->next;		/* Remove from unused list */
   table->next->prev=table->prev;
   table->in_use= thd;
-  /* Add table to list of used tables for this share. */
-  table->s->tdc.used_tables.push_front(table);
   mysql_mutex_unlock(&LOCK_open);
 
   /* The ex-unused table must be fully functional. */
   DBUG_ASSERT(table->db_stat && table->file);
   /* The children must be detached from the table. */
   DBUG_ASSERT(! table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN));
-  check_unused(thd);
   return table;
 }
 
@@ -407,7 +397,6 @@ static TABLE *tc_acquire_table(THD *thd, TABLE_SHARE *share)
 
   While locked:
   - mark object not in use by any thread
-  - remove object from TABLE_SHARE::tdc.used_tables
   - if object is marked for purge, decrement tc_count
   - add object to TABLE_SHARE::tdc.free_tables
   - add object to unused_tables
@@ -433,12 +422,12 @@ bool tc_release_table(TABLE *table)
   DBUG_ASSERT(table->file);
 
   mysql_mutex_lock(&LOCK_open);
-  /* Remove table from the list of tables used in this share. */
-  table->s->tdc.used_tables.remove(table);
   table->in_use= 0;
-  if (table->s->has_old_version() || table->needs_reopen() || !tdc_size)
+  if (table->s->has_old_version() || table->needs_reopen() || !tdc_size ||
+      tc_count > tc_size)
   {
     tc_count--;
+    table->s->tdc.all_tables.remove(table);
     mysql_rwlock_rdlock(&LOCK_flush);
     mysql_mutex_unlock(&LOCK_open);
     intern_close_table(table);
@@ -457,20 +446,6 @@ bool tc_release_table(TABLE *table)
   }
   else
     unused_tables=table->next=table->prev=table;
-  /*
-    We free the least used table, not the subject table,
-    to keep the LRU order.
-  */
-  if (tc_count > tc_size)
-  {
-    TABLE *purge_table= unused_tables;
-    tc_remove_table(purge_table);
-    mysql_rwlock_rdlock(&LOCK_flush);
-    mysql_mutex_unlock(&LOCK_open);
-    intern_close_table(purge_table);
-    mysql_rwlock_unlock(&LOCK_flush);
-  }
-  else
   mysql_mutex_unlock(&LOCK_open);
   check_unused(thd);
   return false;
@@ -636,19 +611,9 @@ ulong tdc_records(void)
 void tdc_purge(bool all)
 {
   DBUG_ENTER("tdc_purge");
-  for (;;)
+  while (all || tdc_records() > tdc_size)
   {
     TABLE_SHARE *share;
-    if (!all)
-    {
-      mysql_rwlock_rdlock(&LOCK_tdc);
-      if (tdc_hash.records <= tdc_size)
-      {
-        mysql_rwlock_unlock(&LOCK_tdc);
-        break;
-      }
-      mysql_rwlock_unlock(&LOCK_tdc);
-    }
 
     mysql_mutex_lock(&LOCK_unused_shares);
     if (!oldest_unused_share->tdc.next)
@@ -684,7 +649,7 @@ void tdc_init_share(TABLE_SHARE *share)
   mysql_mutex_init(key_TABLE_SHARE_LOCK_table_share,
                    &share->tdc.LOCK_table_share, MY_MUTEX_INIT_FAST);
   share->tdc.m_flush_tickets.empty();
-  share->tdc.used_tables.empty();
+  share->tdc.all_tables.empty();
   share->tdc.free_tables.empty();
   tdc_assign_new_table_id(share);
   share->version= tdc_refresh_version();
@@ -701,7 +666,7 @@ void tdc_deinit_share(TABLE_SHARE *share)
   DBUG_ENTER("tdc_deinit_share");
   DBUG_ASSERT(share->tdc.ref_count == 0);
   DBUG_ASSERT(share->tdc.m_flush_tickets.is_empty());
-  DBUG_ASSERT(share->tdc.used_tables.is_empty());
+  DBUG_ASSERT(share->tdc.all_tables.is_empty());
   DBUG_ASSERT(share->tdc.free_tables.is_empty());
   mysql_mutex_destroy(&share->tdc.LOCK_table_share);
   DBUG_VOID_RETURN;
@@ -846,6 +811,8 @@ TABLE_SHARE *tdc_acquire_share(THD *thd, const char *db, const char *table_name,
   {
     if ((*out_table= tc_acquire_table(thd, share)))
     {
+      mysql_rwlock_unlock(&LOCK_tdc);
+      check_unused(thd);
       DBUG_ASSERT(!(flags & GTS_NOLOCK));
       DBUG_ASSERT(!share->error);
       DBUG_ASSERT(!share->is_view);
@@ -1072,13 +1039,11 @@ bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
 
     TABLE_SHARE::TABLE_list::Iterator it(share->tdc.free_tables);
 #ifndef DBUG_OFF
-    if (remove_type == TDC_RT_REMOVE_ALL)
-      DBUG_ASSERT(share->tdc.used_tables.is_empty());
-    else if (remove_type == TDC_RT_REMOVE_NOT_OWN)
+    if (remove_type == TDC_RT_REMOVE_NOT_OWN)
     {
-      TABLE_SHARE::TABLE_list::Iterator it2(share->tdc.used_tables);
+      TABLE_SHARE::All_share_tables_list::Iterator it2(share->tdc.all_tables);
       while ((table= it2++))
-        DBUG_ASSERT(table->in_use == thd);
+        DBUG_ASSERT(!table->in_use || table->in_use == thd);
     }
 #endif
     /*
@@ -1105,6 +1070,7 @@ bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
     mysql_rwlock_unlock(&LOCK_flush);
 
     check_unused(thd);
+    DBUG_ASSERT(share->tdc.all_tables.is_empty() || remove_type != TDC_RT_REMOVE_ALL);
     tdc_release_share(share);
 
     /* Wait for concurrent threads to free unused objects. */