List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:May 16 2012 6:37am
Subject:bzr push into mysql-trunk branch (Dmitry.Lenev:3895 to 3896) WL#5772
View as plain text  
 3896 Dmitry Lenev	2012-05-15
      WL#5772 "Add partitioned Table Definition Cache to avoid
      using LOCK_open and its derivatives in DML queries".
      
      Review change #8:
      - Improved comments.
      - Renamed Table_cache::rename_table_all() to free_table_all().
      - Changed size of TABLE_SHARE::cache_element array from 
        MAX_TABLE_CACHES to table_cache_instances.

    modified:
      sql/sql_base.cc
      sql/sql_base.h
      sql/sql_const.h
      sql/sys_vars.cc
      sql/table.cc
      sql/table.h
 3895 Dmitry Lenev	2012-05-15
      WL#5772 "Add partitioned Table Definition Cache to avoid
      using LOCK_open and its derivatives in DML queries".
      
      Review change #7:
      - Adjust name of some methods to better reflect what they do.
      - Make former static functions which are critical for performance
        inline methods.

    modified:
      sql/sql_base.cc
      sql/sql_base.h
      sql/sql_test.cc
      sql/table.cc
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2012-05-15 15:10:59 +0000
+++ b/sql/sql_base.cc	2012-05-15 19:36:12 +0000
@@ -782,7 +782,13 @@ uint cached_table_definitions(void)
 
 void Table_cache::free_unused_tables_if_necessary(THD *thd)
 {
-  /* We have too many TABLE instances around let us try to get rid of them. */
+  /*
+    We have too many TABLE instances around let us try to get rid of them.
+
+    Note that we might need to free more than one TABLE object, and thus
+    need the below loop, in case when table_cache_size is changed dynamically,
+    at server run time.
+  */
   if (m_table_count > table_cache_size_per_instance && m_unused_tables)
   {
     mysql_mutex_lock(&LOCK_open);
@@ -9693,22 +9699,20 @@ void tdc_flush_unused_tables()
 
 
 /**
-   Remove all or some (depending on parameter) TABLE objects for
-   the table from all table cache instances.
+   Remove and free all or some (depending on parameter) TABLE objects
+   for the table from all table cache instances.
 
    @param  thd          Thread context
    @param  remove_type  Type of removal. @sa tdc_remove_table().
    @param  key          Key identifying table.
    @param  key_length   Length of key.
 
-   TODO: Find better name? free_something() ?
-
    @note Caller should own LOCK_open and locks on all table cache
          instances.
 */
-void Table_cache::remove_table_all(THD *thd,
-                                   enum_tdc_remove_table_type remove_type,
-                                   const char *key, uint key_length)
+void Table_cache::free_table_all(THD *thd,
+                                 enum_tdc_remove_table_type remove_type,
+                                 const char *key, uint key_length)
 {
   Table_cache::assert_owner_all_and_tdc();
 
@@ -9812,7 +9816,7 @@ void tdc_remove_table(THD *thd, enum_tdc
         used.
       */
       share->version= 0;
-      Table_cache::remove_table_all(thd, remove_type, key, key_length);
+      Table_cache::free_table_all(thd, remove_type, key, key_length);
     }
     else
       (void) my_hash_delete(&table_def_cache, (uchar*) share);

=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h	2012-05-15 15:10:59 +0000
+++ b/sql/sql_base.h	2012-05-15 19:36:12 +0000
@@ -655,6 +655,11 @@ struct Table_cache_element
 
 class Table_cache
 {
+public:
+
+  /** Maximum supported number of table cache instances. */
+  static const int MAX_TABLE_CACHES= 64;
+
 private:
   /**
     The table cache lock protects the following data:
@@ -663,10 +668,11 @@ private:
     2) m_cache hash.
     3) used_tables, free_tables lists in Table_cache_element objects in
        this cache.
-    4) the element in TABLE_SHARE::cache_element[] array that corresponds
+    4) m_table_count - total number of TABLE objects in this cache.
+    5) the element in TABLE_SHARE::cache_element[] array that corresponds
        to this cache,
-    5) in_use member in TABLE object.
-    6) Also ownership of mutexes for all caches are required to update
+    6) in_use member in TABLE object.
+    7) Also ownership of mutexes for all caches are required to update
        the refresh_version and table_def_shutdown_in_progress variables
        and TABLE_SHARE::version member.
 
@@ -758,9 +764,9 @@ public:
   inline bool add_used_table(THD *thd, TABLE *table);
   inline void remove_table(TABLE *table);
 
-  static void remove_table_all(THD *thd,
-                               enum_tdc_remove_table_type remove_type,
-                               const char *key, uint key_length);
+  static void free_table_all(THD *thd,
+                             enum_tdc_remove_table_type remove_type,
+                             const char *key, uint key_length);
 
   static void free_all_unused_tables();
 

=== modified file 'sql/sql_const.h'
--- a/sql/sql_const.h	2012-05-11 16:05:27 +0000
+++ b/sql/sql_const.h	2012-05-15 19:36:12 +0000
@@ -127,8 +127,6 @@
   cache can contain at least all tables of a given statement.
 */
 #define TABLE_DEF_CACHE_MIN     400
-/** Maximum supported number of table cache instances. */
-#define MAX_TABLE_CACHES        64
 
 /*
   Stack reservation.

=== modified file 'sql/sys_vars.cc'
--- a/sql/sys_vars.cc	2012-05-14 20:05:23 +0000
+++ b/sql/sys_vars.cc	2012-05-15 19:36:12 +0000
@@ -2735,7 +2735,7 @@ static Sys_var_ulong Sys_table_cache_siz
 static Sys_var_ulong Sys_table_cache_instances(
        "table_open_cache_instances", "The number of table cache instances",
        READ_ONLY GLOBAL_VAR(table_cache_instances), CMD_LINE(REQUIRED_ARG),
-       VALID_RANGE(1, MAX_TABLE_CACHES), DEFAULT(1),
+       VALID_RANGE(1, Table_cache::MAX_TABLE_CACHES), DEFAULT(1),
        BLOCK_SIZE(1));
 
 static Sys_var_ulong Sys_thread_cache_size(

=== modified file 'sql/table.cc'
--- a/sql/table.cc	2012-05-15 15:10:59 +0000
+++ b/sql/table.cc	2012-05-15 19:36:12 +0000
@@ -322,6 +322,7 @@ TABLE_SHARE *alloc_table_share(TABLE_LIS
   char *key_buff, *path_buff;
   char path[FN_REFLEN];
   uint path_length;
+  Table_cache_element **cache_element_array;
   DBUG_ENTER("alloc_table_share");
   DBUG_PRINT("enter", ("table: '%s'.'%s'",
                        table_list->db, table_list->table_name));
@@ -334,6 +335,8 @@ TABLE_SHARE *alloc_table_share(TABLE_LIS
                        &share, sizeof(*share),
                        &key_buff, key_length,
                        &path_buff, path_length + 1,
+                       &cache_element_array,
+                       table_cache_instances * sizeof(*cache_element_array),
                        NULL))
   {
     memset(share, 0, sizeof(*share));
@@ -360,6 +363,10 @@ TABLE_SHARE *alloc_table_share(TABLE_LIS
 
     share->m_flush_tickets.empty();
 
+    memset(cache_element_array, 0,
+           table_cache_instances * sizeof(*cache_element_array));
+    share->cache_element= cache_element_array;
+
     memcpy((char*) &share->mem_root, (char*) &mem_root, sizeof(mem_root));
     mysql_mutex_init(key_TABLE_SHARE_LOCK_ha_data,
                      &share->LOCK_ha_data, MY_MUTEX_INIT_FAST);
@@ -3221,8 +3228,6 @@ uint Wait_for_flush::get_deadlock_weight
   @retval TRUE  A deadlock is found. A victim is remembered
                 by the visitor.
   @retval FALSE No deadlocks, it's OK to begin wait.
-
-  QQ: Should we move this method to sql_base.cc to allow inlining?
 */
 
 bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,

=== modified file 'sql/table.h'
--- a/sql/table.h	2012-05-11 16:05:27 +0000
+++ b/sql/table.h	2012-05-15 19:36:12 +0000
@@ -585,12 +585,14 @@ struct TABLE_SHARE
   mysql_mutex_t LOCK_ha_data;           /* To protect access to ha_data */
   TABLE_SHARE *next, **prev;            /* Link to unused shares */
   /**
-    Elements of table cache respresenting this table in each of Table_cache
-    instances. Each element of the array is protected by Table_cache::m_lock
-    in the corresponding Table_cache. False sharing should not be a problem
-    in this case as elements of this array are supposed to be updated rarely.
+    Array of table_cache_instances pointers to elements of table caches
+    respresenting this table in each of Table_cache instances.
+    Allocated along with the share itself in alloc_table_share().
+    Each element of the array is protected by Table_cache::m_lock in the
+    corresponding Table_cache. False sharing should not be a problem in
+    this case as elements of this array are supposed to be updated rarely.
   */
-  Table_cache_element *cache_element[MAX_TABLE_CACHES];
+  Table_cache_element **cache_element;
 
   /* The following is copied to each TABLE on OPEN */
   Field **field;

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (Dmitry.Lenev:3895 to 3896) WL#5772Dmitry Lenev16 May