From: Dmitry Lenev Date: May 16 2012 6:37am Subject: bzr push into mysql-trunk branch (Dmitry.Lenev:3895 to 3896) WL#5772 List-Archive: http://lists.mysql.com/commits/143804 Message-Id: <20120516063712.B62FB420459@jubjub> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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).